Skip to content

[video_player_android] Handle BehindLiveWindowException #5869

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion packages/video_player/video_player_android/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## NEXT
## 2.4.12

* Updates compileSdk version to 34.
* Adds error handling for `BehindLiveWindowException`, which may occur upon live-video playback failure.

## 2.4.11

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,11 @@ public void onPlaybackStateChanged(final int playbackState) {
@Override
public void onPlayerError(@NonNull final PlaybackException error) {
setBuffering(false);
if (eventSink != null) {
if (error.errorCode == PlaybackException.ERROR_CODE_BEHIND_LIVE_WINDOW) {
// See https://exoplayer.dev/live-streaming.html#behindlivewindowexception-and-error_code_behind_live_window
exoPlayer.seekToDefaultPosition();
exoPlayer.prepare();
} else if (eventSink != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would we not still want to forward the video error to Dart?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think not, since it's basically un-actionable from Dart. It could make sense if there was an option to disable this error handling logic from Dart, but it would require a change in platform interface - this seems to be an overkill for this fix.

Copy link
Contributor

Choose a reason for hiding this comment

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

Longer term it would make more sense to have no handling here, and forward more structured errors to the clients so they could decide how to handle them, but we can start with this and see if there are requests for more control.

eventSink.error("VideoError", "Video player had error " + error, null);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,25 @@
package io.flutter.plugins.videoplayer;

import static org.junit.Assert.assertEquals;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.Mockito.*;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import com.google.android.exoplayer2.ExoPlayer;
import com.google.android.exoplayer2.Format;
import com.google.android.exoplayer2.PlaybackException;
import com.google.android.exoplayer2.Player;
import com.google.android.exoplayer2.upstream.DefaultHttpDataSource;
import io.flutter.plugin.common.EventChannel;
import io.flutter.view.TextureRegistry;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -278,4 +281,28 @@ public void onIsPlayingChangedSendsExpectedEvent() {
assertEquals(event2.get("event"), "isPlayingStateUpdate");
assertEquals(event2.get("isPlaying"), false);
}

@Test
public void behindLiveWindowErrorResetsPlayerToDefaultPosition() {
List<Player.Listener> listeners = new LinkedList<>();
doAnswer(invocation -> listeners.add(invocation.getArgument(0)))
.when(fakeExoPlayer)
.addListener(any());

VideoPlayer unused =
new VideoPlayer(
fakeExoPlayer,
fakeEventChannel,
fakeSurfaceTextureEntry,
fakeVideoPlayerOptions,
fakeEventSink,
httpDataSourceFactorySpy);

PlaybackException exception =
new PlaybackException(null, null, PlaybackException.ERROR_CODE_BEHIND_LIVE_WINDOW);
listeners.forEach(listener -> listener.onPlayerError(exception));

verify(fakeExoPlayer).seekToDefaultPosition();
verify(fakeExoPlayer).prepare();
}
}
2 changes: 1 addition & 1 deletion packages/video_player/video_player_android/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.4.11
version: 2.4.12

environment:
sdk: ">=3.0.0 <4.0.0"
Expand Down