Skip to content

[video_player_avfoundation] removes unnecessary duration and size check before sending the initialized event #9534

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

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
42a5ba4
Make sure the AVPlayerItem is ready to play before sending initialize…
LongCatIsLooong Jun 30, 2025
375ec1f
Merge remote-tracking branch 'upstream/main' into video_player_readyT…
LongCatIsLooong Jun 30, 2025
bb89c41
wuh
LongCatIsLooong Jun 30, 2025
1ad1d2e
docs
LongCatIsLooong Jun 30, 2025
63d240b
example app and README.md
LongCatIsLooong Jul 1, 2025
0cfe3f4
format
LongCatIsLooong Jul 1, 2025
60fc5d2
Update CHANGELOG.md
LongCatIsLooong Jul 7, 2025
f3fbff5
Merge branch 'main' into video_player_readyToPlay
LongCatIsLooong Jul 7, 2025
bcc2725
Update pubspec.yaml
LongCatIsLooong Jul 7, 2025
733f5ea
Update CHANGELOG.md
LongCatIsLooong Jul 7, 2025
c86f933
Merge branch 'main' into video_player_readyToPlay
LongCatIsLooong Jul 8, 2025
35c9a08
Update pubspec.yaml
LongCatIsLooong Jul 8, 2025
3b0cd77
Merge branch 'main' into video_player_readyToPlay
LongCatIsLooong Jul 9, 2025
4315c4a
oops
LongCatIsLooong Jul 10, 2025
47ff5d0
Merge remote-tracking branch 'upstream/main' into video_player_readyT…
LongCatIsLooong Jul 17, 2025
e2ce9fd
add a test
LongCatIsLooong Jul 17, 2025
6f232f5
gemini review
LongCatIsLooong Jul 18, 2025
11e8815
Merge remote-tracking branch 'upstream/main' into video_player_readyT…
LongCatIsLooong Aug 11, 2025
f88925e
comments
LongCatIsLooong Aug 11, 2025
72b7474
formatter
LongCatIsLooong Aug 11, 2025
2179d4e
buh
LongCatIsLooong Aug 11, 2025
5392116
fix test
LongCatIsLooong Aug 11, 2025
7769699
fix test
LongCatIsLooong Aug 11, 2025
5457167
add an assert
LongCatIsLooong Aug 11, 2025
b6ad65a
huh
LongCatIsLooong Aug 11, 2025
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.8.4

* Removes unnecessary workarounds for detecting initialized status.

## 2.8.3

* Removes calls to self from init and dealloc, for maintainability.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -939,6 +939,31 @@ - (void)testPlayerShouldNotDropEverySecondFrame {
OCMVerifyAllWithDelay(mockTextureRegistry, 10);
}

- (void)testVideoOutputIsAddedWhenAVPlayerItemBecomesReady {
NSObject<FlutterPluginRegistrar> *registrar = OCMProtocolMock(@protocol(FlutterPluginRegistrar));
FVPVideoPlayerPlugin *videoPlayerPlugin =
[[FVPVideoPlayerPlugin alloc] initWithRegistrar:registrar];
FlutterError *error;
[videoPlayerPlugin initialize:&error];
XCTAssertNil(error);
FVPCreationOptions *create = [FVPCreationOptions
makeWithUri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/bee.mp4"
httpHeaders:@{}
viewType:FVPPlatformVideoViewTypeTextureView];

NSNumber *playerIdentifier = [videoPlayerPlugin createWithOptions:create error:&error];
FVPVideoPlayer *player = videoPlayerPlugin.playersByIdentifier[playerIdentifier];
XCTAssertNotNil(player);

AVPlayerItem *item = player.player.currentItem;
[self keyValueObservingExpectationForObject:(id)item
keyPath:@"status"
expectedValue:@(AVPlayerItemStatusReadyToPlay)];
[self waitForExpectationsWithTimeout:10.0 handler:nil];
// Video output is added as soon as the status becomes ready to play.
XCTAssertEqual(item.outputs.count, 1);
}

#if TARGET_OS_IOS
- (void)testVideoPlayerShouldNotOverwritePlayAndRecordNorDefaultToSpeaker {
NSObject<FlutterPluginRegistrar> *registrar = OCMProtocolMock(@protocol(FlutterPluginRegistrar));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@

static void *timeRangeContext = &timeRangeContext;
static void *statusContext = &statusContext;
static void *presentationSizeContext = &presentationSizeContext;
static void *durationContext = &durationContext;
static void *playbackLikelyToKeepUpContext = &playbackLikelyToKeepUpContext;
static void *rateContext = &rateContext;

Expand Down Expand Up @@ -64,8 +62,6 @@ static void FVPRemoveKeyValueObservers(NSObject *observer,
return @{
@"loadedTimeRanges" : [NSValue valueWithPointer:timeRangeContext],
@"status" : [NSValue valueWithPointer:statusContext],
@"presentationSize" : [NSValue valueWithPointer:presentationSizeContext],
@"duration" : [NSValue valueWithPointer:durationContext],
@"playbackLikelyToKeepUp" : [NSValue valueWithPointer:playbackLikelyToKeepUpContext],
};
}
Expand Down Expand Up @@ -277,14 +273,6 @@ - (void)observeValueForKeyPath:(NSString *)path
} else if (context == statusContext) {
AVPlayerItem *item = (AVPlayerItem *)object;
[self reportStatusForPlayerItem:item];
} else if (context == presentationSizeContext || context == durationContext) {
AVPlayerItem *item = (AVPlayerItem *)object;
if (item.status == AVPlayerItemStatusReadyToPlay) {
// Due to an apparent bug, when the player item is ready, it still may not have determined
// its presentation size or duration. When these properties are finally set, re-check if
// all required properties and instantiate the event sink if it is not already set up.
[self reportInitializedIfReadyToPlay];
}
} else if (context == playbackLikelyToKeepUpContext) {
[self updatePlayingState];
if ([[_player currentItem] isPlaybackLikelyToKeepUp]) {
Expand All @@ -301,6 +289,8 @@ - (void)observeValueForKeyPath:(NSString *)path
}

- (void)reportStatusForPlayerItem:(AVPlayerItem *)item {
NSAssert(self.eventListener,
@"reportStatusForPlayerItem was called when the event listener was not set.");
switch (item.status) {
case AVPlayerItemStatusFailed:
[self sendFailedToLoadVideoEvent];
Expand Down Expand Up @@ -383,52 +373,15 @@ - (void)sendFailedToLoadVideoEvent {
}

- (void)reportInitializedIfReadyToPlay {
if (!_isInitialized) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the status API guarantee that it can only transition to readyToPlay once? It seems likely that it would, but I didn't see any docs in a quick look.

AVPlayerItem *currentItem = self.player.currentItem;
CGSize size = currentItem.presentationSize;
CGFloat width = size.width;
CGFloat height = size.height;

// Wait until tracks are loaded to check duration or if there are any videos.
AVAsset *asset = currentItem.asset;
if ([asset statusOfValueForKey:@"tracks" error:nil] != AVKeyValueStatusLoaded) {
void (^trackCompletionHandler)(void) = ^{
if ([asset statusOfValueForKey:@"tracks" error:nil] != AVKeyValueStatusLoaded) {
// Cancelled, or something failed.
return;
}
// This completion block will run on an AVFoundation background queue.
// Hop back to the main thread to set up event sink.
[self performSelector:_cmd onThread:NSThread.mainThread withObject:self waitUntilDone:NO];
};
[asset loadValuesAsynchronouslyForKeys:@[ @"tracks" ]
completionHandler:trackCompletionHandler];
return;
}
AVPlayerItem *currentItem = self.player.currentItem;
NSAssert(currentItem.status == AVPlayerItemStatusReadyToPlay,
@"reportInitializedIfReadyToPlay was called when the item wasn't ready to play.");
NSAssert(!_isInitialized, @"reportInitializedIfReadyToPlay should only be called once.");

Copy link
Contributor Author

@LongCatIsLooong LongCatIsLooong Jul 7, 2025

Choose a reason for hiding this comment

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

Removed the duration / size checks since now the caller has to guarantee the item is .readyToPlay and I think those checks shouldn't be necessary.

Before this you may enter this if block even when the item's status is .unknown and I believe that's why we added the workarounds (size / duration checks).

BOOL hasVideoTracks = [asset tracksWithMediaType:AVMediaTypeVideo].count != 0;
// Audio-only HLS files have no size, so `currentItem.tracks.count` must be used to check for
// track presence, as AVAsset does not always provide track information in HLS streams.
BOOL hasNoTracks = currentItem.tracks.count == 0 && asset.tracks.count == 0;

// The player has not yet initialized when it has no size, unless it is an audio-only track.
// HLS m3u8 video files never load any tracks, and are also not yet initialized until they have
// a size.
if ((hasVideoTracks || hasNoTracks) && height == CGSizeZero.height &&
width == CGSizeZero.width) {
return;
}
// The player may be initialized but still needs to determine the duration.
int64_t duration = [self duration];
if (duration == 0) {
return;
}

_isInitialized = YES;
[self updatePlayingState];

[self.eventListener videoPlayerDidInitializeWithDuration:duration size:size];
}
_isInitialized = YES;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

looks like the flag is still used by the updatePlayingState method.

Copy link
Contributor

Choose a reason for hiding this comment

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

If _isInitialized == true always implies that player status is ready to play then maybe updateRate can be simplified. Maybe updatePlayingState could instead check that player status is ready to play (canPlayFastForward and canPlaySlowForward do not work when status is not ready to play). Btw was not _isInitialized set to false in onCancelWithArguments before?

[self updatePlayingState];
[self.eventListener videoPlayerDidInitializeWithDuration:self.duration
size:currentItem.presentationSize];
}

#pragma mark - FVPVideoPlayerInstanceApi
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,14 @@ NS_ASSUME_NONNULL_BEGIN
@property(nonatomic, readonly) NSNumber *targetPlaybackSpeed;
/// Indicates whether the video player is currently playing.
@property(nonatomic, readonly) BOOL isPlaying;
/// Indicates whether the video player has been initialized.
/// Indicates whether an "initialized" message has been sent to the current Flutter event listener.
///
/// The video player sends an "initialized" message to the event listener when its underlying
/// AVPlayerItem is ready to play and the event listener is set to a non-nil value, whichever occurs
/// last.
///
/// This flag is set to YES when the "initialized" message is first sent, and is never set to NO
/// again.
@property(nonatomic, readonly) BOOL isInitialized;

/// Initializes a new instance of FVPVideoPlayer with the given AVPlayerItem, frame updater, display
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import 'dart:async';
import 'dart:io';
import 'dart:math' as math show max;

import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
Expand Down Expand Up @@ -526,29 +527,27 @@ class _VideoProgressIndicatorState extends State<VideoProgressIndicator> {
const Color bufferedColor = Color.fromRGBO(50, 50, 200, 0.2);
const Color backgroundColor = Color.fromRGBO(200, 200, 200, 0.5);

Widget progressIndicator;
final Widget progressIndicator;
if (controller.value.isInitialized) {
final int duration = controller.value.duration.inMilliseconds;
final int position = controller.value.position.inMilliseconds;

int maxBuffering = 0;
for (final DurationRange range in controller.value.buffered) {
final int end = range.end.inMilliseconds;
if (end > maxBuffering) {
maxBuffering = end;
}
}

final double maxBuffering = duration == 0.0
? 0.0
: controller.value.buffered
.map((DurationRange range) => range.end.inMilliseconds)
.fold(0, math.max) /
duration;
progressIndicator = Stack(
fit: StackFit.passthrough,
children: <Widget>[
LinearProgressIndicator(
value: maxBuffering / duration,
value: maxBuffering,
valueColor: const AlwaysStoppedAnimation<Color>(bufferedColor),
backgroundColor: backgroundColor,
),
LinearProgressIndicator(
value: position / duration,
value: duration == 0.0 ? 0.0 : position / duration,
valueColor: const AlwaysStoppedAnimation<Color>(playedColor),
backgroundColor: Colors.transparent,
),
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.8.3
version: 2.8.4

environment:
sdk: ^3.6.0
Expand Down