From aeecebc6e760ebaf80dc35f189638bb148c1ac4a Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 2 Oct 2024 10:49:22 -0700 Subject: [PATCH] Background/resuming `video_player` on Android sends one `initialized` event (#7722) Closes https://github.com/flutter/flutter/issues/154602. I also added small clarifications to the error in `video_player` (an assertion/error that are better than a `Completer.complete` default error) and updated the documentation in `video_player_platform` (it might have been obvious, but it definitely is now). --- .../video_player/video_player/CHANGELOG.md | 4 +- .../video_player/lib/video_player.dart | 10 ++++ .../video_player/video_player/pubspec.yaml | 2 +- .../video_player_android/CHANGELOG.md | 7 ++- .../videoplayer/ExoPlayerEventListener.java | 7 ++- .../plugins/videoplayer/VideoPlayer.java | 4 +- .../plugins/videoplayer/VideoPlayerTest.java | 49 +++++++++++++++++++ .../video_player_android/pubspec.yaml | 2 +- .../CHANGELOG.md | 3 +- .../lib/video_player_platform_interface.dart | 2 + .../pubspec.yaml | 2 +- 11 files changed, 83 insertions(+), 9 deletions(-) diff --git a/packages/video_player/video_player/CHANGELOG.md b/packages/video_player/video_player/CHANGELOG.md index c41c8f131fdf..5e448be001b3 100644 --- a/packages/video_player/video_player/CHANGELOG.md +++ b/packages/video_player/video_player/CHANGELOG.md @@ -1,6 +1,8 @@ -## NEXT +## 2.9.2 * Updates minimum supported SDK version to Flutter 3.19/Dart 3.3. +* Throws a more descriptive `StateError` in the case where + `VideoPlayerController.initialize` receives more than one `initialized` event. ## 2.9.1 diff --git a/packages/video_player/video_player/lib/video_player.dart b/packages/video_player/video_player/lib/video_player.dart index c8acd6e969b5..b7ba8340fa66 100644 --- a/packages/video_player/video_player/lib/video_player.dart +++ b/packages/video_player/video_player/lib/video_player.dart @@ -465,6 +465,16 @@ class VideoPlayerController extends ValueNotifier { errorDescription: null, isCompleted: false, ); + assert( + !initializingCompleter.isCompleted, + 'VideoPlayerController already initialized. This is typically a ' + 'sign that an implementation of the VideoPlayerPlatform ' + '(${_videoPlayerPlatform.runtimeType}) has a bug and is sending ' + 'more than one initialized event per instance.', + ); + if (initializingCompleter.isCompleted) { + throw StateError('VideoPlayerController already initialized'); + } initializingCompleter.complete(null); _applyLooping(); _applyVolume(); diff --git a/packages/video_player/video_player/pubspec.yaml b/packages/video_player/video_player/pubspec.yaml index 7ee58c198416..6e8b92684d7c 100644 --- a/packages/video_player/video_player/pubspec.yaml +++ b/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, 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.9.1 +version: 2.9.2 environment: sdk: ^3.3.0 diff --git a/packages/video_player/video_player_android/CHANGELOG.md b/packages/video_player/video_player_android/CHANGELOG.md index ec3069bd07ba..f0623caa96d6 100644 --- a/packages/video_player/video_player_android/CHANGELOG.md +++ b/packages/video_player/video_player_android/CHANGELOG.md @@ -1,3 +1,8 @@ +## 2.7.6 + +* Fixes a [bug](https://github.com/flutter/flutter/issues/154602) where + resuming a video player would cause a `Bad state: Future already completed`. + ## 2.7.5 * Add a deprecation suppression in advance of a new `SurfaceProducer` API. @@ -18,12 +23,10 @@ * Re-adds Impeller support. - ## 2.7.1 * Revert Impeller support. - ## 2.7.0 * Re-adds [support for Impeller](https://docs.flutter.dev/release/breaking-changes/android-surface-plugins). diff --git a/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/ExoPlayerEventListener.java b/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/ExoPlayerEventListener.java index 0940cc8b3227..c8f2816571de 100644 --- a/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/ExoPlayerEventListener.java +++ b/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/ExoPlayerEventListener.java @@ -14,11 +14,16 @@ final class ExoPlayerEventListener implements Player.Listener { private final ExoPlayer exoPlayer; private final VideoPlayerCallbacks events; private boolean isBuffering = false; - private boolean isInitialized = false; + private boolean isInitialized; ExoPlayerEventListener(ExoPlayer exoPlayer, VideoPlayerCallbacks events) { + this(exoPlayer, events, false); + } + + ExoPlayerEventListener(ExoPlayer exoPlayer, VideoPlayerCallbacks events, boolean initialized) { this.exoPlayer = exoPlayer; this.events = events; + this.isInitialized = initialized; } private void setBuffering(boolean buffering) { diff --git a/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java b/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java index 53c584fc29e7..c8afd2ebc728 100644 --- a/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java +++ b/packages/video_player/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java @@ -108,7 +108,9 @@ private ExoPlayer createVideoPlayer() { exoPlayer.prepare(); exoPlayer.setVideoSurface(surfaceProducer.getSurface()); - exoPlayer.addListener(new ExoPlayerEventListener(exoPlayer, videoPlayerEvents)); + + boolean wasInitialized = savedStateDuring != null; + exoPlayer.addListener(new ExoPlayerEventListener(exoPlayer, videoPlayerEvents, wasInitialized)); setAudioAttributes(exoPlayer, options.mixWithOthers); return exoPlayer; diff --git a/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java b/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java index 2cf0193df7b4..c968848cbbe3 100644 --- a/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java +++ b/packages/video_player/video_player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java @@ -13,6 +13,7 @@ import androidx.media3.common.C; import androidx.media3.common.PlaybackParameters; import androidx.media3.common.Player; +import androidx.media3.common.VideoSize; import androidx.media3.exoplayer.ExoPlayer; import io.flutter.view.TextureRegistry; import org.junit.Before; @@ -48,6 +49,7 @@ public final class VideoPlayerTest { @Mock private ExoPlayer mockExoPlayer; @Captor private ArgumentCaptor attributesCaptor; @Captor private ArgumentCaptor callbackCaptor; + @Captor private ArgumentCaptor listenerCaptor; @Rule public MockitoRule initRule = MockitoJUnit.rule(); @@ -197,6 +199,53 @@ public void onSurfaceProducerDestroyedAndRecreatedReleasesAndThenRecreatesAndRes videoPlayer.dispose(); } + @Test + public void onInitializedCalledWhenVideoPlayerInitiallyCreated() { + VideoPlayer videoPlayer = createVideoPlayer(); + + // Pretend we have a video, and capture the registered event listener. + when(mockExoPlayer.getVideoSize()).thenReturn(new VideoSize(300, 200)); + verify(mockExoPlayer).addListener(listenerCaptor.capture()); + Player.Listener listener = listenerCaptor.getValue(); + + // Trigger an event that would trigger onInitialized. + listener.onPlaybackStateChanged(Player.STATE_READY); + verify(mockEvents).onInitialized(anyInt(), anyInt(), anyLong(), anyInt()); + + videoPlayer.dispose(); + } + + @Test + public void onSurfaceCreatedDoesNotSendInitializeEventAgain() { + // The VideoPlayer contract assumes that the event "initialized" is sent exactly once + // (duplicate events cause an error to be thrown at the shared Dart layer). This test verifies + // that the onInitialized event is sent exactly once per player. + // + // Regression test for https://github.com/flutter/flutter/issues/154602. + VideoPlayer videoPlayer = createVideoPlayer(); + when(mockExoPlayer.getVideoSize()).thenReturn(new VideoSize(300, 200)); + + // Capture the lifecycle events so we can simulate onSurfaceCreated/Destroyed. + verify(mockProducer).setCallback(callbackCaptor.capture()); + TextureRegistry.SurfaceProducer.Callback producerLifecycle = callbackCaptor.getValue(); + + // Trigger destroyed/created. + producerLifecycle.onSurfaceDestroyed(); + producerLifecycle.onSurfaceCreated(); + + // Initial listener, and the new one from the resume. + verify(mockExoPlayer, times(2)).addListener(listenerCaptor.capture()); + Player.Listener listener = listenerCaptor.getValue(); + + // Now trigger that same event, which would happen in the case of a background/resume. + listener.onPlaybackStateChanged(Player.STATE_READY); + + // Was not called because it was a result of a background/resume. + verify(mockEvents, never()).onInitialized(anyInt(), anyInt(), anyLong(), anyInt()); + + videoPlayer.dispose(); + } + @Test public void onSurfaceCreatedWithoutDestroyDoesNotRecreate() { // Initially create the video player, which creates the initial surface. diff --git a/packages/video_player/video_player_android/pubspec.yaml b/packages/video_player/video_player_android/pubspec.yaml index cbf557252b20..1ab81efd9dcd 100644 --- a/packages/video_player/video_player_android/pubspec.yaml +++ b/packages/video_player/video_player_android/pubspec.yaml @@ -2,7 +2,7 @@ name: video_player_android description: Android implementation of the video_player plugin. repository: https://github.com/flutter/packages/tree/main/packages/video_player/video_player_android issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+video_player%22 -version: 2.7.5 +version: 2.7.6 environment: sdk: ^3.5.0 diff --git a/packages/video_player/video_player_platform_interface/CHANGELOG.md b/packages/video_player/video_player_platform_interface/CHANGELOG.md index 03f53195175e..471cde8298d4 100644 --- a/packages/video_player/video_player_platform_interface/CHANGELOG.md +++ b/packages/video_player/video_player_platform_interface/CHANGELOG.md @@ -1,6 +1,7 @@ -## NEXT +## 6.2.3 * Updates minimum supported SDK version to Flutter 3.19/Dart 3.3. +* Clarified that `VideoEventType.initialized` cannot be sent more than once. ## 6.2.2 diff --git a/packages/video_player/video_player_platform_interface/lib/video_player_platform_interface.dart b/packages/video_player/video_player_platform_interface/lib/video_player_platform_interface.dart index 46b992f58c61..d169c5f16d45 100644 --- a/packages/video_player/video_player_platform_interface/lib/video_player_platform_interface.dart +++ b/packages/video_player/video_player_platform_interface/lib/video_player_platform_interface.dart @@ -278,6 +278,8 @@ class VideoEvent { /// completed or to communicate buffering events or play state changed. enum VideoEventType { /// The video has been initialized. + /// + /// A maximum of one event of this type may be emitted per instance. initialized, /// The playback has ended. diff --git a/packages/video_player/video_player_platform_interface/pubspec.yaml b/packages/video_player/video_player_platform_interface/pubspec.yaml index 89a8f55702d9..7ed49e9e2d3d 100644 --- a/packages/video_player/video_player_platform_interface/pubspec.yaml +++ b/packages/video_player/video_player_platform_interface/pubspec.yaml @@ -4,7 +4,7 @@ repository: https://github.com/flutter/packages/tree/main/packages/video_player/ issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+video_player%22 # NOTE: We strongly prefer non-breaking changes, even at the expense of a # less-clean API. See https://flutter.dev/go/platform-interface-breaking-changes -version: 6.2.2 +version: 6.2.3 environment: sdk: ^3.3.0