Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[video_player_avfoundation] fix playback speed resetting #7657

Merged
merged 14 commits into from
Jan 15, 2025
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## NEXT
## 2.6.2

* Fixes playback speed resetting.
* Updates minimum supported SDK version to Flutter 3.19/Dart 3.3.

## 2.6.1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -656,6 +656,8 @@ - (void)testSeekToleranceWhenSeekingToEnd {
// Change playback speed.
[videoPlayerPlugin setPlaybackSpeed:2 forPlayer:textureId.integerValue error:&error];
XCTAssertNil(error);
[videoPlayerPlugin playPlayer:textureId.integerValue error:&error];
XCTAssertNil(error);
XCTAssertEqual(avPlayer.rate, 2);
XCTAssertEqual(avPlayer.timeControlStatus, AVPlayerTimeControlStatusWaitingToPlayAtSpecifiedRate);

Expand Down Expand Up @@ -790,6 +792,41 @@ - (void)testPublishesInRegistration {
XCTAssertTrue([publishedValue isKindOfClass:[FVPVideoPlayerPlugin class]]);
}

- (void)testUpdatePlayingStateShouldNotResetRate {
NSObject<FlutterPluginRegistrar> *registrar =
[GetPluginRegistry() registrarForPlugin:@"testUpdatePlayingStateShouldNotResetRate"];

FVPVideoPlayerPlugin *videoPlayerPlugin = [[FVPVideoPlayerPlugin alloc]
initWithAVFactory:[[StubFVPAVFactory alloc] initWithPlayer:nil output:nil]
displayLinkFactory:nil
registrar:registrar];

FlutterError *error;
[videoPlayerPlugin initialize:&error];
XCTAssertNil(error);
FVPCreationOptions *create = [FVPCreationOptions
makeWithAsset:nil
uri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/bee.mp4"
packageName:nil
formatHint:nil
httpHeaders:@{}];
NSNumber *textureId = [videoPlayerPlugin createWithOptions:create error:&error];
FVPVideoPlayer *player = videoPlayerPlugin.playersByTextureId[textureId];

XCTestExpectation *initializedExpectation = [self expectationWithDescription:@"initialized"];
[player onListenWithArguments:nil
eventSink:^(NSDictionary<NSString *, id> *event) {
if ([event[@"event"] isEqualToString:@"initialized"]) {
[initializedExpectation fulfill];
}
}];
[self waitForExpectationsWithTimeout:10 handler:nil];

[videoPlayerPlugin setPlaybackSpeed:2 forPlayer:textureId.integerValue error:&error];
[videoPlayerPlugin playPlayer:textureId.integerValue error:&error];
XCTAssertEqual(player.player.rate, 2);
}

#if TARGET_OS_IOS
- (void)validateTransformFixForOrientation:(UIImageOrientation)orientation {
AVAssetTrack *track = [[FakeAVAssetTrack alloc] initWithOrientation:orientation];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ @interface FVPVideoPlayer ()
@property(nonatomic) CGAffineTransform preferredTransform;
@property(nonatomic, readonly) BOOL disposed;
@property(nonatomic, readonly) BOOL isPlaying;
// Playback speed when video is playing.
@property(nonatomic, readonly) NSNumber *playbackSpeed;
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be more accurate to call this targetPlaybackSpeed (and document it as something like "The target playback speed requested by the plugin client."), since it's entirely possible for the playback speed to not be this value while playing.

@property(nonatomic) BOOL isLooping;
@property(nonatomic, readonly) BOOL isInitialized;
// The updater that drives callbacks to the engine to indicate that a new frame is ready.
Expand Down Expand Up @@ -397,7 +399,15 @@ - (void)updatePlayingState {
return;
}
if (_isPlaying) {
[_player play];
// Calling play is the same as setting the rate to 1.0 (or to defaultRate depending on ios
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: iOS

// version) so last set playback speed must be set here if any instead.
// https://github.com/flutter/flutter/issues/71264
// https://github.com/flutter/flutter/issues/73643
if (_playbackSpeed) {
[self updateRate];
} else {
[_player play];
stuartmorgan marked this conversation as resolved.
Show resolved Hide resolved
}
} else {
[_player pause];
}
Expand All @@ -406,6 +416,29 @@ - (void)updatePlayingState {
_displayLink.running = _isPlaying || self.waitingForFrame;
}

- (void)updateRate {
stuartmorgan marked this conversation as resolved.
Show resolved Hide resolved
// See https://developer.apple.com/library/archive/qa/qa1772/_index.html for an explanation of
// these checks.
// If status is not AVPlayerItemStatusReadyToPlay then both canPlayFastForward
// and canPlaySlowForward are always false and it is unknown whether video can
// be played at these speeds, updatePlayingState will be called again when
// status changes to AVPlayerItemStatusReadyToPlay.
float speed = _playbackSpeed.floatValue;
if (speed > 2.0 && !_player.currentItem.canPlayFastForward) {
if (_player.currentItem.status != AVPlayerItemStatusReadyToPlay) {
Copy link
Contributor

Choose a reason for hiding this comment

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

what if we call updateRate with speed < 2.0? should we also check the status here? Should this status check be moved to top of the fucntion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That check is to confirm whether canPlay... returned false because it cannot be played at speed>2 instead of because status was not ready to play. There is no need to check status for speeds for which is not needed canPlay.... But as I am thinking about it now maybe status should be fetched before canPlay... to avoid a false positive when status becomes ready right after canPlay....

return;
}
speed = 2.0;
}
if (speed < 1.0 && !_player.currentItem.canPlaySlowForward) {
if (_player.currentItem.status != AVPlayerItemStatusReadyToPlay) {
return;
}
speed = 1.0;
}
_player.rate = speed;
}

- (void)setupEventSinkIfReadyToPlay {
if (_eventSink && !_isInitialized) {
AVPlayerItem *currentItem = self.player.currentItem;
Expand Down Expand Up @@ -519,27 +552,8 @@ - (void)setVolume:(double)volume {
}

- (void)setPlaybackSpeed:(double)speed {
// See https://developer.apple.com/library/archive/qa/qa1772/_index.html for an explanation of
// these checks.
if (speed > 2.0 && !_player.currentItem.canPlayFastForward) {
if (_eventSink != nil) {
_eventSink([FlutterError errorWithCode:@"VideoError"
message:@"Video cannot be fast-forwarded beyond 2.0x"
details:nil]);
}
return;
}

if (speed < 1.0 && !_player.currentItem.canPlaySlowForward) {
if (_eventSink != nil) {
_eventSink([FlutterError errorWithCode:@"VideoError"
message:@"Video cannot be slow-forwarded"
details:nil]);
}
return;
}

_player.rate = speed;
_playbackSpeed = @(speed);
[self updatePlayingState];
}

- (CVPixelBufferRef)copyPixelBuffer {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: video_player_avfoundation
description: iOS and macOS implementation of the video_player plugin.
repository: https://github.com/flutter/packages/tree/main/packages/video_player/video_player_avfoundation
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+video_player%22
version: 2.6.1
version: 2.6.2

environment:
sdk: ^3.3.0
Expand Down