diff --git a/core/src/main/java/com/novoda/noplayer/PlayerSurfaceHolder.java b/core/src/main/java/com/novoda/noplayer/PlayerSurfaceHolder.java index 204c5686..570efe19 100644 --- a/core/src/main/java/com/novoda/noplayer/PlayerSurfaceHolder.java +++ b/core/src/main/java/com/novoda/noplayer/PlayerSurfaceHolder.java @@ -22,7 +22,7 @@ public static PlayerSurfaceHolder create(SurfaceView surfaceView) { public static PlayerSurfaceHolder create(TextureView textureView) { PlayerViewSurfaceHolder surfaceHolder = new PlayerViewSurfaceHolder(); textureView.setSurfaceTextureListener(surfaceHolder); - return new PlayerSurfaceHolder(null, textureView, new PlayerViewSurfaceHolder()); + return new PlayerSurfaceHolder(null, textureView, surfaceHolder); } PlayerSurfaceHolder(@Nullable SurfaceView surfaceView, @Nullable TextureView textureView, PlayerViewSurfaceHolder surfaceHolder) { diff --git a/core/src/main/java/com/novoda/noplayer/PlayerViewSurfaceHolder.java b/core/src/main/java/com/novoda/noplayer/PlayerViewSurfaceHolder.java index 24132902..8e2ce954 100644 --- a/core/src/main/java/com/novoda/noplayer/PlayerViewSurfaceHolder.java +++ b/core/src/main/java/com/novoda/noplayer/PlayerViewSurfaceHolder.java @@ -5,6 +5,7 @@ import android.view.Surface; import android.view.SurfaceHolder; import android.view.TextureView; +import com.novoda.noplayer.model.Either; import java.util.ArrayList; import java.util.List; @@ -13,12 +14,12 @@ class PlayerViewSurfaceHolder implements SurfaceHolder.Callback, TextureView.Sur private final List callbacks = new ArrayList<>(); @Nullable - private Surface surface; + private Either eitherSurface; @Override public void surfaceCreated(SurfaceHolder surfaceHolder) { - this.surface = surfaceHolder.getSurface(); - notifyListeners(surface); + this.eitherSurface = Either.right(surfaceHolder); + notifyListeners(eitherSurface); callbacks.clear(); } @@ -35,11 +36,17 @@ public void surfaceDestroyed(SurfaceHolder holder) { @Override public void onSurfaceTextureAvailable(SurfaceTexture surfaceTexture, int width, int height) { - this.surface = new Surface(surfaceTexture); - notifyListeners(surface); + this.eitherSurface = Either.left(new Surface(surfaceTexture)); + notifyListeners(eitherSurface); callbacks.clear(); } + private void notifyListeners(Either either) { + for (Callback callback : callbacks) { + callback.onSurfaceReady(either); + } + } + @Override public void onSurfaceTextureSizeChanged(SurfaceTexture surface, int width, int height) { // do nothing @@ -58,27 +65,21 @@ public void onSurfaceTextureUpdated(SurfaceTexture surface) { // do nothing } - private void notifyListeners(Surface surface) { - for (Callback callback : callbacks) { - callback.onSurfaceReady(surface); - } - } - private void setSurfaceNotReady() { - surface = null; + eitherSurface = null; } @Override public void requestSurface(Callback callback) { - if (isSurfaceHolderReady()) { - callback.onSurfaceReady(surface); + if (isSurfaceReady()) { + callback.onSurfaceReady(eitherSurface); } else { callbacks.add(callback); } } - private boolean isSurfaceHolderReady() { - return surface != null; + private boolean isSurfaceReady() { + return eitherSurface != null; } @Override diff --git a/core/src/main/java/com/novoda/noplayer/SurfaceRequester.java b/core/src/main/java/com/novoda/noplayer/SurfaceRequester.java index c58453b9..df8146a6 100644 --- a/core/src/main/java/com/novoda/noplayer/SurfaceRequester.java +++ b/core/src/main/java/com/novoda/noplayer/SurfaceRequester.java @@ -1,6 +1,8 @@ package com.novoda.noplayer; import android.view.Surface; +import android.view.SurfaceHolder; +import com.novoda.noplayer.model.Either; public interface SurfaceRequester { @@ -10,8 +12,7 @@ public interface SurfaceRequester { interface Callback { - void onSurfaceReady(Surface surface); - + void onSurfaceReady(Either surface); } } diff --git a/core/src/main/java/com/novoda/noplayer/internal/mediaplayer/AndroidMediaPlayerFacade.java b/core/src/main/java/com/novoda/noplayer/internal/mediaplayer/AndroidMediaPlayerFacade.java index 2f13827d..ecda160b 100644 --- a/core/src/main/java/com/novoda/noplayer/internal/mediaplayer/AndroidMediaPlayerFacade.java +++ b/core/src/main/java/com/novoda/noplayer/internal/mediaplayer/AndroidMediaPlayerFacade.java @@ -6,12 +6,13 @@ import android.net.Uri; import android.support.annotation.Nullable; import android.view.Surface; - +import android.view.SurfaceHolder; import com.novoda.noplayer.internal.mediaplayer.PlaybackStateChecker.PlaybackState; import com.novoda.noplayer.internal.mediaplayer.forwarder.MediaPlayerForwarder; import com.novoda.noplayer.internal.utils.NoPlayerLog; import com.novoda.noplayer.internal.utils.Optional; import com.novoda.noplayer.model.AudioTracks; +import com.novoda.noplayer.model.Either; import com.novoda.noplayer.model.PlayerAudioTrack; import com.novoda.noplayer.model.PlayerSubtitleTrack; import com.novoda.noplayer.model.PlayerVideoTrack; @@ -67,7 +68,7 @@ static AndroidMediaPlayerFacade newInstance(Context context, MediaPlayerForwarde this.mediaPlayerCreator = mediaPlayerCreator; } - void prepareVideo(Uri videoUri, Surface surface) { + void prepareVideo(Uri videoUri, Either surface) { requestAudioFocus(); release(); try { @@ -83,7 +84,7 @@ private void requestAudioFocus() { audioManager.requestAudioFocus(null, AudioManager.STREAM_MUSIC, AudioManager.AUDIOFOCUS_GAIN); } - private MediaPlayer createAndBindMediaPlayer(Surface surface, + private MediaPlayer createAndBindMediaPlayer(Either surface, Uri videoUri) throws IOException, IllegalStateException, IllegalArgumentException { MediaPlayer mediaPlayer = mediaPlayerCreator.createMediaPlayer(); mediaPlayer.setOnPreparedListener(internalPreparedListener); @@ -92,7 +93,7 @@ private MediaPlayer createAndBindMediaPlayer(Surface surface, mediaPlayer.setOnErrorListener(internalErrorListener); mediaPlayer.setOnBufferingUpdateListener(internalBufferingUpdateListener); mediaPlayer.setDataSource(context, videoUri, NO_HEADERS); - mediaPlayer.setSurface(surface); + attachSurface(mediaPlayer, surface); mediaPlayer.setAudioStreamType(AudioManager.STREAM_MUSIC); mediaPlayer.setScreenOnWhilePlaying(true); @@ -173,13 +174,29 @@ void release() { } } - void start(Surface surface) throws IllegalStateException { + void start(Either surface) throws IllegalStateException { assertIsInPlaybackState(); - mediaPlayer.setSurface(surface); + attachSurface(mediaPlayer, surface); currentState = PLAYING; mediaPlayer.start(); } + private void attachSurface(final MediaPlayer mediaPlayer, Either surface) { + Either.Consumer setSurface = new Either.Consumer() { + @Override + public void accept(Surface value) { + mediaPlayer.setSurface(value); + } + }; + Either.Consumer setDisplay = new Either.Consumer() { + @Override + public void accept(SurfaceHolder value) { + mediaPlayer.setDisplay(value); + } + }; + surface.apply(setSurface, setDisplay); + } + void pause() throws IllegalStateException { assertIsInPlaybackState(); diff --git a/core/src/main/java/com/novoda/noplayer/internal/mediaplayer/AndroidMediaPlayerImpl.java b/core/src/main/java/com/novoda/noplayer/internal/mediaplayer/AndroidMediaPlayerImpl.java index cbe18da1..1cb6d918 100644 --- a/core/src/main/java/com/novoda/noplayer/internal/mediaplayer/AndroidMediaPlayerImpl.java +++ b/core/src/main/java/com/novoda/noplayer/internal/mediaplayer/AndroidMediaPlayerImpl.java @@ -3,20 +3,22 @@ import android.media.MediaPlayer; import android.net.Uri; import android.view.Surface; +import android.view.SurfaceHolder; import android.view.View; import com.novoda.noplayer.Listeners; import com.novoda.noplayer.NoPlayer; import com.novoda.noplayer.Options; import com.novoda.noplayer.PlayerInformation; import com.novoda.noplayer.PlayerState; -import com.novoda.noplayer.PlayerView; import com.novoda.noplayer.PlayerSurfaceHolder; +import com.novoda.noplayer.PlayerView; import com.novoda.noplayer.SurfaceRequester; import com.novoda.noplayer.internal.Heart; import com.novoda.noplayer.internal.listeners.PlayerListenersHolder; import com.novoda.noplayer.internal.mediaplayer.forwarder.MediaPlayerForwarder; import com.novoda.noplayer.internal.utils.Optional; import com.novoda.noplayer.model.AudioTracks; +import com.novoda.noplayer.model.Either; import com.novoda.noplayer.model.LoadTimeout; import com.novoda.noplayer.model.PlayerAudioTrack; import com.novoda.noplayer.model.PlayerSubtitleTrack; @@ -146,7 +148,7 @@ public void play() throws IllegalStateException { heart.startBeatingHeart(); requestSurface(new SurfaceRequester.Callback() { @Override - public void onSurfaceReady(Surface surface) { + public void onSurfaceReady(Either surface) { mediaPlayer.start(surface); listenersHolder.getStateChangedListeners().onVideoPlaying(); } @@ -160,7 +162,7 @@ public void playAt(final long positionInMillis) throws IllegalStateException { } else { requestSurface(new SurfaceRequester.Callback() { @Override - public void onSurfaceReady(Surface surface) { + public void onSurfaceReady(Either surface) { initialSeekWorkaround(surface, positionInMillis); } }); @@ -171,7 +173,7 @@ public void onSurfaceReady(Surface surface) { * Workaround to fix some devices (nexus 7 2013 in particular) from natively crashing the mediaplayer * by starting the mediaplayer before seeking it. */ - private void initialSeekWorkaround(Surface surface, final long initialPlayPositionInMillis) throws IllegalStateException { + private void initialSeekWorkaround(Either surface, final long initialPlayPositionInMillis) throws IllegalStateException { listenersHolder.getBufferStateListeners().onBufferStarted(); initialisePlaybackForSeeking(surface); delayedActionExecutor.performAfterDelay(new DelayedActionExecutor.Action() { @@ -182,7 +184,7 @@ public void perform() { }, INITIAL_PLAY_SEEK_DELAY_IN_MILLIS); } - private void initialisePlaybackForSeeking(Surface surface) { + private void initialisePlaybackForSeeking(Either surface) { mediaPlayer.start(surface); mediaPlayer.pause(); } @@ -231,7 +233,7 @@ public void loadVideo(final Uri uri, final Options options) { listenersHolder.getBufferStateListeners().onBufferStarted(); requestSurface(new SurfaceRequester.Callback() { @Override - public void onSurfaceReady(Surface surface) { + public void onSurfaceReady(Either surface) { mediaPlayer.prepareVideo(uri, surface); } }); diff --git a/core/src/main/java/com/novoda/noplayer/model/Either.java b/core/src/main/java/com/novoda/noplayer/model/Either.java new file mode 100644 index 00000000..dfdfc05b --- /dev/null +++ b/core/src/main/java/com/novoda/noplayer/model/Either.java @@ -0,0 +1,50 @@ +package com.novoda.noplayer.model; + +public abstract class Either { + + public static Either left(L left) { + return new Left<>(left); + } + + public static Either right(R right) { + return new Right<>(right); + } + + Either() { + // restrict subclasses to the package + } + + public abstract void apply(Consumer leftConsumer, Consumer rightConsumer); + + static class Left extends Either { + + private final L valueLeft; + + Left(L valueLeft) { + this.valueLeft = valueLeft; + } + + @Override + public void apply(Consumer leftConsumer, Consumer rightConsumer) { + leftConsumer.accept(valueLeft); + } + } + + static class Right extends Either { + + private final R valueRight; + + Right(R valueRight) { + this.valueRight = valueRight; + } + + @Override + public void apply(Consumer leftConsumer, Consumer rightConsumer) { + rightConsumer.accept(valueRight); + } + } + + public interface Consumer { + void accept(T value); + } +} diff --git a/core/src/test/java/com/novoda/noplayer/internal/mediaplayer/AndroidMediaPlayerFacadeTest.java b/core/src/test/java/com/novoda/noplayer/internal/mediaplayer/AndroidMediaPlayerFacadeTest.java index 6aac2238..95da2e0d 100644 --- a/core/src/test/java/com/novoda/noplayer/internal/mediaplayer/AndroidMediaPlayerFacadeTest.java +++ b/core/src/test/java/com/novoda/noplayer/internal/mediaplayer/AndroidMediaPlayerFacadeTest.java @@ -5,10 +5,12 @@ import android.media.MediaPlayer; import android.net.Uri; import android.view.Surface; +import android.view.SurfaceHolder; import com.novoda.noplayer.SurfaceRequester; import com.novoda.noplayer.internal.mediaplayer.forwarder.MediaPlayerForwarder; import com.novoda.noplayer.internal.utils.NoPlayerLog; import com.novoda.noplayer.model.AudioTracks; +import com.novoda.noplayer.model.Either; import com.novoda.noplayer.model.PlayerAudioTrack; import com.novoda.noplayer.model.PlayerAudioTrackFixture; import com.novoda.noplayer.model.PlayerSubtitleTrack; @@ -94,6 +96,7 @@ public class AndroidMediaPlayerFacadeTest { private MediaPlayer.OnCompletionListener completionListener; @Mock private MediaPlayerForwarder forwarder; + private Either eitherSurface; private AndroidMediaPlayerFacade facade; @@ -105,6 +108,16 @@ public void setUp() { given(mediaPlayerCreator.createMediaPlayer()).willReturn(mediaPlayer); given(playbackStateChecker.isInPlaybackState(eq(mediaPlayer), any(PlaybackStateChecker.PlaybackState.class))).willReturn(IS_IN_PLAYBACK_STATE); + eitherSurface = Either.left(surface); + givenSurfaceRequesterReturns(eitherSurface); + + given(forwarder.onPreparedListener()).willReturn(preparedListener); + given(forwarder.onCompletionListener()).willReturn(completionListener); + given(forwarder.onErrorListener()).willReturn(errorListener); + given(forwarder.onSizeChangedListener()).willReturn(videoSizeChangedListener); + } + + private void givenSurfaceRequesterReturns(final Either surface) { doAnswer(new Answer() { @Override public Void answer(InvocationOnMock invocation) { @@ -113,11 +126,6 @@ public Void answer(InvocationOnMock invocation) { return null; } }).when(surfaceRequester).requestSurface(any(SurfaceRequester.Callback.class)); - - given(forwarder.onPreparedListener()).willReturn(preparedListener); - given(forwarder.onCompletionListener()).willReturn(completionListener); - given(forwarder.onErrorListener()).willReturn(errorListener); - given(forwarder.onSizeChangedListener()).willReturn(videoSizeChangedListener); } @Test @@ -137,8 +145,8 @@ public void whenPreparing_thenDoesNotReleaseMediaPlayer() { @Test public void whenPreparingMultipleTimes_thenReleasesMediaPlayer() { - facade.prepareVideo(ANY_URI, surface); - facade.prepareVideo(ANY_URI, surface); + facade.prepareVideo(ANY_URI, eitherSurface); + facade.prepareVideo(ANY_URI, eitherSurface); verify(mediaPlayer).reset(); verify(mediaPlayer).release(); @@ -152,12 +160,27 @@ public void whenPreparing_thenSetsDataSource() throws IOException { } @Test - public void whenPreparing_thenSetsDisplay() { - givenMediaPlayerIsPrepared(); + public void givenSurfaceRequesterReturnsSurface_whenPreparing_thenSetsSurface() { + Surface surface = mock(Surface.class); + Either eitherSurface = Either.left(surface); + givenSurfaceRequesterReturns(eitherSurface); + + givenMediaPlayerIsPreparedWith(eitherSurface); verify(mediaPlayer).setSurface(surface); } + @Test + public void givenSurfaceRequesterReturnsSurfaceHolder_whenPreparing_thenSetsDisplay() { + SurfaceHolder surfaceHolder = mock(SurfaceHolder.class); + Either eitherSurface = Either.right(surfaceHolder); + givenSurfaceRequesterReturns(eitherSurface); + + givenMediaPlayerIsPreparedWith(eitherSurface); + + verify(mediaPlayer).setDisplay(surfaceHolder); + } + @Test public void whenPreparing_thenSetsStreamMusicAudioStreamType() { givenMediaPlayerIsPrepared(); @@ -196,7 +219,7 @@ public Void answer(InvocationOnMock invocation) { @Test public void givenBoundPreparedListener_andMediaPlayerIsPrepared_whenPrepared_thenForwardsOnPrepared() { - facade.prepareVideo(ANY_URI, surface); + facade.prepareVideo(ANY_URI, eitherSurface); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(MediaPlayer.OnPreparedListener.class); verify(mediaPlayer).setOnPreparedListener(argumentCaptor.capture()); argumentCaptor.getValue().onPrepared(mediaPlayer); @@ -288,27 +311,39 @@ public void givenMediaPlayerIsPrepared_whenReleasing_thenReleasesMediaPlayer() { } @Test - public void givenMediaPlayerIsPrepared_whenStarting_thenSetsDisplay() { + public void givenMediaPlayerIsPreparedWithSurface_whenStarting_thenSetsSurface() { givenMediaPlayerIsPrepared(); reset(mediaPlayer); - facade.start(surface); + facade.start(eitherSurface); verify(mediaPlayer).setSurface(surface); } + @Test + public void givenMediaPlayerIsPreparedWithSurfaceHolder_whenStarting_thenSetsDisplay() { + SurfaceHolder surfaceHolder = mock(SurfaceHolder.class); + Either eitherSurface = Either.right(surfaceHolder); + givenMediaPlayerIsPreparedWith(eitherSurface); + reset(mediaPlayer); + + facade.start(eitherSurface); + + verify(mediaPlayer).setDisplay(surfaceHolder); + } + @Test public void givenMediaPlayerIsNotPrepared_whenStarting_thenThrowsIllegalStateException() { thrown.expect(ExceptionMatcher.matches(ERROR_MESSAGE, IllegalStateException.class)); - facade.start(surface); + facade.start(eitherSurface); } @Test public void givenMediaPlayerIsPrepared_whenStarting_thenStartsMediaPlayer() { givenMediaPlayerIsPrepared(); - facade.start(surface); + facade.start(eitherSurface); verify(mediaPlayer).start(); } @@ -601,7 +636,11 @@ public void givenVolumeWasSet_whenGettingVolume_theReturnsSetVolume() { } private void givenMediaPlayerIsPrepared() { - facade.prepareVideo(ANY_URI, surface); + givenMediaPlayerIsPreparedWith(eitherSurface); + } + + private void givenMediaPlayerIsPreparedWith(Either eitherSurface) { + facade.prepareVideo(ANY_URI, eitherSurface); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(MediaPlayer.OnPreparedListener.class); verify(mediaPlayer).setOnPreparedListener(argumentCaptor.capture()); argumentCaptor.getValue().onPrepared(mediaPlayer); diff --git a/core/src/test/java/com/novoda/noplayer/internal/mediaplayer/AndroidMediaPlayerImplTest.java b/core/src/test/java/com/novoda/noplayer/internal/mediaplayer/AndroidMediaPlayerImplTest.java index 0707ec71..1865a24a 100644 --- a/core/src/test/java/com/novoda/noplayer/internal/mediaplayer/AndroidMediaPlayerImplTest.java +++ b/core/src/test/java/com/novoda/noplayer/internal/mediaplayer/AndroidMediaPlayerImplTest.java @@ -3,6 +3,7 @@ import android.media.MediaPlayer; import android.net.Uri; import android.view.Surface; +import android.view.SurfaceHolder; import android.view.View; import com.novoda.noplayer.ContentType; import com.novoda.noplayer.NoPlayer; @@ -17,6 +18,7 @@ import com.novoda.noplayer.internal.mediaplayer.forwarder.MediaPlayerForwarder; import com.novoda.noplayer.internal.utils.NoPlayerLog; import com.novoda.noplayer.model.AudioTracks; +import com.novoda.noplayer.model.Either; import com.novoda.noplayer.model.LoadTimeout; import com.novoda.noplayer.model.PlayerAudioTrack; import com.novoda.noplayer.model.PlayerAudioTrackFixture; @@ -686,7 +688,7 @@ public void onLoadTimeout() { @Mock NoPlayer.StateChangedListener stateChangedListener; @Mock - Surface surface; + Either surface; @Mock PlayerView playerView; @Mock diff --git a/core/src/test/java/com/novoda/noplayer/model/EitherTest.java b/core/src/test/java/com/novoda/noplayer/model/EitherTest.java new file mode 100644 index 00000000..43b288d8 --- /dev/null +++ b/core/src/test/java/com/novoda/noplayer/model/EitherTest.java @@ -0,0 +1,37 @@ +package com.novoda.noplayer.model; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import static org.mockito.Mockito.verify; + +@RunWith(MockitoJUnitRunner.class) +public class EitherTest { + + @Mock + private Either.Consumer leftConsumer; + @Mock + private Either.Consumer rightConsumer; + + @Test + public void givenEitherContainsLeft_whenApplyingConsumers_thenRunsLeftConsumerWithCorrectValue() { + String value = "foo"; + Either either = Either.left(value); + + either.apply(leftConsumer, rightConsumer); + + verify(leftConsumer).accept(value); + } + + @Test + public void givenEitherContainsRight_whenApplyingConsumers_thenRunsRightConsumerWithCorrectValue() { + Integer value = 42; + Either either = Either.right(value); + + either.apply(leftConsumer, rightConsumer); + + verify(rightConsumer).accept(value); + } +}