-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[video_player_android] Avoid sending unset duration on initialization #11709
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,16 +4,30 @@ | |||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| package io.flutter.plugins.videoplayer; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| import android.os.Handler; | ||||||||||||||||||||||||||||||
| import android.os.Looper; | ||||||||||||||||||||||||||||||
| import androidx.annotation.NonNull; | ||||||||||||||||||||||||||||||
| import androidx.annotation.Nullable; | ||||||||||||||||||||||||||||||
| import androidx.media3.common.C; | ||||||||||||||||||||||||||||||
| import androidx.media3.common.PlaybackException; | ||||||||||||||||||||||||||||||
| import androidx.media3.common.Player; | ||||||||||||||||||||||||||||||
| import androidx.media3.common.Timeline; | ||||||||||||||||||||||||||||||
| import androidx.media3.common.Tracks; | ||||||||||||||||||||||||||||||
| import androidx.media3.exoplayer.ExoPlayer; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| public abstract class ExoPlayerEventListener implements Player.Listener { | ||||||||||||||||||||||||||||||
| private static final long DURATION_UNSET_INITIALIZATION_TIMEOUT_MS = 500; | ||||||||||||||||||||||||||||||
| private boolean isInitialized = false; | ||||||||||||||||||||||||||||||
| private boolean isWaitingForValidDuration = false; | ||||||||||||||||||||||||||||||
| private final Handler mainHandler = new Handler(Looper.getMainLooper()); | ||||||||||||||||||||||||||||||
| private final Runnable initializationFallback = | ||||||||||||||||||||||||||||||
| () -> { | ||||||||||||||||||||||||||||||
| if (!isInitialized && isWaitingForValidDuration) { | ||||||||||||||||||||||||||||||
| isWaitingForValidDuration = false; | ||||||||||||||||||||||||||||||
| isInitialized = true; | ||||||||||||||||||||||||||||||
| sendInitialized(); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||
| protected final ExoPlayer exoPlayer; | ||||||||||||||||||||||||||||||
| protected final VideoPlayerCallbacks events; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
|
@@ -51,6 +65,39 @@ public ExoPlayerEventListener( | |||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| protected abstract void sendInitialized(); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| /** Cancels pending initialization callbacks when the player is disposed. */ | ||||||||||||||||||||||||||||||
| public void dispose() { | ||||||||||||||||||||||||||||||
| isWaitingForValidDuration = false; | ||||||||||||||||||||||||||||||
| mainHandler.removeCallbacks(initializationFallback); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| private boolean hasValidDuration() { | ||||||||||||||||||||||||||||||
| return exoPlayer.getDuration() != C.TIME_UNSET; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| private boolean shouldWaitForValidDuration() { | ||||||||||||||||||||||||||||||
| return !exoPlayer.isCurrentMediaItemLive() && !exoPlayer.isCurrentMediaItemDynamic(); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| private void maybeSendInitialized() { | ||||||||||||||||||||||||||||||
| if (isInitialized) { | ||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if (!hasValidDuration() && shouldWaitForValidDuration()) { | ||||||||||||||||||||||||||||||
| if (!isWaitingForValidDuration) { | ||||||||||||||||||||||||||||||
| isWaitingForValidDuration = true; | ||||||||||||||||||||||||||||||
| mainHandler.postDelayed(initializationFallback, DURATION_UNSET_INITIALIZATION_TIMEOUT_MS); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
Comment on lines
+87
to
+93
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current implementation resets the initialization fallback timer on every call to Consider only posting the delayed runnable if
Suggested change
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| isWaitingForValidDuration = false; | ||||||||||||||||||||||||||||||
| isInitialized = true; | ||||||||||||||||||||||||||||||
| mainHandler.removeCallbacks(initializationFallback); | ||||||||||||||||||||||||||||||
| sendInitialized(); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||
| public void onPlaybackStateChanged(final int playbackState) { | ||||||||||||||||||||||||||||||
| PlatformPlaybackState platformState = PlatformPlaybackState.UNKNOWN; | ||||||||||||||||||||||||||||||
|
|
@@ -60,10 +107,7 @@ public void onPlaybackStateChanged(final int playbackState) { | |||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||
| case Player.STATE_READY: | ||||||||||||||||||||||||||||||
| platformState = PlatformPlaybackState.READY; | ||||||||||||||||||||||||||||||
| if (!isInitialized) { | ||||||||||||||||||||||||||||||
| isInitialized = true; | ||||||||||||||||||||||||||||||
| sendInitialized(); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| maybeSendInitialized(); | ||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||
| case Player.STATE_ENDED: | ||||||||||||||||||||||||||||||
| platformState = PlatformPlaybackState.ENDED; | ||||||||||||||||||||||||||||||
|
|
@@ -75,6 +119,13 @@ public void onPlaybackStateChanged(final int playbackState) { | |||||||||||||||||||||||||||||
| events.onPlaybackStateChanged(platformState); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||
| public void onTimelineChanged(@NonNull Timeline timeline, int reason) { | ||||||||||||||||||||||||||||||
| if (isWaitingForValidDuration && exoPlayer.getPlaybackState() == Player.STATE_READY) { | ||||||||||||||||||||||||||||||
| maybeSendInitialized(); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
Comment on lines
+123
to
+127
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The PR description mentions that new tests were added, but no changes to Please add unit tests that verify:
References
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||
| public void onPlayerError(@NonNull final PlaybackException error) { | ||||||||||||||||||||||||||||||
| if (error.errorCode == PlaybackException.ERROR_CODE_BEHIND_LIVE_WINDOW) { | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of
Handler.postDelayedintroduces a risk of crashing if the player is disposed before the timeout expires. IfinitializationFallbackruns afterexoPlayer.release(), callingsendInitialized()(which typically accesses the player) will likely result in anIllegalStateException.Please add a cleanup method to cancel the pending callback, and ensure the
VideoPlayerimplementation stores the listener and calls this method during disposal.