Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[camera] Fixed a crash when streaming on iOS #4520

Merged
fluttergithubbot merged 37 commits into from
Mar 29, 2022
Merged

[camera] Fixed a crash when streaming on iOS #4520

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
29beaa7
Fixed crash when streaming in iOS
zuvola Nov 18, 2021
aec0f93
Fixed skip judgment
zuvola Nov 18, 2021
0d9f61a
Merge branch 'flutter:master' into fix_ios_streaming
zuvola Nov 22, 2021
7cff775
Add test
zuvola Nov 22, 2021
5d1fb56
Update CHANGELOG.md
zuvola Nov 22, 2021
7c0fa9e
Fix test
zuvola Nov 22, 2021
3056e17
Add frameStack property to startImageStream
zuvola Dec 10, 2021
1a48ffc
Merge branch 'flutter:master' into fix_ios_streaming
zuvola Dec 10, 2021
4cb5d6e
Merge branch 'fix_ios_streaming' of https://github.com/zuvola/plugins…
zuvola Dec 10, 2021
27957a1
Bump version to 0.9.4+6
zuvola Dec 10, 2021
97d16c7
format
zuvola Dec 10, 2021
1382d62
Fixed tests
zuvola Dec 14, 2021
46d9df0
Merge branch 'main' into fix_ios_streaming
zuvola Jan 27, 2022
84dc53d
Restored the API and set the number of pending frames to 4
zuvola Jan 28, 2022
3ed1e94
Fixed tests
zuvola Feb 7, 2022
9a3e627
Merge commit 'f7138f7222856623796c2ee2e561f866fcf08d38' into fix_ios_…
zuvola Feb 7, 2022
6191e3a
Merge
zuvola Feb 7, 2022
e732d7b
Fixed CHANGELOG
zuvola Feb 7, 2022
b69ea0f
Fixed tests
zuvola Feb 7, 2022
e9beb1f
Merge branch 'main' into fix_ios_streaming
zuvola Mar 8, 2022
74d9d85
Apply PR feedback
zuvola Mar 10, 2022
6b81e1a
Update messages
zuvola Mar 11, 2022
96f8edb
Merge branch 'main' into fix_ios_streaming
zuvola Mar 11, 2022
1167112
Update to use CameraTestUtils
zuvola Mar 11, 2022
24871c6
Update _Test header
zuvola Mar 11, 2022
7635469
Bump version to 0.9.4+17
zuvola Mar 11, 2022
376bbfa
Inject FLTImageStreamHandler object
zuvola Mar 16, 2022
f276bd0
Capital S
zuvola Mar 16, 2022
f69aed6
Use XCTNSPredicateExpectation
zuvola Mar 16, 2022
9653345
Wait 1 second
zuvola Mar 16, 2022
661cf49
Merge branch 'main' into fix_ios_streaming
zuvola Mar 16, 2022
8e334bf
Format
zuvola Mar 16, 2022
7b8d74f
Using KVO to wait
zuvola Mar 16, 2022
ab233fa
Add comments
zuvola Mar 23, 2022
830d5ad
Update comments
zuvola Mar 24, 2022
095faf3
Merge branch 'main' into fix_ios_streaming
stuartmorgan-g Mar 25, 2022
e185988
Version bump
stuartmorgan-g Mar 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/camera/camera/ios/Classes/FLTCam.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ NS_ASSUME_NONNULL_BEGIN
- (void)setExposureModeWithResult:(FLTThreadSafeFlutterResult *)result mode:(NSString *)modeStr;
- (void)setFocusModeWithResult:(FLTThreadSafeFlutterResult *)result mode:(NSString *)modeStr;
- (void)applyFocusMode;

/// Sets that a streaming image has been received.
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Mar 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Sets" implies that there's a boolean toggle here, which isn't the case. And the last sentence of this comment isn't relevant to clients using the method.

I wold replace this whole declaration comment with something like:

/**
 * Acknowledges the receipt of one image stream frame.
 *
 * This should be called each time a frame is received. Failing to call it may
 * cause later frames to be dropped instead of streamed.
 */

(This file uses /** ... */ style. Ideally we'd be consistent within the whole project, but we should at least have file-level consistency.)

Copy link
Contributor Author

@zuvola zuvola Mar 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I learned a lot. Thank you.
I ended up using your suggestion in its entirety.

/// Call it every time a streaming image is received.
/// To prevent memory consumption, the number of frames pending processing is limited.
- (void)receivedImageStreamData;
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Mar 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a declaration comment, per style guide.

(I know almost nothing else in this header current has one, but existing style violations aren't a reason to perpetuate new style violations.)

Copy link
Contributor Author

@zuvola zuvola Mar 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.
But, my English isn't very good, so please point out any oddities.


/**
Expand Down
5 changes: 5 additions & 0 deletions packages/camera/camera/ios/Classes/FLTCam.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,13 @@ @interface FLTCam () <AVCaptureVideoDataOutputSampleBufferDelegate,
@property(assign, nonatomic) BOOL videoIsDisconnected;
@property(assign, nonatomic) BOOL audioIsDisconnected;
@property(assign, nonatomic) BOOL isAudioSetup;

/// Number of frames currently pending processing.
@property(assign, nonatomic) int streamingPendingFramesCount;

/// Maximum number of frames pending processing.
@property(assign, nonatomic) int maxStreamingPendingFramesCount;

@property(assign, nonatomic) UIDeviceOrientation lockedCaptureOrientation;
@property(assign, nonatomic) CMTime lastVideoSampleTime;
@property(assign, nonatomic) CMTime lastAudioSampleTime;
Expand Down
9 changes: 6 additions & 3 deletions packages/camera/camera/ios/Classes/FLTCam_Test.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@
#import "FLTSavePhotoDelegate.h"

@interface FLTImageStreamHandler : NSObject <FlutterStreamHandler>
// The queue on which `eventSink` property should be accessed

/// The queue on which `eventSink` property should be accessed
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Mar 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: missing period.

@property(nonatomic, strong) dispatch_queue_t captureSessionQueue;
// `eventSink` property should be accessed on `captureSessionQueue`.
// The block itself should be invoked on the main queue.

/// `eventSink` property should be accessed on `captureSessionQueue`.
/// The block itself should be invoked on the main queue.
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Mar 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A declaration comment should start wit a description of what something is, before getting into how to use it. So:

/// The event sink to stream camera events to Dart.
///
/// The property should only be accessed on `captureSessionQueue`.
/// The block itself should be invoked on the main queue.

@property FlutterEventSink eventSink;

@end

// APIs exposed for unit testing.
Expand Down