Closed teolemon closed 2 years ago
There is an issue I’m currently working on: a product added, then removed, can not be scanned anymore.
so we have to be really careful when we talk about the scan. It might be side effects from other parts of the code.
@teolemon Any news from the iOS users? On my side, I've tried to reproduce the OOM issue on iOS by letting the camera for hours… without any problem :(
For the permission's screen, do you know where Quentin took the illustrations for the onboarding?
My suggestions on the topic as a back-seat driver, ordered by decreasing relevancy:
For the isolate thing, we only have 1 isolate running. If a decoding is happening, the request is put in a queue, waiting for the task to be done or replaced by another scan.
You mean that beside the isolate currently running, there's at maximum another one pending, am I right? Even there, that means that we have 2 isolates, memory-wise. Plus all the memory tap dancing when you're replacing the pending isolate (if I've understood well). I guess that we should not put the next isolate in the queue, and just ignore it: the camera refresh is fast enough to trigger another scan event and create a new isolate (only when no isolate is running). Does that make sense?
No there is just one Isolate, but I thought you meant that each photo = one Isolate, which is not the case.
The data is only sent when the decoding happens, so no worry about that.
But it seems that the issue is not about the Isolate itself, but more related to the camera. Unless I have a device with the issue, I can’t say more, because users are only stating « it doesn’t work ». Really helpful, thanks >_<
No there is just one Isolate, but I thought you meant that each photo = one Isolate, which is not the case. The data is only sent when the decoding happens, so no worry about that.
I need to have a look at your code then, there's something I don't understand.
But it seems that the issue is not about the Isolate itself, but more related to the camera.
Indeed, that's why I ranked the specific camera scan page/widget as more important.
Unless I have a device with the issue, I can’t say more, because users are only stating « it doesn’t work ». Really helpful, thanks >_<
Unfortunately that's a common pattern for users :) That's also why I recommended a less ambitious specific page/widget, with less reasons to fail. And not a degraded UX (IMHO).
@g123k As I said, I don't think the isolate
part is the most problematic.
That said, I would like you to run performance tests (when you have time) around the types of barcodes. Currently you use many types, and many of them are probably irrelevant (cf. https://www.scandit.com/blog/types-barcodes-choosing-right-barcode/). I don't know how faster things would be if we check for fewer types.
BarcodeFormat.ean8, // Retail, ok
BarcodeFormat.ean13, // Retail, ok
BarcodeFormat.upca, // Retail, ok
BarcodeFormat.upce, // Retail, ok
BarcodeFormat.code39, // Automotive and Defense, probably irrelevant
BarcodeFormat.code93, // Retail, Manufacturing and Logistics, not sure
BarcodeFormat.code128, // Supply Chain, probably irrelevant
BarcodeFormat.itf, // Packaging, not sure
BarcodeFormat.codabar, // Logistics, Healthcare and Education, probably irrelevant
I also think that your computation of average time is slightly flawed, because you consider both actual barcode scan operations (which is good) AND aborted operations (that failed because another operation is already running) (which is not relevant).
"Kann kein neues Produkt hinzufügen, wenn ich es einscanne."
"Kann kein neues Produkt hinzufügen, wenn ich es einscanne."
Translated: I can't add new products when I scan them
Once again, the sentence by itself doesn't help us. Is he able to scan, and no product is showing? Or can't he scan?
That's the real issue we have… with this issue.
@teolemon Any feedback from the iOS users you emailed?
@g123k I agree, that can lead to different interpretations. Wouldn't that be an appropriate reason to implement #2662 sooner?
I lost the draft email we made during the call. I just did this https://calendly.com/openfoodfacts/debugging-the-scan-on-open-food-facts-together @g123k
Just a quick update: this morning we were finally able to see the problem in a real device with @teolemon. The camera is working well, but the scan doesn’t seem to work.
I will receive the faulty device tomorrow Friday (a OnePlus Nord CE 2) and hope this time, we will fix the issue.
Good luck @g123k!
Camera crash log (smoothie downloaded today from play store on Android 21). The onboarding is fine, but the app crashes when I land on the camera/scan page:
Installing build/app/outputs/flutter-apk/app.apk... 56,8s
Syncing files to device SM G360F... 456ms
Flutter run key commands.
r Hot reload. 🔥🔥🔥
R Hot restart.
h List all available interactive commands.
d Detach (terminate "flutter run" but leave application running).
c Clear the screen
q Quit (terminate the application on the device).
💪 Running with sound null safety 💪
An Observatory debugger and profiler on SM G360F is available at:
http://127.0.0.1:61088/omqQd85GBPI=/
The Flutter DevTools debugger and profiler on SM G360F is available at:
http://127.0.0.1:9100?uri=http://127.0.0.1:61088/omqQd85GBPI=/
I/flutter (13731): 2022-08-22T15:01:11.466688 I SmoothService: New app session started
W/ArrayUtils(13731): Ignoring invalid value vintage-cold
W/ArrayUtils(13731): Ignoring invalid value point-blue
W/ArrayUtils(13731): Ignoring invalid value point-red-yellow
W/ArrayUtils(13731): Ignoring invalid value emboss
W/ArrayUtils(13731): Ignoring invalid value sketch
W/ArrayUtils(13731): Ignoring invalid value neon
W/ArrayUtils(13731): Ignoring invalid value beauty
W/ArrayUtils(13731): Ignoring invalid value vintage-cold
W/ArrayUtils(13731): Ignoring invalid value point-blue
W/ArrayUtils(13731): Ignoring invalid value point-red-yellow
W/ArrayUtils(13731): Ignoring invalid value emboss
W/ArrayUtils(13731): Ignoring invalid value sketch
W/ArrayUtils(13731): Ignoring invalid value neon
W/ArrayUtils(13731): Ignoring invalid value beauty
I/Timeline(13731): Timeline: Activity_idle id: android.os.BinderProxy@763f797 time:53574227
I/flutter (13731): unhandled element filter; Picture key: AssetBundlePictureKey(bundle: PlatformAssetBundle#90112(), name: "assets/onboarding/reinvention.svg", colorFilter: null, theme: SvgTheme(currentColor: Color(0xff000000), fontSize: 14.0, xHeight: 7.0))
D/ViewRootImpl(13731): ViewPostImeInputStage ACTION_DOWN
D/ViewRootImpl(13731): ViewPostImeInputStage ACTION_DOWN
D/ViewRootImpl(13731): ViewPostImeInputStage ACTION_DOWN
D/ViewRootImpl(13731): ViewPostImeInputStage ACTION_DOWN
D/ViewRootImpl(13731): ViewPostImeInputStage ACTION_DOWN
I/flutter (13731): unhandled element defs; Picture key: PictureKey(Instance of 'NetworkPictureKeyData', colorFilter: null, theme: SvgTheme(currentColor: Color(0xff000000), fontSize: 14.0, xHeight: 7.0))
I/flutter (13731): unhandled element sodipodi:namedview; Picture key: PictureKey(Instance of 'NetworkPictureKeyData', colorFilter: null, theme: SvgTheme(currentColor: Color(0xff000000), fontSize: 14.0, xHeight: 7.0))
I/flutter (13731): unhandled element metadata; Picture key: PictureKey(Instance of 'NetworkPictureKeyData', colorFilter: null, theme: SvgTheme(currentColor: Color(0xff000000), fontSize: 14.0, xHeight: 7.0))
D/ViewRootImpl(13731): ViewPostImeInputStage ACTION_DOWN
D/ViewRootImpl(13731): ViewPostImeInputStage ACTION_DOWN
D/ViewRootImpl(13731): ViewPostImeInputStage ACTION_DOWN
D/ViewRootImpl(13731): ViewPostImeInputStage ACTION_DOWN
I/flutter (13731): 2022-08-22T15:01:53.140629 D PermissionListener: New permission value: DevicePermission{permission: Permission.camera, status: DevicePermissionStatus.checking}
I/flutter (13731): 2022-08-22T15:01:55.571279 D PermissionListener: New permission value: DevicePermission{permission: Permission.camera, status: DevicePermissionStatus.granted}
I/flutter (13731): 2022-08-22T15:01:55.877159 D CameraController: New controller created
I/flutter (13731): 2022-08-22T15:01:55.947373 D CameraController: Controller is being initialized
I/flutter (13731): 2022-08-22T15:01:55.950917 D CameraController: New camera state = _CameraState.beingInitialized
W/ArrayUtils(13731): Ignoring invalid value vintage-cold
W/ArrayUtils(13731): Ignoring invalid value point-blue
W/ArrayUtils(13731): Ignoring invalid value point-red-yellow
W/ArrayUtils(13731): Ignoring invalid value emboss
W/ArrayUtils(13731): Ignoring invalid value sketch
W/ArrayUtils(13731): Ignoring invalid value neon
W/ArrayUtils(13731): Ignoring invalid value beauty
W/ArrayUtils(13731): Ignoring invalid value vintage-cold
W/ArrayUtils(13731): Ignoring invalid value point-blue
W/ArrayUtils(13731): Ignoring invalid value point-red-yellow
W/ArrayUtils(13731): Ignoring invalid value emboss
W/ArrayUtils(13731): Ignoring invalid value sketch
W/ArrayUtils(13731): Ignoring invalid value neon
W/ArrayUtils(13731): Ignoring invalid value beauty
I/CameraManager(13731): Using legacy camera HAL.
W/ArrayUtils(13731): Ignoring invalid value vintage-cold
W/ArrayUtils(13731): Ignoring invalid value point-blue
W/ArrayUtils(13731): Ignoring invalid value point-red-yellow
W/ArrayUtils(13731): Ignoring invalid value emboss
W/ArrayUtils(13731): Ignoring invalid value sketch
W/ArrayUtils(13731): Ignoring invalid value neon
W/ArrayUtils(13731): Ignoring invalid value beauty
I/Camera (13731): startPreview
I/CameraDeviceState(13731): Legacy camera service transitioning to state CONFIGURING
I/RequestThread-0(13731): Configure outputs: 2 surfaces configured.
D/Camera (13731): app passed NULL surface
I/RequestThread-0(13731): configureOutputs - set take picture size to 1280x720
I/CameraDeviceState(13731): Legacy camera service transitioning to state IDLE
I/Camera (13731): CameraCaptureSession onConfigured
I/Camera (13731): Updating builder settings
D/Camera (13731): Updating builder with feature: FpsRangeFeature [7500, 120000]
D/Camera (13731): Updating builder with feature: NoiseReductionFeature fast
I/Camera (13731): updateNoiseReduction | currentSetting: fast
D/Camera (13731): Updating builder with feature: FlashFeature auto
D/Camera (13731): Updating builder with feature: SensorOrientationFeature 90
D/Camera (13731): Updating builder with feature: ExposureLockFeature auto
D/Camera (13731): Updating builder with feature: ExposurePointFeature null
D/Camera (13731): Updating builder with feature: AutoFocusFeature auto
D/Camera (13731): Updating builder with feature: ZoomLevelFeature 1.0
D/Camera (13731): Updating builder with feature: ResolutionFeature high
D/Camera (13731): Updating builder with feature: ExposureOffsetFeature 0.0
D/Camera (13731): Updating builder with feature: FocusPointFeature null
I/RequestQueue(13731): Repeating capture request set.
W/LegacyRequestMapper(13731): convertRequestMetadata - control.awbRegions setting is not supported, ignoring value
I/Camera (13731): unlockAutoFocus
I/RequestQueue(13731): Repeating capture request cancelled.
I/RequestQueue(13731): Repeating capture request set.
I/RequestQueue(13731): Repeating capture request cancelled.
I/RequestQueue(13731): Repeating capture request set.
I/RequestQueue(13731): Repeating capture request cancelled.
I/RequestQueue(13731): Repeating capture request set.
I/RequestQueue(13731): Repeating capture request cancelled.
I/RequestQueue(13731): Repeating capture request set.
I/RequestQueue(13731): Repeating capture request cancelled.
E/AndroidRuntime(13731): FATAL EXCEPTION: RequestThread-0
E/AndroidRuntime(13731): Process: org.openfoodfacts.scanner, PID: 13731
E/AndroidRuntime(13731): java.lang.RuntimeException: startPreview failed
E/AndroidRuntime(13731): at android.hardware.Camera.startPreview(Native Method)
E/AndroidRuntime(13731): at android.hardware.camera2.legacy.RequestThreadManager.startPreview(RequestThreadManager.java:275)
E/AndroidRuntime(13731): at android.hardware.camera2.legacy.RequestThreadManager.doPreviewCapture(RequestThreadManager.java:317)
E/AndroidRuntime(13731): at android.hardware.camera2.legacy.RequestThreadManager.access$1600(RequestThreadManager.java:61)
E/AndroidRuntime(13731): at android.hardware.camera2.legacy.RequestThreadManager$5.handleMessage(RequestThreadManager.java:756)
E/AndroidRuntime(13731): at android.os.Handler.dispatchMessage(Handler.java:98)
E/AndroidRuntime(13731): at android.os.Looper.loop(Looper.java:135)
E/AndroidRuntime(13731): at android.os.HandlerThread.run(HandlerThread.java:61)
I/flutter (13731): 2022-08-22T15:01:57.730979 D LifeCycleManagerState: Visibility: visible
I/flutter (13731): 2022-08-22T15:01:57.809114 D LifeCycleManagerState: Lifecycle: on pause
I/flutter (13731): 2022-08-22T15:01:57.844509 D CameraController: Image stream being paused
I/flutter (13731): 2022-08-22T15:01:57.852374 D CameraController: New camera state = _CameraState.beingPaused
I/RequestQueue(13731): Repeating capture request set.
I/RequestQueue(13731): Repeating capture request cancelled.
E/OpenGLRenderer(13731): SFEffectCache:clear(), mSize = 0
W/Adreno-ES20(13731): <core_glDeleteShader:170>: GL_INVALID_VALUE
I/flutter (13731): 2022-08-22T15:01:58.194422 D CameraController: New camera state = _CameraState.paused
Lost connection to device.
Which device was it?
It's my canary device: 2015 Samsung Galaxy Core Prime. I'm currently running tests on it; I'll keep you posted.
Ok. Please keep me posted. Could you test if in debug mode you also have the issue?
That's what I'm doing, after the crash on the release version. Could be related to memory: on the release version I saw the camera working once, then freeze, then crash. And later, always crash. On the debug mode more memory is asked, and I've never seen the camera work at least once. When I say memory, it probably means the scan. My current test for the next 20 minutes: make the camera work, with a disabled barcode scan.
I have to go now. Not enough data so far to say something relevant.
Good news: I've finally received the OnePlus phone and I can reproduce the issue. Bad news: MLKit doesn't scan anything
Testing the example from the Flutter library (google_mlkit_barcode_scanning) doesn't scan.
… But after trying the Google sample app, I'm able to detect barcodes with MLKit. So it seems the error is on the library we use. I will now try to understand what are the differences (which may takes some time).
Maybe we have different ml kit version either a older maybe buggy one or the native app uses the not bundled release
Unfortunately, that's the same library version on both apps. 'com.google.mlkit:barcode-scanning:17.0.2'
So two options:
I didn't have time this morning to take a better look at the problem. But I think our current implementation is broken, by having 2 native plugins, as it forces us to continuously send messages between the Dart code and the native one.
If I sum, here is the current flow:
I was wondering, if we shouldn't create our own plugin (= native) handling both the camera and MLKit:
Instead of doing Dart <-> camera preview <-> Native and Dart <-> camera data <-> Native, we will have : Only two outputs from the plugin: the camera feed on one side and decoded barcoded, no more back and forth messages.
What would you think about this kind of solution? Because it could also (partially) fix the issue with Out of Memory.
@g123k this plan makes complete sense, doing it like that would also save use the hassle with the isolate as native code by default runs on another thread. The only thing which only you can give answer to is if it justifies the time needed to get this to work.
@g123k The main question is: are you fortunate enough to have a device on which the camera/scan currently crashes?
This is such an important feature - on my smartphone it's even beyond important as the app crashes on home page.
I'm not at all against a heavy refactoring if things work better (and I would strongly prefer the refactoring to be coded in flutter). If we're talking more specifically about your suggested solution, I'm not sure I see the added value: I'm in favor of stupid codes that do perfectly a stupid task, like in Unix when you "pipe" commands like ls -l | awk '{print $5 " " $9}' | sort -rn
. We could have better performances with a specific code that does the same thing, but we would lose on flexibility and maintenance.
For scan issues, after spending hours trying to understand the root cause of the issue, I have to surrender… 😔
This is the cause of MLKit being unable to decode any barcode
As we've already discussed, the solution is to use an all-in-one library, but we have to deploy a fixed version of the app ASAP.
I will then develop an "alternative" mode for the scan, where instead of passing the camera feed, it will be a File. This requires a few minor changes on the camera library + on Smoothie.
Also the app will use this "alternative" mode by default on OnePlus devices + can be changed via the settings. @teolemon Could you create a list of brands or models being affected by the scan issue? (not the camera 🤪), please?
Just happened on my Pixel 6. I Can't scan anymore https://photos.app.goo.gl/WRowi487cFo6SJru9
@g123k reverting to stable fixes the issue for me
I’m pretty sure your issue is related to https://github.com/openfoodfacts/smooth-app/issues/2905
but let’s try to revert
On the latest downloaded version from the Play Store (3.8.1+722) I still have the crash when I land on the home/camera page:
2022-09-04 14:09:37.350 22792-22885/? E/AndroidRuntime: FATAL EXCEPTION: RequestThread-0
Process: org.openfoodfacts.scanner, PID: 22792
java.lang.RuntimeException: startPreview failed
at android.hardware.Camera.startPreview(Native Method)
at android.hardware.camera2.legacy.RequestThreadManager.startPreview(RequestThreadManager.java:275)
at android.hardware.camera2.legacy.RequestThreadManager.doPreviewCapture(RequestThreadManager.java:317)
at android.hardware.camera2.legacy.RequestThreadManager.access$1600(RequestThreadManager.java:61)
at android.hardware.camera2.legacy.RequestThreadManager$5.handleMessage(RequestThreadManager.java:756)
at android.os.Handler.dispatchMessage(Handler.java:98)
at android.os.Looper.loop(Looper.java:135)
at android.os.HandlerThread.run(HandlerThread.java:61)
and
2022-09-04 14:09:37.960 22792-22792/? E/ActivityThread: Activity org.openfoodfacts.scanner.MainActivity has leaked IntentReceiver q4.a$a@1285d5e6 that was originally registered here. Are you missing a call to unregisterReceiver()?
android.app.IntentReceiverLeaked: Activity org.openfoodfacts.scanner.MainActivity has leaked IntentReceiver q4.a$a@1285d5e6 that was originally registered here. Are you missing a call to unregisterReceiver()?
at android.app.LoadedApk$ReceiverDispatcher.<init>(LoadedApk.java:898)
at android.app.LoadedApk.getReceiverDispatcher(LoadedApk.java:699)
at android.app.ContextImpl.registerReceiverInternal(ContextImpl.java:1815)
at android.app.ContextImpl.registerReceiver(ContextImpl.java:1795)
at android.app.ContextImpl.registerReceiver(ContextImpl.java:1789)
at android.content.ContextWrapper.registerReceiver(ContextWrapper.java:495)
at q4.a.k(DeviceOrientationManager.java:3)
at q4.b.<init>(SensorOrientationFeature.java:7)
at g4.c.a(CameraFeatureFactoryImpl.java:1)
at g4.d.k(CameraFeatures.java:8)
at f4.t.<init>(Camera.java:12)
at f4.p0.c(MethodCallHandlerImpl.java:11)
at f4.p0.d(MethodCallHandlerImpl.java:1)
at f4.p0.a(MethodCallHandlerImpl.java)
at f4.o0.a(R8$$SyntheticClass)
at f4.a0.d(CameraPermissions.java:4)
at f4.p0.onMethodCall(MethodCallHandlerImpl.java:80)
at b4.k$a.a(MethodChannel.java:2)
at o3.c.l(DartMessenger.java:2)
at o3.c.m(DartMessenger.java:2)
at o3.c.i(DartMessenger.java)
at o3.b.run(R8$$SyntheticClass)
at android.os.Handler.handleCallback(Handler.java:739)
at android.os.Handler.dispatchMessage(Handler.java:95)
at android.os.Looper.loop(Looper.java:135)
at android.app.ActivityThread.main(ActivityThread.java:5608)
at java.lang.reflect.Method.invoke(Native Method)
at java.lang.reflect.Method.invoke(Method.java:372)
at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1397)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1192)
In your case, it’s coming from the camera driver developed by Samsung. So it’s only for your device. I’m not sure it’s worth the case to support only one device.
instead we should focus on global scan issues
Could you also create issues per device instead of using this thread, please? This is very difficult for me to follow the issue on Slack, Meet, GitHub…
Fair enough @g123k: #2917
You can have two kinds of issues:
If you are in the second case, please download this APK: drive.google.com/file/d/1F6S0pGpf8OQHhqKSw1opWQPRxpcmYdDV/view?usp=sharing (app name: ML Kit in Flutter) and go to Barcode Scanning (first item) and tell me if you're able to scan.
Thanks
Since the alternative mode is now available in the app, what's the plan to check if it fixes the issue?
The feedback we have seem to indicate that the issue is now fixed with the alternative mode. I will work on a PR to enable the alternative mode by default and make the current one available in the settings.
It think we should consider this issue as fixed with the "alternative mode" enabled by default
What
We have a big scan issue. We discussed it at length with @g123k the other day. I'm pinning this until we solve it, and people are happy again.
People are complaining on all channels (email, appstore, app survey)
I believe we didn't ship some of the fixes in the last release, so native bug is still on-going, and people are still complaining: https://github.com/openfoodfacts/smooth-app/pull/2740/files#diff-5898cc81afbf2560f009273208f7247c03d829eabc864ab252c5ef475a713f5eR151
Complaints are focused on Redmi, Oppo, Oneplus
Here's raw user feedback if you'd like to peruse, Scan issues are classified as such: https://docs.google.com/spreadsheets/d/1KsJv8_RxfDDd5ngmeQ4k8ELsudl-YhleXEoDFoGoSAE/edit#gid=1439550856
Linked issues
https://github.com/openfoodfacts/smooth-app/issues/2742 We pinpointed this with @g123k. It's probably not the whole story, but might be contributing to it
We might want to explore an isolate as well since people have been complaining about the app freezing
2381
We might want to explore the new camerax plugin to benefit from whatever little compat gains they did. There's a new plugin in town that might be worth exploring
2746
A product added, then removed, can not be scanned anymore.