Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android should always use NV21 image format #1116

Open
navaronbracke opened this issue Jul 5, 2024 · 12 comments
Open

Android should always use NV21 image format #1116

navaronbracke opened this issue Jul 5, 2024 · 12 comments
Assignees
Labels
android Platform android bug Something isn't working

Comments

@navaronbracke
Copy link
Collaborator

navaronbracke commented Jul 5, 2024

On Android, mobile_scanner currently has a bug, where it defaults to using YUV, instead of NV21.

Per the documentation in https://developer.android.com/reference/android/hardware/Camera.Parameters#setPreviewFormat(int)
the NV21 format is always supported. So we should always be using NV21 on Android.

Per the docs in https://developer.android.com/reference/android/graphics/ImageFormat#NV21 it seems that NV21 is the default format when no other format is requested.

The discussion in #698 also mentions that the affected device only supports NV21.

Supersedes #698 #1124 and #1077

@navaronbracke navaronbracke added bug Something isn't working android Platform android labels Jul 5, 2024
@navaronbracke navaronbracke self-assigned this Jul 5, 2024
@LorenzoVianello
Copy link

Any idea about dev time?
I'm currently stuck to PROD with an app which uses this pacakge, and I'm a little worried about

Cheers!

@navaronbracke
Copy link
Collaborator Author

I'm currently a bit stretched on some other work, but I can start implementing this fix at the end of this week.

I don't expect this to take too long to implement, though. We'll have to specify that we use NV21 for the camera preview and drop the conversion to YUV entirely. I don't expect it to be much else, in terms of a fix.

@nijs9
Copy link

nijs9 commented Sep 12, 2024

@navaronbracke Today we encountered this issue on a Galaxy A23 (we are using mobile_scanner 5.2.3). Could you let us know if your proposed fix is already included in the latest version? Thanks!

@navaronbracke
Copy link
Collaborator Author

navaronbracke commented Sep 12, 2024

The reported issue is for Samsung A23 devices. I do not yet have an update, although I want this fixed for v5.3.0

@alanlanglois
Copy link

Also happening on a Samsung A54 5G Android 14.

@navaronbracke thanks for the hard work! Do you have any ETA for the 5.3.0?

@pgiacomo69
Copy link

pgiacomo69 commented Sep 27, 2024

@navaronbracke for me it's critical too, if you have a working version I can test it in the field, distributing a Beta App to affected users.

@navaronbracke
Copy link
Collaborator Author

I am currently looking into landing a fix for a different issue, but after that, fixing the NV21 formats bug is next on my list.

@pgiacomo69
Copy link

@navaronbracke can you tell me if this fix is simple and how/where make it, I've the urgency to put it in place now, thanks :)

@navaronbracke
Copy link
Collaborator Author

navaronbracke commented Nov 14, 2024

I had another look at this issue and also had a look at what the camera plugin does.

CameraX always sends back YUV_420_888 image data, which cannot be changed.
However, the Android implementation of camera always converts YUV_420_888 to NV21, for sending back to Dart (in case people want to process frames on the Dart side). Conveniently, MLKit also requires NV21 as the format for the input images.

So we will have to always convert YUV_420_888 image data to NV21 before we pass it to the image analyzer.

I believe that this will require us to do the following:

  1. Write a new converter class that converts image data to the NV21 image format
  1. In MobileScanner.kt convert the incoming image data to NV21 and only then call process() for he converted input image (this is probably the bug, as we do not convert anything in the current implementation)
  2. Update the YuvToRgbConverter.kt class to handle the new workflow
  • depending on whether you keep or discard the original Yuv image data, you can keep the old converter around, or write a new converter that converts NV21 image data to RGB (but I fear that this is extra work that could be avoided? Because you would be adding an extra conversion in this case)
  • It uses some deprecated API's (see Deprecated RenderScript APIs in Mobile Scanner Plugin #1142), so if you can fix that, please do
  • It also has some other warnings, which you might want to address, to keep the code maintainable

Do bear in mind when starting your work, to start from the ios-vision branch, as that is the current beta for version 7.0.0

@julfabre
Copy link

Hi,
it's same here, user with Samsung phones can not use code bar detection ....

Thanks you @navaronbracke for the resume, can anyone know kotlin, have time to fix this ?

Is a quite big issue for the module, that all Samsung phone can't use any apps with code bar detection ...

@pgiacomo69
Copy link

@julfabre I would like to implement that fix, I’m waiting because I haven’t an affected phone in my hands. @navaronbracke, can the problem be simulated, forcing the weong camera output?

@navaronbracke
Copy link
Collaborator Author

I don't have a Samsung device at hand either. I'm not sure if you can force the wrong camera output.
The problem isn't that the camera outputs frames in YUV_420_888 (which it always does), but that we pass those directly to MLKit, without converting these to NV21 first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android Platform android bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants