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.

Sign up for GitHub

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
auto-submit merged 7 commits into from
Feb 12, 2024
Merged

[video_player_android] Handle BehindLiveWindowException #5869

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
df58d1b
[video_player_android] Handle BehindLiveWindowException
SynSzakala Jan 11, 2024
128ef9d
[video_player_android] Handle BehindLiveWindowException
SynSzakala Jan 11, 2024
da6280e
Merge branch 'main' into video_player_streaming_exception_fix
SynSzakala Jan 12, 2024
2cc8ab1
Merge branch 'main' into video_player_streaming_exception_fix
SynSzakala Jan 12, 2024
c5f4b4f
Merge remote-tracking branch 'origin/main' into video_player_streamin…
SynSzakala Feb 1, 2024
af94aa0
Merge remote-tracking branch 'origin/video_player_streaming_exception…
SynSzakala Feb 1, 2024
6ca509c
Nits in changelog and tests
SynSzakala Feb 1, 2024
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
4 changes: 4 additions & 0 deletions packages/video_player/video_player_android/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2.4.12

* Adds error handling for `BehindLiveWindowException`
Copy link
Contributor

@camsim99 camsim99 Jan 22, 2024

Choose a reason for hiding this comment

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

Nit:

Suggested change
* Adds error handling for `BehindLiveWindowException`
* Adds error handling for `BehindLiveWindowException`, which may occur upon video playback failure.


## 2.4.11

* Updates minimum supported SDK version to Flutter 3.10/Dart 3.0.
Expand Down
6 changes: 5 additions & 1 deletion ...ideo_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java
View file
Open in desktop
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

@camsim99 camsim99 Jan 22, 2024

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

@SynSzakala SynSzakala Feb 1, 2024

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
Collaborator

@stuartmorgan-g stuartmorgan-g Feb 12, 2024

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
33 changes: 30 additions & 3 deletions ..._player_android/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerTest.java
View file
Open in desktop
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 behindLiveWindowErrorResets() {
Copy link
Contributor

@camsim99 camsim99 Jan 22, 2024

Choose a reason for hiding this comment

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

Nit: can this be slightly more descriptive to describe what we expect? Something like:

Suggested change
public void behindLiveWindowErrorResets() {
public void behindLiveWindowErrorResetsPlaybackPositionToDefault() {

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
View file
Open in desktop
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