Skip to content

Commit afd0434

Browse files
[video_player] Move iOS/macOS to per-player-instance Pigeon APIs (flutter#9529)
Rather than having a single API to talk directly to the plugin, and having most of the methods in that API just do a map lookup and dispatch to a player instance, which was necessary when Pigeon didn't support instantiating multiple instances of an API, have each player instance set up an API instance that the Dart code can talk to directly. This follows the pattern used by plugins that migrated to Pigeon more recently (e.g., `google_maps_flutter` has a very similar pattern), reduces boilerplate native code, and moves closer to what an FFI-based implementation would look like if we go that route in the future. Since the Dart unit tests needed to be significantly reworked anyway, this also moves to the pattern we are using in newer plugin code, where we use `mockito` to mock the Pigeon API surface. The "call log" approach it replaces dates back to pre-Pigeon, when only way to test that the right platform calls were made was to intercept and track method channel calls at the framework level. Also updates to the latest version of Pigeon. ## Pre-Review Checklist [^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 60600d3 commit afd0434

File tree

18 files changed

+958
-1160
lines changed

18 files changed

+958
-1160
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.7.3
2+
3+
* Restructures the communication between Dart and native code.
4+
15
## 2.7.2
26

37
* Uses `CADisplayLink` on macOS 14.0+.

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

Lines changed: 83 additions & 161 deletions
Large diffs are not rendered by default.

packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPTextureBasedVideoPlayer.m

Lines changed: 10 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -28,32 +28,28 @@ @interface FVPTextureBasedVideoPlayer ()
2828
// (e.g., after a seek while paused). If YES, the display link should continue to run until the next
2929
// frame is successfully provided.
3030
@property(nonatomic, assign) BOOL waitingForFrame;
31-
@property(nonatomic, copy) void (^onDisposed)(int64_t);
3231
@end
3332

3433
@implementation FVPTextureBasedVideoPlayer
3534
- (instancetype)initWithAsset:(NSString *)asset
3635
frameUpdater:(FVPFrameUpdater *)frameUpdater
3736
displayLink:(NSObject<FVPDisplayLink> *)displayLink
3837
avFactory:(id<FVPAVFactory>)avFactory
39-
viewProvider:(NSObject<FVPViewProvider> *)viewProvider
40-
onDisposed:(void (^)(int64_t))onDisposed {
38+
viewProvider:(NSObject<FVPViewProvider> *)viewProvider {
4139
return [self initWithURL:[NSURL fileURLWithPath:[FVPVideoPlayer absolutePathForAssetName:asset]]
4240
frameUpdater:frameUpdater
4341
displayLink:displayLink
4442
httpHeaders:@{}
4543
avFactory:avFactory
46-
viewProvider:viewProvider
47-
onDisposed:onDisposed];
44+
viewProvider:viewProvider];
4845
}
4946

5047
- (instancetype)initWithURL:(NSURL *)url
5148
frameUpdater:(FVPFrameUpdater *)frameUpdater
5249
displayLink:(NSObject<FVPDisplayLink> *)displayLink
5350
httpHeaders:(nonnull NSDictionary<NSString *, NSString *> *)headers
5451
avFactory:(id<FVPAVFactory>)avFactory
55-
viewProvider:(NSObject<FVPViewProvider> *)viewProvider
56-
onDisposed:(void (^)(int64_t))onDisposed {
52+
viewProvider:(NSObject<FVPViewProvider> *)viewProvider {
5753
NSDictionary<NSString *, id> *options = nil;
5854
if ([headers count] != 0) {
5955
options = @{@"AVURLAssetHTTPHeaderFieldsKey" : headers};
@@ -64,24 +60,21 @@ - (instancetype)initWithURL:(NSURL *)url
6460
frameUpdater:frameUpdater
6561
displayLink:displayLink
6662
avFactory:avFactory
67-
viewProvider:viewProvider
68-
onDisposed:onDisposed];
63+
viewProvider:viewProvider];
6964
}
7065

7166
- (instancetype)initWithPlayerItem:(AVPlayerItem *)item
7267
frameUpdater:(FVPFrameUpdater *)frameUpdater
7368
displayLink:(NSObject<FVPDisplayLink> *)displayLink
7469
avFactory:(id<FVPAVFactory>)avFactory
75-
viewProvider:(NSObject<FVPViewProvider> *)viewProvider
76-
onDisposed:(void (^)(int64_t))onDisposed {
70+
viewProvider:(NSObject<FVPViewProvider> *)viewProvider {
7771
self = [super initWithPlayerItem:item avFactory:avFactory viewProvider:viewProvider];
7872

7973
if (self) {
8074
_frameUpdater = frameUpdater;
8175
_displayLink = displayLink;
8276
_frameUpdater.displayLink = _displayLink;
8377
_selfRefresh = true;
84-
_onDisposed = [onDisposed copy];
8578

8679
// This is to fix 2 bugs: 1. blank video for encrypted video streams on iOS 16
8780
// (https://github.com/flutter/flutter/issues/111457) and 2. swapped width and height for some
@@ -117,10 +110,10 @@ - (void)updatePlayingState {
117110
_displayLink.running = self.isPlaying || self.waitingForFrame;
118111
}
119112

120-
- (void)seekTo:(int64_t)location completionHandler:(void (^)(BOOL))completionHandler {
113+
- (void)seekTo:(NSInteger)position completion:(void (^)(FlutterError *_Nullable))completion {
121114
CMTime previousCMTime = self.player.currentTime;
122-
[super seekTo:location
123-
completionHandler:^(BOOL completed) {
115+
[super seekTo:position
116+
completion:^(FlutterError *error) {
124117
if (CMTimeCompare(self.player.currentTime, previousCMTime) != 0) {
125118
// Ensure that a frame is drawn once available, even if currently paused. In theory a
126119
// race is possible here where the new frame has already drawn by the time this code
@@ -131,8 +124,8 @@ - (void)seekTo:(int64_t)location completionHandler:(void (^)(BOOL))completionHan
131124
[self expectFrame];
132125
}
133126

134-
if (completionHandler) {
135-
completionHandler(completed);
127+
if (completion) {
128+
completion(error);
136129
}
137130
}];
138131
}
@@ -153,12 +146,6 @@ - (void)disposeSansEventChannel {
153146
_displayLink = nil;
154147
}
155148

156-
- (void)dispose {
157-
[super dispose];
158-
159-
_onDisposed(self.frameUpdater.textureIdentifier);
160-
}
161-
162149
#pragma mark - FlutterTexture
163150

164151
- (CVPixelBufferRef)copyPixelBuffer {

packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayer.m

Lines changed: 44 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
#import "./include/video_player_avfoundation/FVPVideoPlayer.h"
66
#import "./include/video_player_avfoundation/FVPVideoPlayer_Internal.h"
7-
#import "./include/video_player_avfoundation/FVPVideoPlayer_Test.h"
87

98
#import <GLKit/GLKit.h>
109

@@ -104,6 +103,25 @@ - (void)dealloc {
104103
}
105104
}
106105

106+
/// This method allows you to dispose without touching the event channel. This
107+
/// is useful for the case where the Engine is in the process of deconstruction
108+
/// so the channel is going to die or is already dead.
109+
- (void)disposeSansEventChannel {
110+
_disposed = YES;
111+
[self removeKeyValueObservers];
112+
113+
[self.player replaceCurrentItemWithPlayerItem:nil];
114+
[[NSNotificationCenter defaultCenter] removeObserver:self];
115+
}
116+
117+
- (void)dispose {
118+
[self disposeSansEventChannel];
119+
if (_onDisposed) {
120+
_onDisposed();
121+
}
122+
[_eventChannel setStreamHandler:nil];
123+
}
124+
107125
+ (NSString *)absolutePathForAssetName:(NSString *)assetName {
108126
NSString *path = [[NSBundle mainBundle] pathForResource:assetName ofType:nil];
109127
#if TARGET_OS_OSX
@@ -405,57 +423,56 @@ - (void)setupEventSinkIfReadyToPlay {
405423
}
406424
}
407425

408-
- (void)play {
426+
#pragma mark - FVPVideoPlayerInstanceApi
427+
428+
- (void)playWithError:(FlutterError *_Nullable *_Nonnull)error {
409429
_isPlaying = YES;
410430
[self updatePlayingState];
411431
}
412432

413-
- (void)pause {
433+
- (void)pauseWithError:(FlutterError *_Nullable *_Nonnull)error {
414434
_isPlaying = NO;
415435
[self updatePlayingState];
416436
}
417437

418-
- (int64_t)position {
419-
return FVPCMTimeToMillis([_player currentTime]);
420-
}
421-
422-
- (int64_t)duration {
423-
// Note: https://openradar.appspot.com/radar?id=4968600712511488
424-
// `[AVPlayerItem duration]` can be `kCMTimeIndefinite`,
425-
// use `[[AVPlayerItem asset] duration]` instead.
426-
return FVPCMTimeToMillis([[[_player currentItem] asset] duration]);
438+
- (nullable NSNumber *)position:(FlutterError *_Nullable *_Nonnull)error {
439+
return @(FVPCMTimeToMillis([_player currentTime]));
427440
}
428441

429-
- (void)seekTo:(int64_t)location completionHandler:(void (^)(BOOL))completionHandler {
430-
CMTime targetCMTime = CMTimeMake(location, 1000);
442+
- (void)seekTo:(NSInteger)position completion:(void (^)(FlutterError *_Nullable))completion {
443+
CMTime targetCMTime = CMTimeMake(position, 1000);
431444
CMTimeValue duration = _player.currentItem.asset.duration.value;
432445
// Without adding tolerance when seeking to duration,
433446
// seekToTime will never complete, and this call will hang.
434447
// see issue https://github.com/flutter/flutter/issues/124475.
435-
CMTime tolerance = location == duration ? CMTimeMake(1, 1000) : kCMTimeZero;
448+
CMTime tolerance = position == duration ? CMTimeMake(1, 1000) : kCMTimeZero;
436449
[_player seekToTime:targetCMTime
437450
toleranceBefore:tolerance
438451
toleranceAfter:tolerance
439452
completionHandler:^(BOOL completed) {
440-
if (completionHandler) {
441-
completionHandler(completed);
453+
if (completion) {
454+
dispatch_async(dispatch_get_main_queue(), ^{
455+
completion(nil);
456+
});
442457
}
443458
}];
444459
}
445460

446-
- (void)setIsLooping:(BOOL)isLooping {
447-
_isLooping = isLooping;
461+
- (void)setLooping:(BOOL)looping error:(FlutterError *_Nullable *_Nonnull)error {
462+
_isLooping = looping;
448463
}
449464

450-
- (void)setVolume:(double)volume {
465+
- (void)setVolume:(double)volume error:(FlutterError *_Nullable *_Nonnull)error {
451466
_player.volume = (float)((volume < 0.0) ? 0.0 : ((volume > 1.0) ? 1.0 : volume));
452467
}
453468

454-
- (void)setPlaybackSpeed:(double)speed {
469+
- (void)setPlaybackSpeed:(double)speed error:(FlutterError *_Nullable *_Nonnull)error {
455470
_targetPlaybackSpeed = @(speed);
456471
[self updatePlayingState];
457472
}
458473

474+
#pragma mark - FlutterStreamHandler
475+
459476
- (FlutterError *_Nullable)onCancelWithArguments:(id _Nullable)arguments {
460477
_eventSink = nil;
461478
return nil;
@@ -480,20 +497,13 @@ - (FlutterError *_Nullable)onListenWithArguments:(id _Nullable)arguments
480497
return nil;
481498
}
482499

483-
/// This method allows you to dispose without touching the event channel. This
484-
/// is useful for the case where the Engine is in the process of deconstruction
485-
/// so the channel is going to die or is already dead.
486-
- (void)disposeSansEventChannel {
487-
_disposed = YES;
488-
[self removeKeyValueObservers];
489-
490-
[self.player replaceCurrentItemWithPlayerItem:nil];
491-
[[NSNotificationCenter defaultCenter] removeObserver:self];
492-
}
500+
#pragma mark - Private
493501

494-
- (void)dispose {
495-
[self disposeSansEventChannel];
496-
[_eventChannel setStreamHandler:nil];
502+
- (int64_t)duration {
503+
// Note: https://openradar.appspot.com/radar?id=4968600712511488
504+
// `[AVPlayerItem duration]` can be `kCMTimeIndefinite`,
505+
// use `[[AVPlayerItem asset] duration]` instead.
506+
return FVPCMTimeToMillis([[[_player currentItem] asset] duration]);
497507
}
498508

499509
/// Removes all key-value observers set up for the player.

packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m

Lines changed: 20 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -115,18 +115,32 @@ - (int64_t)onPlayerSetup:(FVPVideoPlayer *)player {
115115

116116
int64_t playerIdentifier;
117117
if (textureBasedPlayer) {
118-
playerIdentifier = [self.registry registerTexture:(FVPTextureBasedVideoPlayer *)player];
118+
playerIdentifier = [self.registry registerTexture:textureBasedPlayer];
119119
[textureBasedPlayer setTextureIdentifier:playerIdentifier];
120120
} else {
121121
playerIdentifier = self.nextNonTexturePlayerIdentifier--;
122122
}
123123

124+
NSObject<FlutterBinaryMessenger> *messenger = self.messenger;
125+
NSString *channelSuffix = [NSString stringWithFormat:@"%lld", playerIdentifier];
126+
// Set up the player-specific API handler, and its onDispose unregistration.
127+
SetUpFVPVideoPlayerInstanceApiWithSuffix(messenger, player, channelSuffix);
128+
__weak typeof(self) weakSelf = self;
129+
BOOL isTextureBased = textureBasedPlayer != nil;
130+
player.onDisposed = ^() {
131+
SetUpFVPVideoPlayerInstanceApiWithSuffix(messenger, nil, channelSuffix);
132+
if (isTextureBased) {
133+
[weakSelf.registry unregisterTexture:playerIdentifier];
134+
}
135+
};
136+
// Set up the event channel.
124137
FlutterEventChannel *eventChannel = [FlutterEventChannel
125-
eventChannelWithName:[NSString stringWithFormat:@"flutter.io/videoPlayer/videoEvents%lld",
126-
playerIdentifier]
127-
binaryMessenger:_messenger];
138+
eventChannelWithName:[NSString stringWithFormat:@"flutter.io/videoPlayer/videoEvents%@",
139+
channelSuffix]
140+
binaryMessenger:messenger];
128141
[eventChannel setStreamHandler:player];
129142
player.eventChannel = eventChannel;
143+
130144
self.playersByIdentifier[@(playerIdentifier)] = player;
131145

132146
// Ensure that the first frame is drawn once available, even if the video isn't played, since
@@ -211,27 +225,20 @@ - (nullable FVPTextureBasedVideoPlayer *)texturePlayerWithOptions:
211225
[frameUpdater displayLinkFired];
212226
}];
213227

214-
__weak typeof(self) weakSelf = self;
215-
void (^onDisposed)(int64_t) = ^(int64_t textureIdentifier) {
216-
[weakSelf.registry unregisterTexture:textureIdentifier];
217-
};
218-
219228
if (options.asset) {
220229
NSString *assetPath = [self assetPathFromCreationOptions:options];
221230
return [[FVPTextureBasedVideoPlayer alloc] initWithAsset:assetPath
222231
frameUpdater:frameUpdater
223232
displayLink:displayLink
224233
avFactory:self.avFactory
225-
viewProvider:self.viewProvider
226-
onDisposed:onDisposed];
234+
viewProvider:self.viewProvider];
227235
} else if (options.uri) {
228236
return [[FVPTextureBasedVideoPlayer alloc] initWithURL:[NSURL URLWithString:options.uri]
229237
frameUpdater:frameUpdater
230238
displayLink:displayLink
231239
httpHeaders:options.httpHeaders
232240
avFactory:self.avFactory
233-
viewProvider:self.viewProvider
234-
onDisposed:onDisposed];
241+
viewProvider:self.viewProvider];
235242
}
236243

237244
return nil;
@@ -271,54 +278,6 @@ - (void)disposePlayer:(NSInteger)playerIdentifier error:(FlutterError **)error {
271278
[player dispose];
272279
}
273280

274-
- (void)setLooping:(BOOL)isLooping
275-
forPlayer:(NSInteger)playerIdentifier
276-
error:(FlutterError **)error {
277-
FVPVideoPlayer *player = self.playersByIdentifier[@(playerIdentifier)];
278-
player.isLooping = isLooping;
279-
}
280-
281-
- (void)setVolume:(double)volume
282-
forPlayer:(NSInteger)playerIdentifier
283-
error:(FlutterError **)error {
284-
FVPVideoPlayer *player = self.playersByIdentifier[@(playerIdentifier)];
285-
[player setVolume:volume];
286-
}
287-
288-
- (void)setPlaybackSpeed:(double)speed
289-
forPlayer:(NSInteger)playerIdentifier
290-
error:(FlutterError **)error {
291-
FVPVideoPlayer *player = self.playersByIdentifier[@(playerIdentifier)];
292-
[player setPlaybackSpeed:speed];
293-
}
294-
295-
- (void)playPlayer:(NSInteger)playerIdentifier error:(FlutterError **)error {
296-
FVPVideoPlayer *player = self.playersByIdentifier[@(playerIdentifier)];
297-
[player play];
298-
}
299-
300-
- (nullable NSNumber *)positionForPlayer:(NSInteger)playerIdentifier error:(FlutterError **)error {
301-
FVPVideoPlayer *player = self.playersByIdentifier[@(playerIdentifier)];
302-
return @([player position]);
303-
}
304-
305-
- (void)seekTo:(NSInteger)position
306-
forPlayer:(NSInteger)playerIdentifier
307-
completion:(nonnull void (^)(FlutterError *_Nullable))completion {
308-
FVPVideoPlayer *player = self.playersByIdentifier[@(playerIdentifier)];
309-
[player seekTo:position
310-
completionHandler:^(BOOL finished) {
311-
dispatch_async(dispatch_get_main_queue(), ^{
312-
completion(nil);
313-
});
314-
}];
315-
}
316-
317-
- (void)pausePlayer:(NSInteger)playerIdentifier error:(FlutterError **)error {
318-
FVPVideoPlayer *player = self.playersByIdentifier[@(playerIdentifier)];
319-
[player pause];
320-
}
321-
322281
- (void)setMixWithOthers:(BOOL)mixWithOthers
323282
error:(FlutterError *_Nullable __autoreleasing *)error {
324283
#if TARGET_OS_OSX

packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/include/video_player_avfoundation/FVPTextureBasedVideoPlayer.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,15 @@ NS_ASSUME_NONNULL_BEGIN
2121
displayLink:(NSObject<FVPDisplayLink> *)displayLink
2222
httpHeaders:(nonnull NSDictionary<NSString *, NSString *> *)headers
2323
avFactory:(id<FVPAVFactory>)avFactory
24-
viewProvider:(NSObject<FVPViewProvider> *)viewProvider
25-
onDisposed:(void (^)(int64_t))onDisposed;
24+
viewProvider:(NSObject<FVPViewProvider> *)viewProvider;
2625

2726
/// Initializes a new instance of FVPTextureBasedVideoPlayer with the given asset, frame updater,
2827
/// display link, AV factory, and registrar.
2928
- (instancetype)initWithAsset:(NSString *)asset
3029
frameUpdater:(FVPFrameUpdater *)frameUpdater
3130
displayLink:(NSObject<FVPDisplayLink> *)displayLink
3231
avFactory:(id<FVPAVFactory>)avFactory
33-
viewProvider:(NSObject<FVPViewProvider> *)viewProvider
34-
onDisposed:(void (^)(int64_t))onDisposed;
32+
viewProvider:(NSObject<FVPViewProvider> *)viewProvider;
3533

3634
/// Sets the texture Identifier for the frame updater. This method should be called once the texture
3735
/// identifier is obtained from the texture registry.

0 commit comments

Comments
 (0)