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

LongCatIsLooong · 2025-06-30T22:06:00Z

Fixes flutter/flutter#166914. The duration check was first introduced here but zero division should probably be handled by the client not the plugin.

Also I suspect that flutter/flutter#91975 wasn't caused by an AVFoundation bug but rather a bug in video_player_avfoundation:

The _isInitialized flag was roughly equivalent to .readyToPlay a long time ago. The flag was later re-purposed to indicate whether an initialized message has already been sent (to make sure the message is only sent once), and that PR also introduced a size check to determine whether the media is loaded (which doesn't seem to be correct, .readyToPlay is the public API for this, see Apple's AVPlayerDemo). But that PR didn't update the onListenWithArguments implementation so the plugin started sending initialized events on first subscription even the AVPlayerItem isn't ready to play. So that looks like a bug in the plugin not in AVFoundation and that allows us to remove a bunch of workarounds.

Couldn't reproduce flutter/flutter#91975 on iOS16 with this patch (I can't install iOS15 sim runtime, it fails silently, but on iOS16 it still hits the size + duration check callpath so I think not much has changed from iOS15 to iOS16 as far as AVFoundation stuff is concerned).
I didn't try to reproduce flutter/flutter#19197 with this patch since the issue doesn't have a minimal repro. I did not change the _isInitialized semantic so I assume the patch won't regress that fix.

More on isInitialized:
The player implementation doesn't have to rely on this property, if AVPlayerItem.Status can not recover from .failed to .readyToPlay (i.e. it can only change to .readyToPlay once). But the documentation doesn't say if that is the case.

Pre-Review Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩² ↩³

…d event to the plugin

…oPlay

LongCatIsLooong · 2025-07-07T23:46:06Z

I'm still trying to generate a sample video with 0 duration (which seems to be an AVFoundation bug as the video plays in chrome, but not in quicktime) so I can add a test. But much of this change should already be covered by existing tests added for the workarounds in the past.

LongCatIsLooong · 2025-07-07T23:53:34Z

...undation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayer.m

+  CGSize size = currentItem.presentationSize;
+  CGFloat width = size.width;
+  CGFloat height = size.height;
+


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).

...undation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayer.m

hellohuanlin · 2025-07-09T20:29:32Z

packages/video_player/video_player_avfoundation/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 2.8.1
+
+* Removes unnecessary workarounds, fixes "initialized" event not firing when the duration of the media is 0.


Removes unnecessary workarounds

What workarounds?

The size / duration checks removed in the sendInitialized method.

It's unclear from reading the CHANGELOG.

misos1 · 2025-07-10T17:22:50Z

...undation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayer.m

      case AVPlayerItemStatusUnknown:
        break;
      case AVPlayerItemStatusReadyToPlay:
        [item addOutput:_videoOutput];


If status is changed to AVPlayerItemStatusReadyToPlay before the event sink is set up then it seems this addOutput will not be called.

Oops thanks for catching this!

…oPlay

LongCatIsLooong · 2025-07-18T00:15:25Z

/gemini review

gemini-code-assist

Code Review

This pull request simplifies the video initialization logic by correctly using the AVPlayerItem.status. The removal of workarounds makes the code cleaner and easier to understand. The added test and updated documentation are also valuable contributions.

I've added a few suggestions to further improve robustness and maintainability in the Objective-C code, mainly around defensive checks and handling of enum values in switch statements. Overall, this is a solid improvement.

gemini-code-assist · 2025-07-18T00:17:42Z

...undation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayer.m

+  switch (self.player.currentItem.status) {
+    case AVPlayerItemStatusUnknown:
+      // When this method is called when the media is still loading, do nothing:
+      // sendVideoInitializedEvent or sendFailedToLoadVideoEvent will be called
+      // by KVO on status updates.
+      return nil;
+    case AVPlayerItemStatusReadyToPlay:
+      [self sendVideoInitializedEvent];
+      return nil;
+    case AVPlayerItemStatusFailed:
+      [self sendFailedToLoadVideoEvent];
+      return nil;
  }
-  [self setupEventSinkIfReadyToPlay];
-  return nil;
 }


This function is declared to return a FlutterError * (or nil), but it lacks a default case in the switch statement. If self.player.currentItem.status has a value not covered by the case statements (e.g., if the enum is extended in a future SDK), this function would have undefined behavior. Adding a default case that returns nil will fix this correctness issue.

switch (self.player.currentItem.status) { case AVPlayerItemStatusUnknown: // When this method is called when the media is still loading, do nothing: // sendVideoInitializedEvent or sendFailedToLoadVideoEvent will be called // by KVO on status updates. return nil; case AVPlayerItemStatusReadyToPlay: [self sendVideoInitializedEvent]; return nil; case AVPlayerItemStatusFailed: [self sendFailedToLoadVideoEvent]; return nil; default: NSAssert(NO, @"Unknown AVPlayerItemStatus: %ld", (long)self.player.currentItem.status); return nil; }

gemini-code-assist · 2025-07-18T00:17:42Z

...undation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayer.m

      case AVPlayerItemStatusUnknown:
        break;
      case AVPlayerItemStatusReadyToPlay:
        [item addOutput:_videoOutput];


To improve robustness, it's safer to check if the video output has already been added to the player item before calling addOutput:. This check prevents potential issues if the player item status transitions in unexpected ways.

if (![item.outputs containsObject:_videoOutput]) { [item addOutput:_videoOutput]; }

…oPlay

LongCatIsLooong · 2025-09-25T23:43:35Z

packages/video_player/video_player/lib/video_player.dart

-  void deactivate() {
-    super.deactivate();
-    widget.controller.removeListener(_listener);
+  void dispose() {


drive-by fix: deactivate is called either when the state is going to be reparented or disposed.

LongCatIsLooong · 2025-09-25T23:44:54Z

packages/video_player/video_player/lib/video_player.dart


+  void _didUpdateControllerValue() {
+    setState(() {
+      // The build method reads from controller.value.


The mounted check wasn't necessary. The state will stop listening when the element is unmounted.

This reverts commit ddf7236.

LongCatIsLooong · 2025-09-30T17:42:03Z

Ready for review:

Added an if to prevent "initialized" from being sent for more than once, in case AVPlayer can enter .readyToPlay state more than once. (However as a result the dart implementation won't be able to recover from a failed state or update size/duration in case .readyToPlay can be sent a second time).
Fixes widget implementation in the dart code.

LongCatIsLooong added 3 commits June 30, 2025 13:32

Make sure the AVPlayerItem is ready to play before sending initialize…

42a5ba4

…d event to the plugin

Merge remote-tracking branch 'upstream/main' into video_player_readyT…

375ec1f

…oPlay

wuh

bb89c41

github-actions bot added platform-ios platform-macos p: video_player labels Jun 30, 2025

LongCatIsLooong added 3 commits June 30, 2025 15:15

docs

1ad1d2e

example app and README.md

63d240b

format

0cfe3f4

LongCatIsLooong changed the title ~~Make sure the AVPlayerItem is .readyToPlay before emitting an initialized event~~ [video_player_avfoundation] Make sure the AVPlayerItem is .readyToPlay before emitting an initialized event Jul 1, 2025

LongCatIsLooong added 4 commits July 7, 2025 15:34

Update CHANGELOG.md

60fc5d2

Merge branch 'main' into video_player_readyToPlay

f3fbff5

Update pubspec.yaml

bcc2725

Update CHANGELOG.md

733f5ea

LongCatIsLooong marked this pull request as ready for review July 7, 2025 23:46

LongCatIsLooong requested a review from hellohuanlin as a code owner July 7, 2025 23:46

LongCatIsLooong commented Jul 7, 2025

View reviewed changes

LongCatIsLooong added 2 commits July 8, 2025 13:08

Merge branch 'main' into video_player_readyToPlay

c86f933

Update pubspec.yaml

35c9a08

LongCatIsLooong commented Jul 8, 2025

View reviewed changes

...undation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayer.m Outdated Show resolved Hide resolved

Merge branch 'main' into video_player_readyToPlay

3b0cd77

hellohuanlin reviewed Jul 9, 2025

View reviewed changes

misos1 reviewed Jul 10, 2025

View reviewed changes

LongCatIsLooong added 3 commits July 10, 2025 15:13

oops

4315c4a

Merge remote-tracking branch 'upstream/main' into video_player_readyT…

47ff5d0

…oPlay

add a test

e2ce9fd

gemini-code-assist bot reviewed Jul 18, 2025

View reviewed changes

gemini review

6f232f5

LongCatIsLooong requested review from ash2moon, tarrinneal and chunhtai as code owners September 25, 2025 23:20

github-actions bot added p: go_router platform-android p: webview_flutter labels Sep 25, 2025

LongCatIsLooong marked this pull request as draft September 25, 2025 23:21

Merge remote-tracking branch 'upstream/main' into video_player_readyT…

349f248

…oPlay

LongCatIsLooong force-pushed the video_player_readyToPlay branch from 01ded58 to 349f248 Compare September 25, 2025 23:37

github-actions bot removed p: go_router platform-android p: webview_flutter labels Sep 25, 2025

review

48ecaec

github-actions bot added the platform-android label Sep 25, 2025

LongCatIsLooong commented Sep 25, 2025

View reviewed changes

LongCatIsLooong added 4 commits September 25, 2025 17:16

buh

ddf7236

Revert "buh"

a74417f

This reverts commit ddf7236.

reformat

0d58f8c

version

42cc71f

LongCatIsLooong removed request for bparrishMines, chunhtai, tarrinneal and ash2moon September 27, 2025 00:15

LongCatIsLooong added 3 commits September 29, 2025 11:15

fix test

04d3a83

fix test

ebe1856

buh

f3eeb7b

LongCatIsLooong marked this pull request as ready for review September 30, 2025 17:30

LongCatIsLooong requested a review from stuartmorgan-g September 30, 2025 17:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

LongCatIsLooong commented Jun 30, 2025 •

edited

Loading

Uh oh!

LongCatIsLooong commented Jul 7, 2025

Uh oh!

LongCatIsLooong Jul 7, 2025 •

edited

Loading

Uh oh!

Uh oh!

hellohuanlin Jul 9, 2025

Uh oh!

LongCatIsLooong Jul 9, 2025 •

edited

Loading

Uh oh!

hellohuanlin Jul 29, 2025 •

edited

Loading

Uh oh!

misos1 Jul 10, 2025

Uh oh!

LongCatIsLooong Jul 10, 2025

Uh oh!

LongCatIsLooong commented Jul 18, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Jul 18, 2025

Uh oh!

gemini-code-assist bot Jul 18, 2025

Uh oh!

LongCatIsLooong Sep 25, 2025

Uh oh!

LongCatIsLooong Sep 25, 2025

Uh oh!

LongCatIsLooong commented Sep 30, 2025

Uh oh!

Uh oh!

		@@ -1,3 +1,7 @@
		## 2.8.1

		* Removes unnecessary workarounds, fixes "initialized" event not firing when the duration of the media is 0.

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

Are you sure you want to change the base?

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

Uh oh!

Conversation

LongCatIsLooong commented Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-Review Checklist

Footnotes

Uh oh!

LongCatIsLooong commented Jul 7, 2025

Uh oh!

LongCatIsLooong Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hellohuanlin Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hellohuanlin Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

misos1 Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong commented Jul 18, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong commented Sep 30, 2025

Uh oh!

Uh oh!

LongCatIsLooong commented Jun 30, 2025 •

edited

Loading

LongCatIsLooong Jul 7, 2025 •

edited

Loading

LongCatIsLooong Jul 9, 2025 •

edited

Loading

hellohuanlin Jul 29, 2025 •

edited

Loading