[video_player_avfoundation, camera_avfoundation] never overwrite but only upgrade audio session category #7143
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -184,6 +184,8 @@ - (instancetype)initWithMediaSettings:(FCPPlatformMediaSettings *)mediaSettings | |
| _videoFormat = kCVPixelFormatType_32BGRA; | ||
| _inProgressSavePhotoDelegates = [NSMutableDictionary dictionary]; | ||
| _fileFormat = FCPPlatformImageFileFormatJpeg; | ||
| _videoCaptureSession.automaticallyConfiguresApplicationAudioSession = NO; | ||
| _audioCaptureSession.automaticallyConfiguresApplicationAudioSession = NO; | ||
stuartmorgan-g marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // To limit memory consumption, limit the number of frames pending processing. | ||
| // After some testing, 4 was determined to be the best maximum value. | ||
|
|
@@ -673,7 +675,8 @@ - (void)captureOutput:(AVCaptureOutput *)output | |
| if (_isFirstVideoSample) { | ||
| [_videoWriter startSessionAtSourceTime:currentSampleTime]; | ||
| // fix sample times not being numeric when pause/resume happens before first sample buffer | ||
| // arrives | ||
| // https://github.com/flutter/flutter/issues/132014 | ||
| _lastVideoSampleTime = currentSampleTime; | ||
| _lastAudioSampleTime = currentSampleTime; | ||
| _isFirstVideoSample = NO; | ||
|
|
@@ -1231,9 +1234,7 @@ - (BOOL)setupWriterForPath:(NSString *)path { | |
| return NO; | ||
| } | ||
|
|
||
| [self setUpCaptureSessionForAudioIfNeeded]; | ||
|
|
||
| _videoWriter = [[AVAssetWriter alloc] initWithURL:outputURL | ||
| fileType:AVFileTypeMPEG4 | ||
|
|
@@ -1313,9 +1314,42 @@ - (BOOL)setupWriterForPath:(NSString *)path { | |
| return YES; | ||
| } | ||
|
|
||
| // this same function is also in video_player_avfoundation | ||
|
||
| // configure application wide audio session manually to prevent overwriting | ||
| // flag MixWithOthers by capture session, only change category if it is considered | ||
| // as upgrade which means it can only enable ability to play in silent mode or | ||
|
||
| // ability to record audio but never disables it, that could affect other plugins | ||
|
||
| // which depend on this global state, only change category or options if there is | ||
| // change to prevent unnecessary lags and silence | ||
|
||
| // https://github.com/flutter/flutter/issues/131553 | ||
|
||
| static void upgradeAudioSessionCategory(AVAudioSessionCategory requestedCategory, | ||
| AVAudioSessionCategoryOptions options) { | ||
| NSSet *playCategories = [NSSet | ||
| setWithObjects:AVAudioSessionCategoryPlayback, AVAudioSessionCategoryPlayAndRecord, nil]; | ||
| NSSet *recordCategories = | ||
| [NSSet setWithObjects:AVAudioSessionCategoryRecord, AVAudioSessionCategoryPlayAndRecord, nil]; | ||
| NSSet *requiredCategories = | ||
| [NSSet setWithObjects:requestedCategory, AVAudioSession.sharedInstance.category, nil]; | ||
| BOOL needPlay = [requiredCategories intersectsSet:playCategories]; | ||
|
||
| BOOL needRecord = [requiredCategories intersectsSet:recordCategories]; | ||
| if (needPlay && needRecord) { | ||
| requestedCategory = AVAudioSessionCategoryPlayAndRecord; | ||
| } else if (needPlay) { | ||
| requestedCategory = AVAudioSessionCategoryPlayback; | ||
| } else if (needRecord) { | ||
| requestedCategory = AVAudioSessionCategoryRecord; | ||
| } | ||
| options = AVAudioSession.sharedInstance.categoryOptions | options; | ||
| if ([requestedCategory isEqualToString:AVAudioSession.sharedInstance.category] && | ||
| options == AVAudioSession.sharedInstance.categoryOptions) { | ||
| return; | ||
| } | ||
| [AVAudioSession.sharedInstance setCategory:requestedCategory withOptions:options error:nil]; | ||
| } | ||
|
|
||
| - (void)setUpCaptureSessionForAudioIfNeeded { | ||
| // Don't setup audio twice or we will lose the audio. | ||
| if (!_mediaSettings.enableAudio || _isAudioSetup) { | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -1331,6 +1365,19 @@ - (void)setUpCaptureSessionForAudio { | |
| // Setup the audio output. | ||
| _audioOutput = [[AVCaptureAudioDataOutput alloc] init]; | ||
|
|
||
| dispatch_block_t block = ^{ | ||
| // Setup options implicit to AVAudioSessionCategoryPlayback to not disturb video_player. | ||
|
||
| upgradeAudioSessionCategory(AVAudioSessionCategoryPlayAndRecord, | ||
| AVAudioSessionCategoryOptionDefaultToSpeaker | | ||
| AVAudioSessionCategoryOptionAllowBluetoothA2DP | | ||
| AVAudioSessionCategoryOptionAllowAirPlay); | ||
| }; | ||
| if (!NSThread.isMainThread) { | ||
| dispatch_sync(dispatch_get_main_queue(), block); | ||
| } else { | ||
| block(); | ||
|
Contributor
There was a problem hiding this comment. is
Contributor
Author
There was a problem hiding this comment. It is called on the main thread in tests if I remember correctly #7143 (comment).
Contributor
There was a problem hiding this comment. is it only called on main thread in tests? Can you give an example of such failure? I'd like to see if it's possible to run on background thread in those tests
Contributor
Author
There was a problem hiding this comment. Try to run tests with this: //if (!NSThread.isMainThread) {
dispatch_sync(dispatch_get_main_queue(), block);
//} else {
//block();
//}And you will get this inside xcode: When running outside of xcode with debugging there will be probably just crash, you can add You will probably need to run
Contributor
There was a problem hiding this comment.
So
Contributor
There was a problem hiding this comment.
Could you explain the problem with FLTEnsureToRunOnMainQueue? Maybe a concrete example?
I don't know why you are comparing with interfaces and factories. Imagine you have this code: Here your production code only calls Logic A, and test code only calls Logic B. Then the test isn't really providing any confidence right? What I suggest is: Then in the test, you can do: This way, the test code actually covers Logic A, providing us with confidence in production.
Contributor
Author
There was a problem hiding this comment. Your assertion will fail because other tests call What I proposed does not have such problems and tests also cover production logic as I wrote.
Contributor
There was a problem hiding this comment.
Oh I overlooked the part that it's
It would be the same problem, that
Contributor
Author
There was a problem hiding this comment.
Oh I did not think of that, but as you wrote that, I realized that means that also the async test should be untestable, maybe it actually waits in some non blocking way. But anyway I did not see that documented anywhere, maybe that async test is not 100% clean then?
Contributor
There was a problem hiding this comment.
|
||
| } | ||
|
|
||
| if ([_audioCaptureSession canAddInput:audioInput]) { | ||
| [_audioCaptureSession addInput:audioInput]; | ||
|
|
||
|
|
||
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.
I don't think it's going to be clear to a client of
camerawhat this means; please add more context about what the actual problem being solved is. (Also, it's fine to givevideo_playeras an example, but it shouldn't make it sound like this is specific tovideo_player; any other native code could collide.)