Skip to content

Commit fe53da2

Browse files
[video_player] Separate event stream from player on iOS (#9700)
Abstracts the details of the native->Dart communication system from the player instance by adding a listener protocol, and moving the event channel into an implementation of that protocol. Also simplifies the logic to avoid messaging races during channel setup by adding a generic queue to the listener implementation instead of requiring the player to track whether or not it's allowed to send messages, and hard-coding specific messages to re-send later. Part of flutter/flutter#172763 ## Pre-Review Checklist **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
1 parent db6988d commit fe53da2

File tree

11 files changed

+338
-164
lines changed

11 files changed

+338
-164
lines changed

packages/video_player/video_player_avfoundation/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
## 2.8.2
2+
3+
* Restructure internals of Dart notification of video player events.
4+
15
## 2.8.1
26

37
* Restructures internal logic to move more code to Dart.

packages/video_player/video_player_avfoundation/darwin/RunnerTests/VideoPlayerTests.m

Lines changed: 117 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#import <OCMock/OCMock.h>
1010
#import <video_player_avfoundation/AVAssetTrackUtils.h>
11+
#import <video_player_avfoundation/FVPEventBridge.h>
1112
#import <video_player_avfoundation/FVPNativeVideoViewFactory.h>
1213
#import <video_player_avfoundation/FVPTextureBasedVideoPlayer_Test.h>
1314
#import <video_player_avfoundation/FVPVideoPlayerPlugin_Test.h>
@@ -166,6 +167,55 @@ - (instancetype)init {
166167

167168
#pragma mark -
168169

170+
@interface StubEventListener : NSObject <FVPVideoEventListener>
171+
172+
@property(nonatomic) XCTestExpectation *initializationExpectation;
173+
@property(nonatomic) int64_t initializationDuration;
174+
@property(nonatomic) CGSize initializationSize;
175+
176+
- (instancetype)initWithInitializationExpectation:(XCTestExpectation *)expectation;
177+
178+
@end
179+
180+
@implementation StubEventListener
181+
182+
- (instancetype)initWithInitializationExpectation:(XCTestExpectation *)expectation {
183+
self = [super init];
184+
_initializationExpectation = expectation;
185+
return self;
186+
}
187+
188+
- (void)videoPlayerDidComplete {
189+
}
190+
191+
- (void)videoPlayerDidEndBuffering {
192+
}
193+
194+
- (void)videoPlayerDidErrorWithMessage:(NSString *)errorMessage {
195+
}
196+
197+
- (void)videoPlayerDidInitializeWithDuration:(int64_t)duration size:(CGSize)size {
198+
[self.initializationExpectation fulfill];
199+
self.initializationDuration = duration;
200+
self.initializationSize = size;
201+
}
202+
203+
- (void)videoPlayerDidSetPlaying:(BOOL)playing {
204+
}
205+
206+
- (void)videoPlayerDidStartBuffering {
207+
}
208+
209+
- (void)videoPlayerDidUpdateBufferRegions:(NSArray<NSArray<NSNumber *> *> *)regions {
210+
}
211+
212+
- (void)videoPlayerWasDisposed {
213+
}
214+
215+
@end
216+
217+
#pragma mark -
218+
169219
@implementation VideoPlayerTests
170220

171221
- (void)testBlankVideoBugWithEncryptedVideoStreamAndInvertedAspectRatioBugForSomeVideoStream {
@@ -220,9 +270,9 @@ - (void)testPlayerForPlatformViewDoesNotRegisterTexture {
220270
viewProvider:[[StubViewProvider alloc] initWithView:nil]
221271
registrar:registrar];
222272

223-
FlutterError *initalizationError;
224-
[videoPlayerPlugin initialize:&initalizationError];
225-
XCTAssertNil(initalizationError);
273+
FlutterError *initializationError;
274+
[videoPlayerPlugin initialize:&initializationError];
275+
XCTAssertNil(initializationError);
226276
FVPCreationOptions *create = [FVPCreationOptions
227277
makeWithUri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/hls/bee.m3u8"
228278
httpHeaders:@{}
@@ -248,9 +298,9 @@ - (void)testSeekToWhilePausedStartsDisplayLinkTemporarily {
248298
viewProvider:[[StubViewProvider alloc] initWithView:nil]
249299
registrar:registrar];
250300

251-
FlutterError *initalizationError;
252-
[videoPlayerPlugin initialize:&initalizationError];
253-
XCTAssertNil(initalizationError);
301+
FlutterError *initializationError;
302+
[videoPlayerPlugin initialize:&initializationError];
303+
XCTAssertNil(initializationError);
254304
FVPCreationOptions *create = [FVPCreationOptions
255305
makeWithUri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/hls/bee.m3u8"
256306
httpHeaders:@{}
@@ -305,9 +355,9 @@ - (void)testInitStartsDisplayLinkTemporarily {
305355
viewProvider:[[StubViewProvider alloc] initWithView:nil]
306356
registrar:registrar];
307357

308-
FlutterError *initalizationError;
309-
[videoPlayerPlugin initialize:&initalizationError];
310-
XCTAssertNil(initalizationError);
358+
FlutterError *initializationError;
359+
[videoPlayerPlugin initialize:&initializationError];
360+
XCTAssertNil(initializationError);
311361
FVPCreationOptions *create = [FVPCreationOptions
312362
makeWithUri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/hls/bee.m3u8"
313363
httpHeaders:@{}
@@ -351,9 +401,9 @@ - (void)testSeekToWhilePlayingDoesNotStopDisplayLink {
351401
viewProvider:[[StubViewProvider alloc] initWithView:nil]
352402
registrar:registrar];
353403

354-
FlutterError *initalizationError;
355-
[videoPlayerPlugin initialize:&initalizationError];
356-
XCTAssertNil(initalizationError);
404+
FlutterError *initializationError;
405+
[videoPlayerPlugin initialize:&initializationError];
406+
XCTAssertNil(initializationError);
357407
FVPCreationOptions *create = [FVPCreationOptions
358408
makeWithUri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/hls/bee.m3u8"
359409
httpHeaders:@{}
@@ -406,9 +456,9 @@ - (void)testPauseWhileWaitingForFrameDoesNotStopDisplayLink {
406456
viewProvider:[[StubViewProvider alloc] initWithView:nil]
407457
registrar:registrar];
408458

409-
FlutterError *initalizationError;
410-
[videoPlayerPlugin initialize:&initalizationError];
411-
XCTAssertNil(initalizationError);
459+
FlutterError *initializationError;
460+
[videoPlayerPlugin initialize:&initializationError];
461+
XCTAssertNil(initializationError);
412462
FVPCreationOptions *create = [FVPCreationOptions
413463
makeWithUri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/hls/bee.m3u8"
414464
httpHeaders:@{}
@@ -477,16 +527,19 @@ - (void)testBufferingStateFromPlayer {
477527
AVPlayer *avPlayer = player.player;
478528
[avPlayer play];
479529

480-
[player onListenWithArguments:nil
481-
eventSink:^(NSDictionary<NSString *, id> *event) {
482-
if ([event[@"event"] isEqualToString:@"bufferingEnd"]) {
483-
XCTAssertTrue(avPlayer.currentItem.isPlaybackLikelyToKeepUp);
484-
}
485-
486-
if ([event[@"event"] isEqualToString:@"bufferingStart"]) {
487-
XCTAssertFalse(avPlayer.currentItem.isPlaybackLikelyToKeepUp);
488-
}
489-
}];
530+
// TODO(stuartmorgan): Update this test to instead use a mock listener, and add separate unit
531+
// tests of FVPEventBridge.
532+
[(NSObject<FlutterStreamHandler> *)player.eventListener
533+
onListenWithArguments:nil
534+
eventSink:^(NSDictionary<NSString *, id> *event) {
535+
if ([event[@"event"] isEqualToString:@"bufferingEnd"]) {
536+
XCTAssertTrue(avPlayer.currentItem.isPlaybackLikelyToKeepUp);
537+
}
538+
539+
if ([event[@"event"] isEqualToString:@"bufferingStart"]) {
540+
XCTAssertFalse(avPlayer.currentItem.isPlaybackLikelyToKeepUp);
541+
}
542+
}];
490543
XCTestExpectation *bufferingStateExpectation =
491544
[self expectationWithDescription:@"bufferingState"];
492545
NSTimeInterval timeout = 10;
@@ -498,39 +551,39 @@ - (void)testBufferingStateFromPlayer {
498551
}
499552

500553
- (void)testVideoControls {
501-
NSDictionary<NSString *, id> *videoInitialization =
554+
StubEventListener *eventListener =
502555
[self sanityTestURI:@"https://flutter.github.io/assets-for-api-docs/assets/videos/bee.mp4"];
503-
XCTAssertEqualObjects(videoInitialization[@"height"], @720);
504-
XCTAssertEqualObjects(videoInitialization[@"width"], @1280);
505-
XCTAssertEqualWithAccuracy([videoInitialization[@"duration"] intValue], 4000, 200);
556+
XCTAssertEqual(eventListener.initializationSize.height, 720);
557+
XCTAssertEqual(eventListener.initializationSize.width, 1280);
558+
XCTAssertEqualWithAccuracy(eventListener.initializationDuration, 4000, 200);
506559
}
507560

508561
- (void)testAudioControls {
509-
NSDictionary<NSString *, id> *audioInitialization = [self
562+
StubEventListener *eventListener = [self
510563
sanityTestURI:@"https://flutter.github.io/assets-for-api-docs/assets/audio/rooster.mp3"];
511-
XCTAssertEqualObjects(audioInitialization[@"height"], @0);
512-
XCTAssertEqualObjects(audioInitialization[@"width"], @0);
564+
XCTAssertEqual(eventListener.initializationSize.height, 0);
565+
XCTAssertEqual(eventListener.initializationSize.width, 0);
513566
// Perfect precision not guaranteed.
514-
XCTAssertEqualWithAccuracy([audioInitialization[@"duration"] intValue], 5400, 200);
567+
XCTAssertEqualWithAccuracy(eventListener.initializationDuration, 5400, 200);
515568
}
516569

517570
- (void)testHLSControls {
518-
NSDictionary<NSString *, id> *videoInitialization = [self
571+
StubEventListener *eventListener = [self
519572
sanityTestURI:@"https://flutter.github.io/assets-for-api-docs/assets/videos/hls/bee.m3u8"];
520-
XCTAssertEqualObjects(videoInitialization[@"height"], @720);
521-
XCTAssertEqualObjects(videoInitialization[@"width"], @1280);
522-
XCTAssertEqualWithAccuracy([videoInitialization[@"duration"] intValue], 4000, 200);
573+
XCTAssertEqual(eventListener.initializationSize.height, 720);
574+
XCTAssertEqual(eventListener.initializationSize.width, 1280);
575+
XCTAssertEqualWithAccuracy(eventListener.initializationDuration, 4000, 200);
523576
}
524577

525578
- (void)testAudioOnlyHLSControls {
526579
XCTSkip(@"Flaky; see https://github.com/flutter/flutter/issues/164381");
527580

528-
NSDictionary<NSString *, id> *videoInitialization =
581+
StubEventListener *eventListener =
529582
[self sanityTestURI:@"https://flutter.github.io/assets-for-api-docs/assets/videos/hls/"
530583
@"bee_audio_only.m3u8"];
531-
XCTAssertEqualObjects(videoInitialization[@"height"], @0);
532-
XCTAssertEqualObjects(videoInitialization[@"width"], @0);
533-
XCTAssertEqualWithAccuracy([videoInitialization[@"duration"] intValue], 4000, 200);
584+
XCTAssertEqual(eventListener.initializationSize.height, 0);
585+
XCTAssertEqual(eventListener.initializationSize.width, 0);
586+
XCTAssertEqualWithAccuracy(eventListener.initializationDuration, 4000, 200);
534587
}
535588

536589
#if TARGET_OS_IOS
@@ -555,6 +608,8 @@ - (void)testSeekToleranceWhenNotSeekingToEnd {
555608
httpHeaders:@{}
556609
avFactory:stubAVFactory
557610
viewProvider:[[StubViewProvider alloc] initWithView:nil]];
611+
NSObject<FVPVideoEventListener> *listener = OCMProtocolMock(@protocol(FVPVideoEventListener));
612+
player.eventListener = listener;
558613

559614
XCTestExpectation *seekExpectation =
560615
[self expectationWithDescription:@"seekTo has zero tolerance when seeking not to end"];
@@ -577,6 +632,8 @@ - (void)testSeekToleranceWhenSeekingToEnd {
577632
httpHeaders:@{}
578633
avFactory:stubAVFactory
579634
viewProvider:[[StubViewProvider alloc] initWithView:nil]];
635+
NSObject<FVPVideoEventListener> *listener = OCMProtocolMock(@protocol(FVPVideoEventListener));
636+
player.eventListener = listener;
580637

581638
XCTestExpectation *seekExpectation =
582639
[self expectationWithDescription:@"seekTo has non-zero tolerance when seeking to end"];
@@ -592,7 +649,9 @@ - (void)testSeekToleranceWhenSeekingToEnd {
592649

593650
/// Sanity checks a video player playing the given URL with the actual AVPlayer. This is essentially
594651
/// a mini integration test of the player component.
595-
- (NSDictionary<NSString *, id> *)sanityTestURI:(NSString *)testURI {
652+
///
653+
/// Returns the stub event listener to allow tests to inspect the call state.
654+
- (StubEventListener *)sanityTestURI:(NSString *)testURI {
596655
NSURL *testURL = [NSURL URLWithString:testURI];
597656
XCTAssertNotNil(testURL);
598657
FVPVideoPlayer *player =
@@ -603,15 +662,9 @@ - (void)testSeekToleranceWhenSeekingToEnd {
603662
XCTAssertNotNil(player);
604663

605664
XCTestExpectation *initializedExpectation = [self expectationWithDescription:@"initialized"];
606-
__block NSDictionary<NSString *, id> *initializationEvent;
607-
[player onListenWithArguments:nil
608-
eventSink:^(NSDictionary<NSString *, id> *event) {
609-
if ([event[@"event"] isEqualToString:@"initialized"]) {
610-
initializationEvent = event;
611-
XCTAssertEqual(event.count, 4);
612-
[initializedExpectation fulfill];
613-
}
614-
}];
665+
StubEventListener *listener =
666+
[[StubEventListener alloc] initWithInitializationExpectation:initializedExpectation];
667+
player.eventListener = listener;
615668
[self waitForExpectationsWithTimeout:30.0 handler:nil];
616669

617670
// Starts paused.
@@ -634,9 +687,7 @@ - (void)testSeekToleranceWhenSeekingToEnd {
634687
XCTAssertNil(error);
635688
XCTAssertEqual(avPlayer.volume, 0.1f);
636689

637-
[player onCancelWithArguments:nil];
638-
639-
return initializationEvent;
690+
return listener;
640691
}
641692

642693
// Checks whether [AVPlayer rate] KVO observations are correctly detached.
@@ -787,12 +838,15 @@ - (void)testFailedToLoadVideoEventShouldBeAlwaysSent {
787838
[self waitForExpectationsWithTimeout:10.0 handler:nil];
788839

789840
XCTestExpectation *failedExpectation = [self expectationWithDescription:@"failed"];
790-
[player onListenWithArguments:nil
791-
eventSink:^(FlutterError *event) {
792-
if ([event isKindOfClass:FlutterError.class]) {
793-
[failedExpectation fulfill];
794-
}
795-
}];
841+
// TODO(stuartmorgan): Update this test to instead use a mock listener, and add separate unit
842+
// tests of FVPEventBridge.
843+
[(NSObject<FlutterStreamHandler> *)player.eventListener
844+
onListenWithArguments:nil
845+
eventSink:^(FlutterError *event) {
846+
if ([event isKindOfClass:FlutterError.class]) {
847+
[failedExpectation fulfill];
848+
}
849+
}];
796850
[self waitForExpectationsWithTimeout:10.0 handler:nil];
797851
}
798852

@@ -804,12 +858,9 @@ - (void)testUpdatePlayingStateShouldNotResetRate {
804858
viewProvider:[[StubViewProvider alloc] initWithView:nil]];
805859

806860
XCTestExpectation *initializedExpectation = [self expectationWithDescription:@"initialized"];
807-
[player onListenWithArguments:nil
808-
eventSink:^(NSDictionary<NSString *, id> *event) {
809-
if ([event[@"event"] isEqualToString:@"initialized"]) {
810-
[initializedExpectation fulfill];
811-
}
812-
}];
861+
StubEventListener *listener =
862+
[[StubEventListener alloc] initWithInitializationExpectation:initializedExpectation];
863+
player.eventListener = listener;
813864
[self waitForExpectationsWithTimeout:10 handler:nil];
814865

815866
FlutterError *error;

0 commit comments

Comments
 (0)