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

Crop image programatically before scanning #1177

Closed
Tracked by #1255
M123-dev opened this issue Mar 3, 2022 · 16 comments
Closed
Tracked by #1255

Crop image programatically before scanning #1177

M123-dev opened this issue Mar 3, 2022 · 16 comments
Labels
🤳 MLKit 🎯 P1 performance 🤳🥫 Scan We need to be able to scan on low-end, old devices, even with a bad camera, connexion… speed
Milestone

Comments

@M123-dev
Copy link
Member

M123-dev commented Mar 3, 2022

What

  • Resize the image in mk_kit_scan_page,dart -> _processCameraImage() , to only scan the upper part
  • To not block the main thread it would be good to do this in a isolate
  • There should be a way a easy way to deactivate the resizing to measure the performance difference
  • This package could be worth checking out

Part of

@monsieurtanuki
Copy link
Contributor

@M123-dev I've quickly had a look at _processCameraImage(), and here are some suggestions (while I know nothing about scan and camera...):

  • I haven't seen anything related to the the actual "little scan rectangle" - wouldn't we be better off just restricting the process to that area? (smaller than the full screen)
  • I believe the critical part is the "image to buffer" part - to be confirmed by profiler. Here again we could save tons of computations restricting to the "little scan rectangle", and with an array that already has the right size:
    final WriteBuffer allBytes = WriteBuffer();
    for (final Plane plane in image.planes) {
      allBytes.putUint8List(plane.bytes);
    }
    final Uint8List bytes = allBytes.done().buffer.asUint8List();
  • I don't believe in isolate, but perhaps perhaps it's interesting.
  • I've just tried with my old smartphone and my assumptions were right: the barcode was scanned even when not in the target rectangle. We probably scan the whole image. Scanning just the rectangle would save tons of computations.

@M123-dev
Copy link
Member Author

M123-dev commented Mar 4, 2022

Yes @monsieurtanuki this is right. I'd rather choose to only use the upper part of the image, this is probably easier to do and doesn't restrict as much.
My idea was to crop the image with maybe this package: https://github.com/brendan-duncan/image, but only using the right bytes would be way better, the only problem is how to decide which bytes to take

@M123-dev M123-dev changed the title Resize image programatically before scanning Crop image programatically before scanning Mar 4, 2022
@monsieurtanuki
Copy link
Contributor

@M123-dev Let me have a look at that crop part.
Once you've understood the data structure, it's very easy to crop, it's like skipping bytes for each row:

/// A single color plane of image data.
///
/// The number and meaning of the planes in an image are determined by the
/// format of the Image.
class Plane {
  Plane._fromPlatformData(Map<dynamic, dynamic> data)
      : bytes = data['bytes'],
        bytesPerPixel = data['bytesPerPixel'],
        bytesPerRow = data['bytesPerRow'],
        height = data['height'],
        width = data['width'];

  /// Bytes representing this plane.
  final Uint8List bytes;

  /// The distance between adjacent pixel samples on Android, in bytes.
  ///
  /// Will be `null` on iOS.
  final int? bytesPerPixel;

  /// The row stride for this color plane, in bytes.
  final int bytesPerRow;

  /// Height of the pixel buffer on iOS.
  ///
  /// Will be `null` on Android
  final int? height;

  /// Width of the pixel buffer on iOS.
  ///
  /// Will be `null` on Android.
  final int? width;
}

@monsieurtanuki
Copy link
Contributor

I confirm that cropping looks easy from what I read in wikipedia.
Working on it...

@M123-dev
Copy link
Member Author

M123-dev commented Mar 4, 2022

If you say so @monsieurtanuki , I've only skimmed the article, but I have to say for me with physics major it doesn't look exactly easy. I'm really look forward to the PR

monsieurtanuki added a commit to monsieurtanuki/smooth-app that referenced this issue Mar 5, 2022
New files:
* `abstract_camera_image_getter.dart`: Abstract getter of Camera Image, for barcode scan.
* `camera_image_cropper.dart`: Camera Image Cropper, in order to limit the barcode scan computations.
* `camera_image_full_getter.dart`: Camera Image helper where we get the full image.

Impacted file:
* `ml_kit_scan_page.dart`: now using new `CameraImageFullGetter` class - same full screen camera image capture as before for the moment.
@monsieurtanuki
Copy link
Contributor

@M123-dev Actually we don't even know why exactly the phones heat up. Possible reasons:

  1. the camera itself with its stream of data
  2. when we receive an image, the pre-process part
  3. when we pre-processed the image, the actual barcode scan

I'm about to improve my PR (#1184) in order to have clearer minds. And this is going to be a new parameter in the dev mode where we can decide:

  1. we don't care about the camera data at all - we just receive it
  2. we just pre-process the full image data, but without barcode scan call
  3. we just pre-process the image data for only the top half of the image, but without barcode scan call
  4. we pre-process the full image data and call the barcode scan - current status of the app
  5. we pre-process only the top half of the image data and call the barcode scan

I suspect the barcode scan method to be the problem (full image or top half).
If so we should change the UX and for instance let the user decide when to actually try to scan a barcode.
But first, the PR and some tests IRW.

@teolemon IRW means "in real world" ;)

@M123-dev
Copy link
Member Author

M123-dev commented Mar 6, 2022

Sounds like a good plan @monsieurtanuki, just ping me when you are done and I'll have a look

monsieurtanuki added a commit to monsieurtanuki/smooth-app that referenced this issue Mar 6, 2022
Impacted files:
* `ml_kit_scan_page.dart`: added optional scan / no scan options according to dev mode
* `user_preferences.dart`: added methods for dev mode int values
* `user_preferences_dev_mode.dart`: added a "scan mode" option on dev mode
monsieurtanuki added a commit that referenced this issue Mar 6, 2022
…#1184)

New files:
* `abstract_camera_image_getter.dart`: Abstract getter of Camera Image, for barcode scan.
* `camera_image_cropper.dart`: Camera Image Cropper, in order to limit the barcode scan computations.
* `camera_image_full_getter.dart`: Camera Image helper where we get the full image.

Impacted file:
* `ml_kit_scan_page.dart`: added optional scan / no scan options according to dev mode
* `user_preferences.dart`: added methods for dev mode int values
* `user_preferences_dev_mode.dart`: added a "scan mode" option on dev mode
@monsieurtanuki
Copy link
Contributor

@M123-dev Had a chance to run 2-minute tests with each configuration to check when the smartphone is heating up?

@M123-dev
Copy link
Member Author

Not yet @monsieurtanuki , I'll try tomorrow. Do you have any experience on how to measure performance, besides "It felt okay". Does the profiler or logcat have any features for that

@monsieurtanuki
Copy link
Contributor

I think we are fortunate enough to have a feature that heats up the smartphone rather quickly, therefore the "it feels okay" could be good enough in a first approach, for like 3-minute tests on each configuration.
Beyond that, when you run the flutter app you get logs like that, with useful links:

An Observatory debugger and profiler on iPhone 8 Plus is available at:
http://127.0.0.1:54162/MljiZYyPbhM=/
The Flutter DevTools debugger and profiler on iPhone 8 Plus is available at:
http://127.0.0.1:9100?uri=http://127.0.0.1:54162/MljiZYyPbhM=/

Again, the profiler suggests that you run flutter "in profile mode" (something like --profile), but probably the problem will be so obvious that we won't need that accuracy level.

@M123-dev
Copy link
Member Author

Ok I'll have a look at it tomorrow

@M123-dev
Copy link
Member Author

Sorry for the late response, I did some excessive testing on friday.
The tests looked like this:

  • 3min stock camera and after that 3m testing for every option and a one minute break inbetween each test.
  • Phone lying down on the table with no movement so that the circumstances remain the same and factors such as the autofocus and light adjustments are excluded

I first tried it on my own pretty new phone and the performance / temperature was great. The tests went all without a problem (and I even learned that the Samsung stock camera automatically closes itself after two minutes without interaction), the phone got minimal warm over time but it stayed that way and when switching to other apps there was no stutter at all.

So I did the same test on my mother's phone. It's a midclass phone from 2019 (3 Years old) and interestingly, the results remained the same. No performance or temperature problems at all, so I did an extended 30 minute test in half picture scannning mode directly after that and I noticed NO change,

This means that the test unfortunately did not provide any information about what takes the most power from the process, but it did show that the performance is generally quite good. Maybe the changes that we only scan every tenth image has had a direct effect

@monsieurtanuki
Copy link
Contributor

@M123-dev I guess you were not the right person to test it as you've never experienced the heating up of your smartphone, even with the current full screen scan.
Who had this problem in the first place?
I could test on my 2015 Samsung but I guess it's not the target device. (especially because I can no longer access OFF API from it for httpS reasons)

@M123-dev
Copy link
Member Author

M123-dev commented Mar 13, 2022

@jasmeet0817 initially raised the issue, can you maybe test again.

But as long as you are able to run the app why not, you don't need to scan any product, this isn't the target audience, but it would still be great if we have decent performance there aswell

@monsieurtanuki
Copy link
Contributor

@M123-dev In addition to the fact that I cannot access OFF API, the USB connection to my old smartphone is a bit loose, therefore I often cannot test beyond an approximate 2-minute threshold. Then I get the "Lost connection to device" message. Then most of the time I have to switch off my smartphone (because it's not recognized anymore for some reason), switch it on again, in the meanwhile I lose my internet connection because my smartphone is currently my wifi access.

Ok, technically I can test. But it's rather painful. And probably pointless, because the device where the problem appeared has different characteristics. And if ever I tested and told you: "the problem is that we run the barcode scan in an endless loop (that's a fact): we need to change the UX with a 'scan now' button", you'd probably say "it does not look that way in the mocks, and btw we should test on the device where the problem appeared".

Therefore I suggest tests to be run on the device where the problem appeared.

@teolemon teolemon added 🤳🥫 Scan We need to be able to scan on low-end, old devices, even with a bad camera, connexion… and removed rocket science labels Mar 18, 2022
@teolemon
Copy link
Member

Repository owner moved this from Todo (ready 2 dev) to Done in 🤳🥫 The Open Food Facts mobile app (Android & iOS) Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤳 MLKit 🎯 P1 performance 🤳🥫 Scan We need to be able to scan on low-end, old devices, even with a bad camera, connexion… speed
Development

No branches or pull requests

3 participants