Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2.7.3

* Restructures the communication between Dart and native code.

## 2.7.2

* Uses `CADisplayLink` on macOS 14.0+.
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -28,32 +28,28 @@ @interface FVPTextureBasedVideoPlayer ()
// (e.g., after a seek while paused). If YES, the display link should continue to run until the next
// frame is successfully provided.
@property(nonatomic, assign) BOOL waitingForFrame;
@property(nonatomic, copy) void (^onDisposed)(int64_t);
@end

@implementation FVPTextureBasedVideoPlayer
- (instancetype)initWithAsset:(NSString *)asset
frameUpdater:(FVPFrameUpdater *)frameUpdater
displayLink:(NSObject<FVPDisplayLink> *)displayLink
avFactory:(id<FVPAVFactory>)avFactory
viewProvider:(NSObject<FVPViewProvider> *)viewProvider
onDisposed:(void (^)(int64_t))onDisposed {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This changed to being a post-init property assignment because we need the player ID, which is only available just after creation.

viewProvider:(NSObject<FVPViewProvider> *)viewProvider {
return [self initWithURL:[NSURL fileURLWithPath:[FVPVideoPlayer absolutePathForAssetName:asset]]
frameUpdater:frameUpdater
displayLink:displayLink
httpHeaders:@{}
avFactory:avFactory
viewProvider:viewProvider
onDisposed:onDisposed];
viewProvider:viewProvider];
}

- (instancetype)initWithURL:(NSURL *)url
frameUpdater:(FVPFrameUpdater *)frameUpdater
displayLink:(NSObject<FVPDisplayLink> *)displayLink
httpHeaders:(nonnull NSDictionary<NSString *, NSString *> *)headers
avFactory:(id<FVPAVFactory>)avFactory
viewProvider:(NSObject<FVPViewProvider> *)viewProvider
onDisposed:(void (^)(int64_t))onDisposed {
viewProvider:(NSObject<FVPViewProvider> *)viewProvider {
NSDictionary<NSString *, id> *options = nil;
if ([headers count] != 0) {
options = @{@"AVURLAssetHTTPHeaderFieldsKey" : headers};
Expand All @@ -64,24 +60,21 @@ - (instancetype)initWithURL:(NSURL *)url
frameUpdater:frameUpdater
displayLink:displayLink
avFactory:avFactory
viewProvider:viewProvider
onDisposed:onDisposed];
viewProvider:viewProvider];
}

- (instancetype)initWithPlayerItem:(AVPlayerItem *)item
frameUpdater:(FVPFrameUpdater *)frameUpdater
displayLink:(NSObject<FVPDisplayLink> *)displayLink
avFactory:(id<FVPAVFactory>)avFactory
viewProvider:(NSObject<FVPViewProvider> *)viewProvider
onDisposed:(void (^)(int64_t))onDisposed {
viewProvider:(NSObject<FVPViewProvider> *)viewProvider {
self = [super initWithPlayerItem:item avFactory:avFactory viewProvider:viewProvider];

if (self) {
_frameUpdater = frameUpdater;
_displayLink = displayLink;
_frameUpdater.displayLink = _displayLink;
_selfRefresh = true;
_onDisposed = [onDisposed copy];

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

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

if (completionHandler) {
completionHandler(completed);
if (completion) {
completion(error);
}
}];
}
Expand All @@ -153,12 +146,6 @@ - (void)disposeSansEventChannel {
_displayLink = nil;
}

- (void)dispose {
[super dispose];

_onDisposed(self.frameUpdater.textureIdentifier);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This moved to the superclass since all players have an onDisposed now.


#pragma mark - FlutterTexture

- (CVPixelBufferRef)copyPixelBuffer {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

#import "./include/video_player_avfoundation/FVPVideoPlayer.h"
#import "./include/video_player_avfoundation/FVPVideoPlayer_Internal.h"
#import "./include/video_player_avfoundation/FVPVideoPlayer_Test.h"

#import <GLKit/GLKit.h>

Expand Down Expand Up @@ -104,6 +103,25 @@ - (void)dealloc {
}
}

/// This method allows you to dispose without touching the event channel. This
/// is useful for the case where the Engine is in the process of deconstruction
/// so the channel is going to die or is already dead.
- (void)disposeSansEventChannel {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is moved from below without change, to create a #pragma mark - separated section for each protocol implemented.

_disposed = YES;
[self removeKeyValueObservers];

[self.player replaceCurrentItemWithPlayerItem:nil];
[[NSNotificationCenter defaultCenter] removeObserver:self];
}

- (void)dispose {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was also moved, for the same reason. The change is the addition of the _onDisposed call.

[self disposeSansEventChannel];
if (_onDisposed) {
_onDisposed();
}
[_eventChannel setStreamHandler:nil];
}

+ (NSString *)absolutePathForAssetName:(NSString *)assetName {
NSString *path = [[NSBundle mainBundle] pathForResource:assetName ofType:nil];
#if TARGET_OS_OSX
Expand Down Expand Up @@ -405,57 +423,56 @@ - (void)setupEventSinkIfReadyToPlay {
}
}

- (void)play {
#pragma mark - FVPVideoPlayerInstanceApi

- (void)playWithError:(FlutterError *_Nullable *_Nonnull)error {
_isPlaying = YES;
[self updatePlayingState];
}

- (void)pause {
- (void)pauseWithError:(FlutterError *_Nullable *_Nonnull)error {
_isPlaying = NO;
[self updatePlayingState];
}

- (int64_t)position {
return FVPCMTimeToMillis([_player currentTime]);
}

- (int64_t)duration {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is moved below as it's not part of of the protocol.

// Note: https://openradar.appspot.com/radar?id=4968600712511488
// `[AVPlayerItem duration]` can be `kCMTimeIndefinite`,
// use `[[AVPlayerItem asset] duration]` instead.
return FVPCMTimeToMillis([[[_player currentItem] asset] duration]);
- (nullable NSNumber *)position:(FlutterError *_Nullable *_Nonnull)error {
return @(FVPCMTimeToMillis([_player currentTime]));
}

- (void)seekTo:(int64_t)location completionHandler:(void (^)(BOOL))completionHandler {
CMTime targetCMTime = CMTimeMake(location, 1000);
- (void)seekTo:(NSInteger)position completion:(void (^)(FlutterError *_Nullable))completion {
CMTime targetCMTime = CMTimeMake(position, 1000);
CMTimeValue duration = _player.currentItem.asset.duration.value;
// Without adding tolerance when seeking to duration,
// seekToTime will never complete, and this call will hang.
// see issue https://github.com/flutter/flutter/issues/124475.
CMTime tolerance = location == duration ? CMTimeMake(1, 1000) : kCMTimeZero;
CMTime tolerance = position == duration ? CMTimeMake(1, 1000) : kCMTimeZero;
[_player seekToTime:targetCMTime
toleranceBefore:tolerance
toleranceAfter:tolerance
completionHandler:^(BOOL completed) {
if (completionHandler) {
completionHandler(completed);
if (completion) {
dispatch_async(dispatch_get_main_queue(), ^{
completion(nil);
});
}
}];
}

- (void)setIsLooping:(BOOL)isLooping {
_isLooping = isLooping;
- (void)setLooping:(BOOL)looping error:(FlutterError *_Nullable *_Nonnull)error {
_isLooping = looping;
}

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

- (void)setPlaybackSpeed:(double)speed {
- (void)setPlaybackSpeed:(double)speed error:(FlutterError *_Nullable *_Nonnull)error {
_targetPlaybackSpeed = @(speed);
[self updatePlayingState];
}

#pragma mark - FlutterStreamHandler

- (FlutterError *_Nullable)onCancelWithArguments:(id _Nullable)arguments {
_eventSink = nil;
return nil;
Expand All @@ -480,20 +497,13 @@ - (FlutterError *_Nullable)onListenWithArguments:(id _Nullable)arguments
return nil;
}

/// This method allows you to dispose without touching the event channel. This
/// is useful for the case where the Engine is in the process of deconstruction
/// so the channel is going to die or is already dead.
- (void)disposeSansEventChannel {
_disposed = YES;
[self removeKeyValueObservers];

[self.player replaceCurrentItemWithPlayerItem:nil];
[[NSNotificationCenter defaultCenter] removeObserver:self];
}
#pragma mark - Private

- (void)dispose {
[self disposeSansEventChannel];
[_eventChannel setStreamHandler:nil];
- (int64_t)duration {
// Note: https://openradar.appspot.com/radar?id=4968600712511488
// `[AVPlayerItem duration]` can be `kCMTimeIndefinite`,
// use `[[AVPlayerItem asset] duration]` instead.
return FVPCMTimeToMillis([[[_player currentItem] asset] duration]);
}

/// Removes all key-value observers set up for the player.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,18 +115,32 @@ - (int64_t)onPlayerSetup:(FVPVideoPlayer *)player {

int64_t playerIdentifier;
if (textureBasedPlayer) {
playerIdentifier = [self.registry registerTexture:(FVPTextureBasedVideoPlayer *)player];
playerIdentifier = [self.registry registerTexture:textureBasedPlayer];
[textureBasedPlayer setTextureIdentifier:playerIdentifier];
} else {
playerIdentifier = self.nextNonTexturePlayerIdentifier--;
}

NSObject<FlutterBinaryMessenger> *messenger = self.messenger;
NSString *channelSuffix = [NSString stringWithFormat:@"%lld", playerIdentifier];
// Set up the player-specific API handler, and its onDispose unregistration.
SetUpFVPVideoPlayerInstanceApiWithSuffix(messenger, player, channelSuffix);
__weak typeof(self) weakSelf = self;
BOOL isTextureBased = textureBasedPlayer != nil;
player.onDisposed = ^() {
SetUpFVPVideoPlayerInstanceApiWithSuffix(messenger, nil, channelSuffix);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The addition of onDisposed to the base class is to allow easily cleaning up the communication channel, without having to push all the communication channel wire-up into the player class (which would be a bigger change, and is also undesirable for potential conversion to using FFI to talk directly to the player instance).

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious why this would have FFI implications? In my mind the communication channel wire-up / tear-down part are implementation details within the platform, the dart side should be able to tell the plugin to create a player, or dispose an existing player without involving any communication channel concepts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If the structure of the native code is that all the channel-related details are contained entirely within the main plugin class (this file), and the player classes are just receiving calls, then switching to FFI can be done with almost no changes to the player instance classes.

In contrast, if I had pushed instance channel setup in the player instance classes in this PR, then a future change to FFI would require more changes in those classes to unwind that, instead of mostly changing this plugin-structure-specific class.

if (isTextureBased) {
[weakSelf.registry unregisterTexture:playerIdentifier];
}
};
// Set up the event channel.
FlutterEventChannel *eventChannel = [FlutterEventChannel
eventChannelWithName:[NSString stringWithFormat:@"flutter.io/videoPlayer/videoEvents%lld",
playerIdentifier]
binaryMessenger:_messenger];
eventChannelWithName:[NSString stringWithFormat:@"flutter.io/videoPlayer/videoEvents%@",
channelSuffix]
binaryMessenger:messenger];
[eventChannel setStreamHandler:player];
player.eventChannel = eventChannel;

self.playersByIdentifier[@(playerIdentifier)] = player;

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

__weak typeof(self) weakSelf = self;
void (^onDisposed)(int64_t) = ^(int64_t textureIdentifier) {
[weakSelf.registry unregisterTexture:textureIdentifier];
};

if (options.asset) {
NSString *assetPath = [self assetPathFromCreationOptions:options];
return [[FVPTextureBasedVideoPlayer alloc] initWithAsset:assetPath
frameUpdater:frameUpdater
displayLink:displayLink
avFactory:self.avFactory
viewProvider:self.viewProvider
onDisposed:onDisposed];
viewProvider:self.viewProvider];
} else if (options.uri) {
return [[FVPTextureBasedVideoPlayer alloc] initWithURL:[NSURL URLWithString:options.uri]
frameUpdater:frameUpdater
displayLink:displayLink
httpHeaders:options.httpHeaders
avFactory:self.avFactory
viewProvider:self.viewProvider
onDisposed:onDisposed];
viewProvider:self.viewProvider];
}

return nil;
Expand Down Expand Up @@ -271,54 +278,6 @@ - (void)disposePlayer:(NSInteger)playerIdentifier error:(FlutterError **)error {
[player dispose];
}

- (void)setLooping:(BOOL)isLooping
forPlayer:(NSInteger)playerIdentifier
error:(FlutterError **)error {
FVPVideoPlayer *player = self.playersByIdentifier[@(playerIdentifier)];
player.isLooping = isLooping;
}

- (void)setVolume:(double)volume
forPlayer:(NSInteger)playerIdentifier
error:(FlutterError **)error {
FVPVideoPlayer *player = self.playersByIdentifier[@(playerIdentifier)];
[player setVolume:volume];
}

- (void)setPlaybackSpeed:(double)speed
forPlayer:(NSInteger)playerIdentifier
error:(FlutterError **)error {
FVPVideoPlayer *player = self.playersByIdentifier[@(playerIdentifier)];
[player setPlaybackSpeed:speed];
}

- (void)playPlayer:(NSInteger)playerIdentifier error:(FlutterError **)error {
FVPVideoPlayer *player = self.playersByIdentifier[@(playerIdentifier)];
[player play];
}

- (nullable NSNumber *)positionForPlayer:(NSInteger)playerIdentifier error:(FlutterError **)error {
FVPVideoPlayer *player = self.playersByIdentifier[@(playerIdentifier)];
return @([player position]);
}

- (void)seekTo:(NSInteger)position
forPlayer:(NSInteger)playerIdentifier
completion:(nonnull void (^)(FlutterError *_Nullable))completion {
FVPVideoPlayer *player = self.playersByIdentifier[@(playerIdentifier)];
[player seekTo:position
completionHandler:^(BOOL finished) {
dispatch_async(dispatch_get_main_queue(), ^{
completion(nil);
});
}];
}

- (void)pausePlayer:(NSInteger)playerIdentifier error:(FlutterError **)error {
FVPVideoPlayer *player = self.playersByIdentifier[@(playerIdentifier)];
[player pause];
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the core of the change: we no longer need boilerplate like this for every player method.


- (void)setMixWithOthers:(BOOL)mixWithOthers
error:(FlutterError *_Nullable __autoreleasing *)error {
#if TARGET_OS_OSX
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,15 @@ NS_ASSUME_NONNULL_BEGIN
displayLink:(NSObject<FVPDisplayLink> *)displayLink
httpHeaders:(nonnull NSDictionary<NSString *, NSString *> *)headers
avFactory:(id<FVPAVFactory>)avFactory
viewProvider:(NSObject<FVPViewProvider> *)viewProvider
onDisposed:(void (^)(int64_t))onDisposed;
viewProvider:(NSObject<FVPViewProvider> *)viewProvider;

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

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