-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Draft: [camera_android] Add NV21 as an image stream format #6985
Conversation
I'm not sure how to fix that formatting issue being flagged by repo_checks. I tried |
The contributing guid has the necessary info about it. There is a link to specific documentation about the tools for testing, formatting, etc. |
Fixed the formatting and licenses but I can't clearly see what |
@acoutts you also may find these discussions about performance of this approach useful: |
Thanks @gordinmitya - i actually found another example which works even better and i'm redoing my approach for this PR. The official google MLKit demo app has a good bmp conversion class which contains some functions to convert YUV420 into NV21: https://github.com/googlesamples/mlkit/blob/master/android/vision-quickstart/app/src/main/java/com/google/mlkit/vision/demo/BitmapUtils.java#L177-L220 Instead of making this PR try and process the YUV data to remove padding, we shouldn't process YUV at all. If a user wants YUV, they should get it exactly as it comes from the camera device - padding and all. It should be up to them to handle padding or do whatever they want to do with it. The problem really is a bad implementation everywhere on how to convert the YUV to NV21 in dart. The most common use case for this is for MLKit which needs NV21 as an input image. All of the dart examples I've seen simply concatenate all 3 planes into one big Uint8List and pass it into MLKit. For devices with padding (like the vivo), this of course doesn't work as the image is invalid. This bad implementation of YUV->NV21 conversion is what my issue here is really about.
Instead I am going to add NV21 as a new image format group option for the flutter camera, using the bmp util functions from google's MLKit demo to convert the frames in native before sending it over to dart. This will handle all padding situations and make sure a valid NV21 image is delivered to the dart image stream. This way there's no change to the existing API, and instead we offer users the ability to get NV21 from the device if they want. |
I've updated my approach to convert to nv21 on the android side before sending back to dart. |
@acoutts after benchmarking, I realized that MLKit solution is more efficient than mine from yuv2buf + it always outputs in NV21 format. So I think you implemented it in the best way. The path with per-pixel manipulations is quite rare (when a phone returns yuv_420_888 image, meaning u and v planes both have pixelStride == 1). However, even in this case, performance is okay. Shame to me. I should have benchmarked my solution more extensively earlier. 🤦 I'll update the code in my repo soon. |
Part 1 ( @camsim99 @bparrishMines i've closed this PR in favor of the 3 split ones above which organizes the changes by each package modified. Please perform reviews on those. We need part 1 merged and published first before 2 and 3 can utilize the new ImageFormatGroup option for nv21. |
Alright I just finished fixing up the tests with the latest approach. This is now ready for review @camsim99 @bparrishMines @stuartmorgan |
We've just completed the migration of the plugin code to the flutter/packages repository, as described in https://flutter.dev/go/flutter-plugins-repo-migration, and this repository is now being archived. Unfortunately that means that all in-progress PRs here must be moved to flutter/packages. Please see our instructions for an explanation of how to move your PR, and if you have any issues moving your PR please don't hesitate to reach out in the #hackers-ecosystem channel in Discord. Our apologies that your PR was caught in this one-time transition. We're aware that it's disruptive in the short term, and appreciate your help in getting us to a better long-term state! |
This PR adds NV21 as an image stream format for android devices. This is the format required for things like MLKit. Unfortunately a lot of people tend to implement the yuv->nv21 incorrectly in dart, which results in countless issues of users saying the image stream doesn't work, or that mlkit doesn't work.
By allowing the plugin to send NV21 to dart, this will fix all of those yuv conversion issues.
Highlights:
ImageReader
calledImageStreamReader
and moved the image stream logic into this class to clean up theCamera
class. This allows us to test the image stream handler in isolation.ImageStreamReader
to make sure it only calls theremovePlaneBufferPadding
function for planes that contain padding.ImageStreamReaderUtils
which contains the logic for trimming the padding.ImageStreamReaderUtils
to make sure it only removes padding when a given buffer contains padding to be removed.removePlaneBufferPadding
is only called when padding is present, and to confirm that ifremovePlaneBufferPadding
does happen to be called with a buffer containing no padding, it still returns a valid buffer.List which issues are fixed by this PR. You must list at least one issue.
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
dart format
.)[shared_preferences]
pubspec.yaml
with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.md
to add a description of the change, following repository CHANGELOG style.///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.