Skip to content

Commit

Permalink
[video_player_avfoundation] fix playback speed resetting (#7657)
Browse files Browse the repository at this point in the history
Calling play is the same as setting the rate to 1.0 (or to `defaultRate` depending on ios version, *documentation in this first link is clearly not updated because it does not mention `defaultRate`*):

https://developer.apple.com/documentation/avfoundation/avplayer/1386726-play?language=objc
https://developer.apple.com/documentation/avfoundation/avplayer/3929373-defaultrate?language=objc

*The second link contains a note about not starting playback by setting the rate to 1.0. I assume this is because of the introduction of `defaultRate` (which can be different than 1.0) and not because `play` may do something more than just setting `rate` as that wording is explicit about setting rate to 1.0, it says nothing about any other value.*

This is also why flutter/plugins#4331 did not work well. It was setting `rate` to 1.0 (through `play`) then immediately to the value set by `setPlaybackSpeed`. One of them or both caused change to `playbackLikelyToKeepUp` which triggered `observeValueForKeyPath` with `playbackLikelyToKeepUpContext` which in turn called again `updatePlayingState` where was `rate` again changed first to 1.0 then to another value and so on. In this PR the rate is changed only once and then to the same value, seems when `rate` is assigned but not really changed it does not trigger anything.

In issues below `seekTo` can trigger `playbackLikelyToKeepUp` change which will call `updatePlayingState` and reset `rate` to 1.0 through `play`. When the speed is set in dart before initialization then the dart side will set it right after calling `play` but even some time after initialization there is still a flood of events calling `updatePlayingState` and it is likely that some of them will call it after `setPlaybackSpeed`. Actually even when `setPlaybackSpeed` was not called on dart side it (dart side) will always change speed after play to 1.0 so it ignores this new `defaultRate` feature.

- fixes flutter/flutter#71264
- fixes flutter/flutter#73643
  • Loading branch information
misos1 authored Jan 15, 2025
1 parent f73cb00 commit 9ea2bef
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 23 deletions.
4 changes: 4 additions & 0 deletions packages/video_player/video_player_avfoundation/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2.6.7

* Fixes playback speed resetting.

## 2.6.6

* Fixes changing global audio session category to be collision free across plugins.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,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 @@ -839,6 +841,41 @@ - (void)testFailedToLoadVideoEventShouldBeAlwaysSent {
[self waitForExpectationsWithTimeout:10.0 handler:nil];
}

- (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)testVideoPlayerShouldNotOverwritePlayAndRecordNorDefaultToSpeaker {
NSObject<FlutterPluginRegistrar> *registrar = [GetPluginRegistry()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ @interface FVPVideoPlayer ()
@property(nonatomic) CGAffineTransform preferredTransform;
/// Indicates whether the video player is currently playing.
@property(nonatomic, readonly) BOOL isPlaying;
/// The target playback speed requested by the plugin client.
@property(nonatomic, readonly) NSNumber *targetPlaybackSpeed;
/// Indicates whether the video player has been initialized.
@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 @@ -323,7 +325,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
// 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 (_targetPlaybackSpeed) {
[self updateRate];
} else {
[_player play];
}
} else {
[_player pause];
}
Expand All @@ -332,6 +342,32 @@ - (void)updatePlayingState {
_displayLink.running = _isPlaying || self.waitingForFrame;
}

/// Synchronizes the player's playback rate with targetPlaybackSpeed, constrained by the playback
/// rate capabilities of the player's current item.
- (void)updateRate {
// 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 = _targetPlaybackSpeed.floatValue;
BOOL readyToPlay = _player.currentItem.status == AVPlayerItemStatusReadyToPlay;
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)sendFailedToLoadVideoEvent {
if (_eventSink == nil) {
return;
Expand Down Expand Up @@ -473,27 +509,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;
_targetPlaybackSpeed = @(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.6
version: 2.6.7

environment:
sdk: ^3.4.0
Expand Down

0 comments on commit 9ea2bef

Please sign in to comment.