-
Notifications
You must be signed in to change notification settings - Fork 28
ExoPlayer2/Mediaplayer IllegalStateExeceptions #41
Conversation
…dia player is not in a playable state
…as this is now a client responsibility
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.
EZ
@@ -27,7 +27,7 @@ boolean isInPlaybackState(MediaPlayer mediaPlayer, PlaybackState playbackState) | |||
PREPARED, | |||
PLAYING, | |||
PAUSED, | |||
COMPLETED; | |||
COMPLETED |
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.
😂
public void givenMediaPlayerIsNotInPlaybackState_whenGettingPosition_thenThrowsIllegalStateException() { | ||
thrown.expect( | ||
ExceptionMatcher.matches( | ||
"Video must be loaded and not in an error state before trying to interact with the player", |
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.
Can we pull this string out into a field? If it ever changes we'd have to change like five times in the test as it stands
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.
can do 👍
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.
Problem
The
MediaPlayer Facade/Impl
is catchingIllegalStateExceptions
and returning safe values, this is the opposite to whatExoPlayer
is doing.Solution
Sync up the MediaPlayer with ExoPlayer by no longer catching
IllegalStateExceptions
when interacting with the MediaPlayer when it's not in a playback state.This is now a responsibility of clients.
Test(s) added
Yep around the exceptions being thrown.
LoadTimeout
Screenshots
no UI changes
Paired with
@Dorvaryn