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

Fixes flutter/flutter#166914. The duration check was first introduced [here](flutter/plugins#1554) 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](https://github.com/flutter/plugins/pull/280/files#diff-f4c7ef029bb5e0be8a6851b6b022c745dc4e99e3739f1c2f7f4944b85bc154a6R174-R178). 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](flutter/plugins#1552) 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](https://developer.apple.com/library/archive/samplecode/AVPlayerDemo/Listings/Classes_AVPlayerDemoPlaybackViewController_m.html#//apple_ref/doc/uid/DTS40010101-Classes_AVPlayerDemoPlaybackViewController_m-DontLinkElementID_8)). 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

[^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.

Author: LongCatIsLooong

packages/video_player/video_player/CHANGELOG.md

@@ -1,5 +1,7 @@
-## NEXT
+## 2.10.1
 
+* Fixes a bug where the `VideoPlayer` widget and `VideoProgressIndicator` widget would stop updating after GlobalKey reparenting.
+* Updates the `VideoProgressIndicator` widget to handle zero-duration videos.
 * Updates minimum supported SDK version to Flutter 3.29/Dart 3.7.
 
 ## 2.10.0

packages/video_player/video_player/lib/video_player.dart

@@ -4,6 +4,7 @@
 
 import 'dart:async';
 import 'dart:io';
+import 'dart:math' as math show max;
 
 import 'package:flutter/foundation.dart';
 import 'package:flutter/material.dart';
@@ -79,7 +80,7 @@ class VideoPlayerValue {
 
   /// The total duration of the video.
   ///
-  /// The duration is [Duration.zero] if the video hasn't been initialized.
+  /// The value is only meaningful when [isInitialized] is true.
   final Duration duration;
 
   /// The current playback position.
@@ -430,7 +431,7 @@ class VideoPlayerController extends ValueNotifier<VideoPlayerValue> {
     _lifeCycleObserver?.initialize();
     _creatingCompleter = Completer<void>();
 
-    late DataSource dataSourceDescription;
+    final DataSource dataSourceDescription;
     switch (dataSourceType) {
       case DataSourceType.asset:
         dataSourceDescription = DataSource(
@@ -863,42 +864,37 @@ class VideoPlayer extends StatefulWidget {
 }
 
 class _VideoPlayerState extends State<VideoPlayer> {
-  _VideoPlayerState() {
-    _listener = () {
-      final int newPlayerId = widget.controller.playerId;
-      if (newPlayerId != _playerId) {
-        setState(() {
-          _playerId = newPlayerId;
-        });
-      }
-    };
-  }
-
-  late VoidCallback _listener;
-
   late int _playerId;
+  void _controllerDidUpdateValue() {
+    final int newPlayerId = widget.controller.playerId;
+    if (newPlayerId != _playerId) {
+      setState(() {
+        _playerId = newPlayerId;
+      });
+    }
+  }
 
   @override
   void initState() {
     super.initState();
     _playerId = widget.controller.playerId;
     // Need to listen for initialization events since the actual widget ID
     // becomes available after asynchronous initialization finishes.
-    widget.controller.addListener(_listener);
+    widget.controller.addListener(_controllerDidUpdateValue);
   }
 
   @override
   void didUpdateWidget(VideoPlayer oldWidget) {
     super.didUpdateWidget(oldWidget);
-    oldWidget.controller.removeListener(_listener);
+    oldWidget.controller.removeListener(_controllerDidUpdateValue);
     _playerId = widget.controller.playerId;
-    widget.controller.addListener(_listener);
+    widget.controller.addListener(_controllerDidUpdateValue);
   }
 
   @override
-  void deactivate() {
-    super.deactivate();
-    widget.controller.removeListener(_listener);
+  void dispose() {
+    widget.controller.removeListener(_controllerDidUpdateValue);
+    super.dispose();
   }
 
   @override
@@ -1089,58 +1085,52 @@ class VideoProgressIndicator extends StatefulWidget {
 }
 
 class _VideoProgressIndicatorState extends State<VideoProgressIndicator> {
-  _VideoProgressIndicatorState() {
-    listener = () {
-      if (!mounted) {
-        return;
-      }
-      setState(() {});
-    };
-  }
-
-  late VoidCallback listener;
-
   VideoPlayerController get controller => widget.controller;
 
   VideoProgressColors get colors => widget.colors;
 
+  void _didUpdateControllerValue() {
+    setState(() {
+      // The build method reads from controller.value.
+    });
+  }
+
   @override
   void initState() {
     super.initState();
-    controller.addListener(listener);
+    controller.addListener(_didUpdateControllerValue);
   }
 
   @override
-  void deactivate() {
-    controller.removeListener(listener);
-    super.deactivate();
+  void dispose() {
+    controller.removeListener(_didUpdateControllerValue);
+    super.dispose();
   }
 
   @override
   Widget build(BuildContext context) {
-    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: AlwaysStoppedAnimation<Color>(colors.bufferedColor),
             backgroundColor: colors.backgroundColor,
           ),
           LinearProgressIndicator(
-            value: position / duration,
+            value: duration == 0.0 ? 0.0 : position / duration,
             valueColor: AlwaysStoppedAnimation<Color>(colors.playedColor),
             backgroundColor: Colors.transparent,
           ),

packages/video_player/video_player/pubspec.yaml

@@ -3,7 +3,7 @@ description: Flutter plugin for displaying inline video with other Flutter
   widgets on Android, iOS, macOS and web.
 repository: https://github.com/flutter/packages/tree/main/packages/video_player/video_player
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+video_player%22
-version: 2.10.0
+version: 2.10.1
 
 environment:
   sdk: ^3.7.0

packages/video_player/video_player/test/video_player_test.dart

@@ -172,6 +172,87 @@ void main() {
     );
   });
 
+  testWidgets(
+    'VideoPlayer still listens for controller changes when reparented',
+    (WidgetTester tester) async {
+      final FakeController controller = FakeController();
+      addTearDown(controller.dispose);
+      final GlobalKey videoKey = GlobalKey();
+      final Widget videoPlayer = KeyedSubtree(
+        key: videoKey,
+        child: VideoPlayer(controller),
+      );
+
+      await tester.pumpWidget(videoPlayer);
+      expect(find.byType(Texture), findsNothing);
+
+      // The VideoPlayer is reparented in the widget tree, before the
+      // underlying player is initialized.
+      await tester.pumpWidget(SizedBox(child: videoPlayer));
+      controller.playerId = 321;
+      controller.value = controller.value.copyWith(
+        duration: const Duration(milliseconds: 100),
+        isInitialized: true,
+      );
+
+      await tester.pump();
+      expect(
+        find.byWidgetPredicate(
+          (Widget widget) => widget is Texture && widget.textureId == 321,
+        ),
+        findsOneWidget,
+      );
+    },
+  );
+
+  testWidgets(
+    'VideoProgressIndicator still listens for controller changes after reparenting',
+    (WidgetTester tester) async {
+      final FakeController controller = FakeController();
+      addTearDown(controller.dispose);
+      final GlobalKey key = GlobalKey();
+      final Widget progressIndicator = VideoProgressIndicator(
+        key: key,
+        controller,
+        allowScrubbing: false,
+      );
+
+      controller.value = controller.value.copyWith(
+        duration: const Duration(milliseconds: 100),
+        position: const Duration(milliseconds: 50),
+        isInitialized: true,
+      );
+      await tester.pumpWidget(MaterialApp(home: progressIndicator));
+      await tester.pump();
+      await tester.pumpWidget(
+        MaterialApp(home: SizedBox(child: progressIndicator)),
+      );
+      expect((key.currentContext! as Element).dirty, isFalse);
+      // Verify that changing value dirties the widget tree.
+      controller.value = controller.value.copyWith(
+        position: const Duration(milliseconds: 100),
+      );
+      expect((key.currentContext! as Element).dirty, isTrue);
+    },
+  );
+
+  testWidgets('VideoPlayer does not crash after loading 0-duration videos', (
+    WidgetTester tester,
+  ) async {
+    final FakeController controller = FakeController();
+    addTearDown(controller.dispose);
+    controller.value = controller.value.copyWith(
+      duration: Duration.zero,
+      isInitialized: true,
+    );
+    await tester.pumpWidget(
+      MaterialApp(
+        home: VideoProgressIndicator(controller, allowScrubbing: false),
+      ),
+    );
+    expect(tester.takeException(), isNull);
+  });
+
   testWidgets('non-zero rotationCorrection value is used', (
     WidgetTester tester,
   ) async {

packages/video_player/video_player_android/example/lib/mini_controller.dart

@@ -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';
@@ -433,9 +434,9 @@ class _VideoPlayerState extends State<VideoPlayer> {
   }
 
   @override
-  void deactivate() {
-    super.deactivate();
+  void dispose() {
     widget.controller.removeListener(_listener);
+    super.dispose();
   }
 
   @override
@@ -535,9 +536,9 @@ class _VideoProgressIndicatorState extends State<VideoProgressIndicator> {
   }
 
   @override
-  void deactivate() {
+  void dispose() {
     controller.removeListener(listener);
-    super.deactivate();
+    super.dispose();
   }
 
   @override
@@ -546,29 +547,28 @@ 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,
           ),

packages/video_player/video_player_avfoundation/CHANGELOG.md

@@ -1,3 +1,9 @@
+## 2.8.6
+
+* Fixes a bug where the video player fails to initialize when `AVFoundation` reports a duration of zero.
+* Fixes a bug in the example app that some widgets stop updating after GlobalKey reparenting.
+* Updates the `VideoProgressIndicator` widget in the example app to handle zero-duration videos.
+
 ## 2.8.5
 
 * Updates minimum supported version to iOS 13 and macOS 10.15.

packages/video_player/video_player_avfoundation/darwin/RunnerTests/VideoPlayerTests.m

@@ -936,6 +936,33 @@ - (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:@{}];
+
+  FVPTexturePlayerIds *identifiers = [videoPlayerPlugin createTexturePlayerWithOptions:create
+                                                                                 error:&error];
+  XCTAssertNil(error);
+  XCTAssertNotNil(identifiers);
+  FVPVideoPlayer *player = videoPlayerPlugin.playersByIdentifier[@(identifiers.playerId)];
+  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));