-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[camera] Fixed a crash when streaming on iOS #4520
Changes from 1 commit
29beaa7
aec0f93
0d9f61a
7cff775
5d1fb56
7c0fa9e
3056e17
1a48ffc
4cb5d6e
27957a1
97d16c7
1382d62
46d9df0
84dc53d
3ed1e94
9a3e627
6191e3a
e732d7b
b69ea0f
e9beb1f
74d9d85
6b81e1a
96f8edb
1167112
24871c6
7635469
376bbfa
f276bd0
f69aed6
9653345
661cf49
8e334bf
7b8d74f
ab233fa
830d5ad
095faf3
e185988
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -403,14 +403,20 @@ class CameraController extends ValueNotifier<CameraValue> { | |||||||||||||||||||||||||||||||||
| /// have significant frame rate drops for [CameraPreview] on lower end | ||||||||||||||||||||||||||||||||||
| /// devices. | ||||||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||||||
| /// On older iOS devices, frames pending processing can exhaust the memory | ||||||||||||||||||||||||||||||||||
| /// and cause crashes. In this case, you can use [frameStack] to limit | ||||||||||||||||||||||||||||||||||
| /// the maximum number of frames pending processing. The default value is 0, | ||||||||||||||||||||||||||||||||||
| /// which means there is no limit. | ||||||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||||||
| /// Throws a [CameraException] if image streaming or video recording has | ||||||||||||||||||||||||||||||||||
| /// already started. | ||||||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||||||
| /// The `startImageStream` method is only available on Android and iOS (other | ||||||||||||||||||||||||||||||||||
| /// platforms won't be supported in current setup). | ||||||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||||||
| // TODO(bmparr): Add settings for resolution and fps. | ||||||||||||||||||||||||||||||||||
| Future<void> startImageStream(onLatestImageAvailable onAvailable) async { | ||||||||||||||||||||||||||||||||||
| Future<void> startImageStream(onLatestImageAvailable onAvailable, | ||||||||||||||||||||||||||||||||||
| {int frameStack = 0}) async { | ||||||||||||||||||||||||||||||||||
| assert(defaultTargetPlatform == TargetPlatform.android || | ||||||||||||||||||||||||||||||||||
| defaultTargetPlatform == TargetPlatform.iOS); | ||||||||||||||||||||||||||||||||||
| _throwIfNotInitialized("startImageStream"); | ||||||||||||||||||||||||||||||||||
|
|
@@ -428,7 +434,8 @@ class CameraController extends ValueNotifier<CameraValue> { | |||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||
| await _channel.invokeMethod<void>('startImageStream'); | ||||||||||||||||||||||||||||||||||
| await _channel | ||||||||||||||||||||||||||||||||||
| .invokeMethod<void>('startImageStream', {'frameStack': frameStack}); | ||||||||||||||||||||||||||||||||||
| value = value.copyWith(isStreamingImages: true); | ||||||||||||||||||||||||||||||||||
| } on PlatformException catch (e) { | ||||||||||||||||||||||||||||||||||
| throw CameraException(e.code, e.message); | ||||||||||||||||||||||||||||||||||
|
|
@@ -438,7 +445,7 @@ class CameraController extends ValueNotifier<CameraValue> { | |||||||||||||||||||||||||||||||||
| _imageStreamSubscription = | ||||||||||||||||||||||||||||||||||
| cameraEventChannel.receiveBroadcastStream().listen( | ||||||||||||||||||||||||||||||||||
| (dynamic imageData) { | ||||||||||||||||||||||||||||||||||
| if (defaultTargetPlatform == TargetPlatform.iOS) { | ||||||||||||||||||||||||||||||||||
| if (defaultTargetPlatform == TargetPlatform.iOS && frameStack > 0) { | ||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||
| _channel.invokeMethod<void>('receivedImageStreamData'); | ||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given that this is asynchronous in both directions I would expect this could cause significant frame loss even on a reasonably powerful device. How much investigation have you done into specifically what the parameters of the crash are? Is it really necessary to have a maximum of a single frame in flight? It seems likely that this needs a more nuanced approach (maybe a hard limit still but higher than one, maybe something that's tunable by plugin clients). It's hard to determine the right fix without more details about what the failure condition actually is.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I modified the plugin to allow you to set a limit on the maximum number of frames pending processing.
I don't have that many devices, but according to flutter/flutter#44436, it seems to be crashing on iPhone 6, 7, 8, etc. These devices still have a large market share that cannot be ignored. Also, for image analysis processes such as OCR, low or medium quality is not always satisfactory.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for this testing. Given this table, I'm fine with starting from setting a fixed value—4 looks like a good place to start—with an explanation in the code of where that number came from, and then if it turns out that causes issues in some cases we can add an API to tune it later. |
||||||||||||||||||||||||||||||||||
| } on PlatformException catch (e) { | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.