[camera] Fixed a crash when streaming on iOS #4520
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| // Copyright 2013 The Flutter Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| @import camera; | ||
| @import camera.Test; | ||
| @import XCTest; | ||
| @import AVFoundation; | ||
| #import <OCMock/OCMock.h> | ||
| #import "CameraTestUtils.h" | ||
|
|
||
| @interface StreamingTests : XCTestCase | ||
| @property(readonly, nonatomic) FLTCam *camera; | ||
| @property(readonly, nonatomic) CMSampleBufferRef sampleBuffer; | ||
| @end | ||
|
|
||
| @implementation StreamingTests | ||
|
|
||
| - (void)setUp { | ||
| dispatch_queue_t captureSessionQueue = dispatch_queue_create("testing", NULL); | ||
| _camera = FLTCreateCamWithCaptureSessionQueue(captureSessionQueue); | ||
| _sampleBuffer = FLTCreateTestSampleBuffer(); | ||
| } | ||
|
|
||
| - (void)tearDown { | ||
| CFRelease(_sampleBuffer); | ||
| } | ||
|
|
||
| - (void)testExceedMaxStreamingPendingFramesCount { | ||
| XCTestExpectation *streamingExpectation = [self | ||
| expectationWithDescription:@"Must not call handler over maxStreamingPendingFramesCount"]; | ||
|
|
||
| id handlerMock = OCMClassMock([FLTImageStreamHandler class]); | ||
| OCMStub([handlerMock eventSink]).andReturn(^(id event) { | ||
| [streamingExpectation fulfill]; | ||
| }); | ||
|
|
||
| id messenger = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); | ||
| [_camera startImageStreamWithMessenger:messenger imageStreamHandler:handlerMock]; | ||
|
|
||
| XCTKVOExpectation *expectation = [[XCTKVOExpectation alloc] initWithKeyPath:@"isStreamingImages" | ||
| object:_camera | ||
| expectedValue:@YES]; | ||
| XCTWaiterResult result = [XCTWaiter waitForExpectations:@[ expectation ] timeout:1]; | ||
| XCTAssertEqual(result, XCTWaiterResultCompleted); | ||
|
|
||
| streamingExpectation.expectedFulfillmentCount = 4; | ||
| for (int i = 0; i < 10; i++) { | ||
| [_camera captureOutput:nil didOutputSampleBuffer:self.sampleBuffer fromConnection:nil]; | ||
| } | ||
|
|
||
| [self waitForExpectationsWithTimeout:1.0 handler:nil]; | ||
| } | ||
|
|
||
| - (void)testReceivedImageStreamData { | ||
| XCTestExpectation *streamingExpectation = | ||
| [self expectationWithDescription: | ||
| @"Must be able to call the handler again when receivedImageStreamData is called"]; | ||
|
|
||
| id handlerMock = OCMClassMock([FLTImageStreamHandler class]); | ||
| OCMStub([handlerMock eventSink]).andReturn(^(id event) { | ||
| [streamingExpectation fulfill]; | ||
| }); | ||
|
|
||
| id messenger = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); | ||
| [_camera startImageStreamWithMessenger:messenger imageStreamHandler:handlerMock]; | ||
|
|
||
| XCTKVOExpectation *expectation = [[XCTKVOExpectation alloc] initWithKeyPath:@"isStreamingImages" | ||
| object:_camera | ||
| expectedValue:@YES]; | ||
| XCTWaiterResult result = [XCTWaiter waitForExpectations:@[ expectation ] timeout:1]; | ||
| XCTAssertEqual(result, XCTWaiterResultCompleted); | ||
|
|
||
| streamingExpectation.expectedFulfillmentCount = 5; | ||
| for (int i = 0; i < 10; i++) { | ||
| [_camera captureOutput:nil didOutputSampleBuffer:self.sampleBuffer fromConnection:nil]; | ||
| } | ||
|
|
||
| [_camera receivedImageStreamData]; | ||
| [_camera captureOutput:nil didOutputSampleBuffer:self.sampleBuffer fromConnection:nil]; | ||
|
|
||
| [self waitForExpectationsWithTimeout:1.0 handler:nil]; | ||
| } | ||
|
|
||
| @end | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -448,6 +448,13 @@ class CameraController extends ValueNotifier<CameraValue> { | |||||||||||||||||||||||||||||||||
| _imageStreamSubscription = | ||||||||||||||||||||||||||||||||||
| cameraEventChannel.receiveBroadcastStream().listen( | ||||||||||||||||||||||||||||||||||
| (dynamic imageData) { | ||||||||||||||||||||||||||||||||||
| if (defaultTargetPlatform == TargetPlatform.iOS) { | ||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||
| _channel.invokeMethod<void>('receivedImageStreamData'); | ||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. 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. There was a problem hiding this comment. 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. There was a problem hiding this comment. 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) { | ||||||||||||||||||||||||||||||||||
| throw CameraException(e.code, e.message); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| onAvailable( | ||||||||||||||||||||||||||||||||||
| CameraImage.fromPlatformData(imageData as Map<dynamic, dynamic>)); | ||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice to see these new test helpers being used :)