From e499f6d38c203e90615902e22b8dcdbfe47675aa Mon Sep 17 00:00:00 2001 From: olly Date: Fri, 9 Apr 2021 17:58:39 +0100 Subject: [PATCH] Improve tests to use output surfaces more realistically Prior to this change, there were some unrealistic quirks in our Robolectric tests. For example, onRenderedFirstFrame would be called when using FakeVideoRenderer, despite no output to render the frame to ever being set. This change improves the realism of these tests. These changes are required for some improvements being made to how outputs are set on video renderers. PiperOrigin-RevId: 367652169 --- .../exoplayer2/SimpleExoPlayerTest.java | 5 +++ .../analytics/AnalyticsCollectorTest.java | 22 +++++++------ .../video/DecoderVideoRendererTest.java | 7 +++-- .../video/MediaCodecVideoRendererTest.java | 26 ++++++++++------ .../testutil/ExoPlayerTestRunner.java | 28 ++++++++++++++--- .../testutil/FakeVideoRenderer.java | 31 ++++++++++++++----- 6 files changed, 86 insertions(+), 33 deletions(-) diff --git a/library/core/src/test/java/com/google/android/exoplayer2/SimpleExoPlayerTest.java b/library/core/src/test/java/com/google/android/exoplayer2/SimpleExoPlayerTest.java index 64841475e4..fb4ad4c648 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/SimpleExoPlayerTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/SimpleExoPlayerTest.java @@ -26,6 +26,8 @@ import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import android.graphics.SurfaceTexture; +import android.view.Surface; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.android.exoplayer2.analytics.AnalyticsListener; @@ -87,6 +89,7 @@ public class SimpleExoPlayerTest { @Test public void releaseAfterRendererEvents_triggersPendingVideoEventsInListener() throws Exception { + Surface surface = new Surface(new SurfaceTexture(/* texName= */ 0)); SimpleExoPlayer player = new SimpleExoPlayer.Builder( ApplicationProvider.getApplicationContext(), @@ -98,11 +101,13 @@ public class SimpleExoPlayerTest { player.addListener(listener); player.setMediaSource( new FakeMediaSource(new FakeTimeline(), ExoPlayerTestRunner.VIDEO_FORMAT)); + player.setVideoSurface(surface); player.prepare(); player.play(); runUntilPlaybackState(player, Player.STATE_READY); player.release(); + surface.release(); ShadowLooper.runMainLooperToNextTask(); verify(listener, atLeastOnce()).onEvents(any(), any()); // EventListener diff --git a/library/core/src/test/java/com/google/android/exoplayer2/analytics/AnalyticsCollectorTest.java b/library/core/src/test/java/com/google/android/exoplayer2/analytics/AnalyticsCollectorTest.java index d8fa67bd48..84ffe482be 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/analytics/AnalyticsCollectorTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/analytics/AnalyticsCollectorTest.java @@ -60,6 +60,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; +import android.graphics.SurfaceTexture; import android.os.Looper; import android.util.SparseArray; import android.view.Surface; @@ -1638,6 +1639,9 @@ public final class AnalyticsCollectorTest { public void onEvents_isReportedWithCorrectEventTimes() throws Exception { SimpleExoPlayer player = new TestExoPlayerBuilder(ApplicationProvider.getApplicationContext()).build(); + Surface surface = new Surface(new SurfaceTexture(/* texName= */ 0)); + player.setVideoSurface(surface); + AnalyticsListener listener = mock(AnalyticsListener.class); Format[] formats = new Format[] { @@ -1663,6 +1667,7 @@ public final class AnalyticsCollectorTest { TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_IDLE); ShadowLooper.runMainLooperToNextTask(); player.release(); + surface.release(); // Verify that expected individual callbacks have been called and capture EventTimes. ArgumentCaptor individualTimelineChangedEventTimes = @@ -1982,11 +1987,13 @@ public final class AnalyticsCollectorTest { @Nullable ActionSchedule actionSchedule, RenderersFactory renderersFactory) throws Exception { + Surface surface = new Surface(new SurfaceTexture(/* texName= */ 0)); TestAnalyticsListener listener = new TestAnalyticsListener(); try { new ExoPlayerTestRunner.Builder(ApplicationProvider.getApplicationContext()) .setMediaSources(mediaSource) .setRenderersFactory(renderersFactory) + .setVideoSurface(surface) .setAnalyticsListener(listener) .setActionSchedule(actionSchedule) .build() @@ -1995,6 +2002,8 @@ public final class AnalyticsCollectorTest { .blockUntilEnded(TIMEOUT_MS); } catch (ExoPlaybackException e) { // Ignore ExoPlaybackException as these may be expected. + } finally { + surface.release(); } return listener; } @@ -2358,12 +2367,7 @@ public final class AnalyticsCollectorTest { @Override public String toString() { - return "{" - + "type=" - + Long.numberOfTrailingZeros(eventType) - + ", windowAndPeriodId=" - + eventWindowAndPeriodId - + '}'; + return "{" + "type=" + eventType + ", windowAndPeriodId=" + eventWindowAndPeriodId + '}'; } } } @@ -2375,14 +2379,12 @@ public final class AnalyticsCollectorTest { */ private static final class EmptyDrmCallback implements MediaDrmCallback { @Override - public byte[] executeProvisionRequest(UUID uuid, ExoMediaDrm.ProvisionRequest request) - throws MediaDrmCallbackException { + public byte[] executeProvisionRequest(UUID uuid, ExoMediaDrm.ProvisionRequest request) { return new byte[0]; } @Override - public byte[] executeKeyRequest(UUID uuid, ExoMediaDrm.KeyRequest request) - throws MediaDrmCallbackException { + public byte[] executeKeyRequest(UUID uuid, ExoMediaDrm.KeyRequest request) { return new byte[0]; } } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/video/DecoderVideoRendererTest.java b/library/core/src/test/java/com/google/android/exoplayer2/video/DecoderVideoRendererTest.java index 848b0ce410..233f8cd440 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/video/DecoderVideoRendererTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/video/DecoderVideoRendererTest.java @@ -67,11 +67,13 @@ public final class DecoderVideoRendererTest { .setHeight(1080) .build(); + private Surface surface; private DecoderVideoRenderer renderer; @Mock private VideoRendererEventListener eventListener; @Before public void setUp() { + surface = new Surface(new SurfaceTexture(/* texName= */ 0)); renderer = new DecoderVideoRenderer( /* allowedJoiningTimeMs= */ 0, @@ -168,17 +170,18 @@ public final class DecoderVideoRendererTest { }; } }; - renderer.setOutputSurface(new Surface(new SurfaceTexture(/* texName= */ 0))); + renderer.setOutputSurface(surface); } @After - public void shutDown() throws Exception { + public void shutDown() { if (renderer.getState() == Renderer.STATE_STARTED) { renderer.stop(); } if (renderer.getState() == Renderer.STATE_ENABLED) { renderer.disable(); } + surface.release(); } @Test diff --git a/library/core/src/test/java/com/google/android/exoplayer2/video/MediaCodecVideoRendererTest.java b/library/core/src/test/java/com/google/android/exoplayer2/video/MediaCodecVideoRendererTest.java index ccc4e89d58..22402bdde8 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/video/MediaCodecVideoRendererTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/video/MediaCodecVideoRendererTest.java @@ -19,7 +19,6 @@ import static com.google.android.exoplayer2.testutil.FakeSampleStream.FakeSample import static com.google.android.exoplayer2.testutil.FakeSampleStream.FakeSampleStreamItem.format; import static com.google.android.exoplayer2.testutil.FakeSampleStream.FakeSampleStreamItem.oneByteSample; import static com.google.common.truth.Truth.assertThat; -import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; @@ -52,6 +51,7 @@ import com.google.android.exoplayer2.upstream.DefaultAllocator; import com.google.android.exoplayer2.util.MimeTypes; import com.google.common.collect.ImmutableList; import java.util.Collections; +import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -75,6 +75,7 @@ public class MediaCodecVideoRendererTest { .build(); private Looper testMainLooper; + private Surface surface; private MediaCodecVideoRenderer mediaCodecVideoRenderer; @Nullable private Format currentOutputFormat; @@ -118,8 +119,13 @@ public class MediaCodecVideoRendererTest { } }; - mediaCodecVideoRenderer.handleMessage( - Renderer.MSG_SET_SURFACE, new Surface(new SurfaceTexture(/* texName= */ 0))); + surface = new Surface(new SurfaceTexture(/* texName= */ 0)); + mediaCodecVideoRenderer.handleMessage(Renderer.MSG_SET_SURFACE, surface); + } + + @After + public void cleanUp() { + surface.release(); } @Test @@ -323,7 +329,7 @@ public class MediaCodecVideoRendererTest { } shadowOf(testMainLooper).idle(); - verify(eventListener).onRenderedFirstFrame(any()); + verify(eventListener).onRenderedFirstFrame(eq(surface), /* renderTimeMs= */ anyLong()); } @Test @@ -353,7 +359,7 @@ public class MediaCodecVideoRendererTest { } shadowOf(testMainLooper).idle(); - verify(eventListener, never()).onRenderedFirstFrame(any()); + verify(eventListener, never()).onRenderedFirstFrame(eq(surface), /* renderTimeMs= */ anyLong()); } @Test @@ -383,7 +389,7 @@ public class MediaCodecVideoRendererTest { } shadowOf(testMainLooper).idle(); - verify(eventListener).onRenderedFirstFrame(any()); + verify(eventListener).onRenderedFirstFrame(eq(surface), /* renderTimeMs= */ anyLong()); } @Test @@ -437,7 +443,8 @@ public class MediaCodecVideoRendererTest { // Expect only the first frame of the first stream to have been rendered. shadowLooper.idle(); - verify(eventListener, times(2)).onRenderedFirstFrame(any()); + verify(eventListener, times(2)) + .onRenderedFirstFrame(eq(surface), /* renderTimeMs= */ anyLong()); } @Test @@ -488,12 +495,13 @@ public class MediaCodecVideoRendererTest { } shadowLooper.idle(); - verify(eventListener).onRenderedFirstFrame(any()); + verify(eventListener).onRenderedFirstFrame(eq(surface), /* renderTimeMs= */ anyLong()); // Render to streamOffsetUs and verify the new first frame gets rendered. mediaCodecVideoRenderer.render(/* positionUs= */ 100, SystemClock.elapsedRealtime() * 1000); shadowLooper.idle(); - verify(eventListener, times(2)).onRenderedFirstFrame(any()); + verify(eventListener, times(2)) + .onRenderedFirstFrame(eq(surface), /* renderTimeMs= */ anyLong()); } } diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/ExoPlayerTestRunner.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/ExoPlayerTestRunner.java index 223acfc557..87818b6455 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/ExoPlayerTestRunner.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/ExoPlayerTestRunner.java @@ -22,6 +22,7 @@ import static junit.framework.TestCase.assertFalse; import android.content.Context; import android.os.HandlerThread; import android.os.Looper; +import android.view.Surface; import androidx.annotation.Nullable; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.ExoPlaybackException; @@ -80,6 +81,7 @@ public final class ExoPlayerTestRunner implements Player.EventListener, ActionSc private Format[] supportedFormats; private Object manifest; private ActionSchedule actionSchedule; + private Surface surface; private Player.EventListener eventListener; private AnalyticsListener analyticsListener; private Integer expectedPlayerEndedCount; @@ -276,6 +278,17 @@ public final class ExoPlayerTestRunner implements Player.EventListener, ActionSc return this; } + /** + * Sets the video {@link Surface}. The default value is {@code null}. + * + * @param surface The {@link Surface} to be used by the player. + * @return This builder. + */ + public Builder setVideoSurface(Surface surface) { + this.surface = surface; + return this; + } + /** * Sets an {@link Player.EventListener} to be registered to listen to player events. * @@ -334,6 +347,7 @@ public final class ExoPlayerTestRunner implements Player.EventListener, ActionSc skipSettingMediaSources, initialWindowIndex, initialPositionMs, + surface, actionSchedule, eventListener, analyticsListener, @@ -347,6 +361,7 @@ public final class ExoPlayerTestRunner implements Player.EventListener, ActionSc private final boolean skipSettingMediaSources; private final int initialWindowIndex; private final long initialPositionMs; + @Nullable private final Surface surface; @Nullable private final ActionSchedule actionSchedule; @Nullable private final Player.EventListener eventListener; @Nullable private final AnalyticsListener analyticsListener; @@ -376,6 +391,7 @@ public final class ExoPlayerTestRunner implements Player.EventListener, ActionSc boolean skipSettingMediaSources, int initialWindowIndex, long initialPositionMs, + @Nullable Surface surface, @Nullable ActionSchedule actionSchedule, @Nullable Player.EventListener eventListener, @Nullable AnalyticsListener analyticsListener, @@ -386,6 +402,7 @@ public final class ExoPlayerTestRunner implements Player.EventListener, ActionSc this.skipSettingMediaSources = skipSettingMediaSources; this.initialWindowIndex = initialWindowIndex; this.initialPositionMs = initialPositionMs; + this.surface = surface; this.actionSchedule = actionSchedule; this.eventListener = eventListener; this.analyticsListener = analyticsListener; @@ -430,6 +447,12 @@ public final class ExoPlayerTestRunner implements Player.EventListener, ActionSc () -> { try { player = playerBuilder.setLooper(Looper.myLooper()).build(); + if (surface != null) { + player.setVideoSurface(surface); + } + if (pauseAtEndOfMediaItems) { + player.setPauseAtEndOfMediaItems(true); + } player.addListener(ExoPlayerTestRunner.this); if (eventListener != null) { player.addListener(eventListener); @@ -437,15 +460,12 @@ public final class ExoPlayerTestRunner implements Player.EventListener, ActionSc if (analyticsListener != null) { player.addAnalyticsListener(analyticsListener); } - if (pauseAtEndOfMediaItems) { - player.setPauseAtEndOfMediaItems(true); - } player.play(); if (actionSchedule != null) { actionSchedule.start( player, playerBuilder.getTrackSelector(), - /* surface= */ null, + surface, handler, /* callback= */ ExoPlayerTestRunner.this); } diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeVideoRenderer.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeVideoRenderer.java index f6c1bacea0..75dfbc365f 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeVideoRenderer.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeVideoRenderer.java @@ -18,6 +18,8 @@ package com.google.android.exoplayer2.testutil; import android.os.Handler; import android.os.SystemClock; +import android.view.Surface; +import androidx.annotation.Nullable; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.ExoPlaybackException; import com.google.android.exoplayer2.Format; @@ -33,6 +35,7 @@ public class FakeVideoRenderer extends FakeRenderer { private final VideoRendererEventListener.EventDispatcher eventDispatcher; private final DecoderCounters decoderCounters; private @MonotonicNonNull Format format; + @Nullable private Surface surface; private long streamOffsetUs; private boolean renderedFirstFrameAfterReset; private boolean mayRenderFirstFrameAfterEnableIfNotStarted; @@ -58,9 +61,7 @@ public class FakeVideoRenderer extends FakeRenderer { throws ExoPlaybackException { super.onStreamChanged(formats, startPositionUs, offsetUs); streamOffsetUs = offsetUs; - if (renderedFirstFrameAfterReset) { - renderedFirstFrameAfterReset = false; - } + renderedFirstFrameAfterReset = false; } @Override @@ -93,19 +94,33 @@ public class FakeVideoRenderer extends FakeRenderer { this.format = format; } + @Override + public void handleMessage(int messageType, @Nullable Object payload) throws ExoPlaybackException { + switch (messageType) { + case MSG_SET_SURFACE: + surface = (Surface) payload; + renderedFirstFrameAfterReset = false; + break; + default: + super.handleMessage(messageType, payload); + } + } + @Override protected boolean shouldProcessBuffer(long bufferTimeUs, long playbackPositionUs) { boolean shouldProcess = super.shouldProcessBuffer(bufferTimeUs, playbackPositionUs); boolean shouldRenderFirstFrame = - !renderedFirstFrameAfterEnable - ? (getState() == Renderer.STATE_STARTED || mayRenderFirstFrameAfterEnableIfNotStarted) - : !renderedFirstFrameAfterReset; + surface != null + && (!renderedFirstFrameAfterEnable + ? (getState() == Renderer.STATE_STARTED + || mayRenderFirstFrameAfterEnableIfNotStarted) + : !renderedFirstFrameAfterReset); shouldProcess |= shouldRenderFirstFrame && playbackPositionUs >= streamOffsetUs; - if (shouldProcess && !renderedFirstFrameAfterReset) { + if (shouldProcess && !renderedFirstFrameAfterReset && surface != null) { @MonotonicNonNull Format format = Assertions.checkNotNull(this.format); eventDispatcher.videoSizeChanged( format.width, format.height, format.rotationDegrees, format.pixelWidthHeightRatio); - eventDispatcher.renderedFirstFrame(/* surface= */ null); + eventDispatcher.renderedFirstFrame(surface); renderedFirstFrameAfterReset = true; renderedFirstFrameAfterEnable = true; }