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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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.6.3

* Fixes playback speed resetting.

## 2.6.2

* Updates Pigeon for non-nullable collection type support.
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,30 @@ - (void)updatePlayingState {
_displayLink.running = _isPlaying || self.waitingForFrame;
}

- (void)updateRate {
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs a declaration comment, per the style guide.

// 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;
bool readyToPlay = _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.

BOOL since this is Obj-C code.

if (speed > 2.0 && !_player.currentItem.canPlayFastForward) {
if (!readyToPlay) {
return;
}
speed = 2.0;
}
if (speed < 1.0 && !_player.currentItem.canPlaySlowForward) {
if (!readyToPlay) {
return;
}
speed = 1.0;
}
_player.rate = speed;
}

- (void)setupEventSinkIfReadyToPlay {
if (_eventSink && !_isInitialized) {
AVPlayerItem *currentItem = self.player.currentItem;
Expand Down Expand Up @@ -519,27 +553,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.2
version: 2.6.3

environment:
sdk: ^3.3.0
Expand Down