From 4fe6e5d7552804a826e92d55cce2e247399306af Mon Sep 17 00:00:00 2001 From: olly Date: Wed, 15 Jun 2016 12:19:43 -0700 Subject: [PATCH] Misc fixes for playback tests. - AllowedVideoJoiningTimeMs must be set to 0 for tests so that tests which disable/enable video renderers don't register a large number of dropped frames. - Fixed a threading issue that could cause occassional test failure. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=124978843 --- .../android/exoplayer/ExoPlayerFactory.java | 31 ++++++++++++-- .../android/exoplayer/SimpleExoPlayer.java | 20 ++++----- .../playbacktests/util/ExoHostedTest.java | 13 +++--- .../playbacktests/util/HostActivity.java | 42 +++++++++++-------- 4 files changed, 69 insertions(+), 37 deletions(-) diff --git a/library/src/main/java/com/google/android/exoplayer/ExoPlayerFactory.java b/library/src/main/java/com/google/android/exoplayer/ExoPlayerFactory.java index 3fbc314bf2..6251622f63 100644 --- a/library/src/main/java/com/google/android/exoplayer/ExoPlayerFactory.java +++ b/library/src/main/java/com/google/android/exoplayer/ExoPlayerFactory.java @@ -38,6 +38,12 @@ public final class ExoPlayerFactory { */ public static final int DEFAULT_MIN_REBUFFER_MS = 5000; + /** + * The default maximum duration for which a video renderer can attempt to seamlessly join an + * ongoing playback. + */ + public static final long DEFAULT_ALLOWED_VIDEO_JOINING_TIME_MS = 5000; + private ExoPlayerFactory() {} /** @@ -49,7 +55,22 @@ public final class ExoPlayerFactory { * @param trackSelector The {@link TrackSelector} that will be used by the instance. */ public static SimpleExoPlayer newSimpleInstance(Context context, TrackSelector trackSelector) { - return newSimpleInstance(context, trackSelector, null, false); + return newSimpleInstance(context, trackSelector, null); + } + + /** + * Obtains a {@link SimpleExoPlayer} instance. + *

+ * Must be called from a thread that has an associated {@link Looper}. + * + * @param context A {@link Context}. + * @param trackSelector The {@link TrackSelector} that will be used by the instance. + * @param drmSessionManager An optional {@link DrmSessionManager}. May be null if the instance + * will not be used for DRM protected playbacks. + */ + public static SimpleExoPlayer newSimpleInstance(Context context, TrackSelector trackSelector, + DrmSessionManager drmSessionManager) { + return newSimpleInstance(context, trackSelector, drmSessionManager, false); } /** @@ -68,7 +89,7 @@ public final class ExoPlayerFactory { public static SimpleExoPlayer newSimpleInstance(Context context, TrackSelector trackSelector, DrmSessionManager drmSessionManager, boolean preferExtensionDecoders) { return newSimpleInstance(context, trackSelector, drmSessionManager, preferExtensionDecoders, - DEFAULT_MIN_BUFFER_MS, DEFAULT_MIN_REBUFFER_MS); + DEFAULT_MIN_BUFFER_MS, DEFAULT_MIN_REBUFFER_MS, DEFAULT_ALLOWED_VIDEO_JOINING_TIME_MS); } /** @@ -88,12 +109,14 @@ public final class ExoPlayerFactory { * @param minRebufferMs A minimum duration of data that must be buffered for playback to resume * after a player-invoked rebuffer (i.e. a rebuffer that occurs due to buffer depletion, and * not due to a user action such as starting playback or seeking). + * @param allowedVideoJoiningTimeMs The maximum duration for which a video renderer can attempt to + * seamlessly join an ongoing playback. */ public static SimpleExoPlayer newSimpleInstance(Context context, TrackSelector trackSelector, DrmSessionManager drmSessionManager, boolean preferExtensionDecoders, int minBufferMs, - int minRebufferMs) { + int minRebufferMs, long allowedVideoJoiningTimeMs) { return new SimpleExoPlayer(context, trackSelector, drmSessionManager, preferExtensionDecoders, - minBufferMs, minRebufferMs); + minBufferMs, minRebufferMs, allowedVideoJoiningTimeMs); } /** diff --git a/library/src/main/java/com/google/android/exoplayer/SimpleExoPlayer.java b/library/src/main/java/com/google/android/exoplayer/SimpleExoPlayer.java index 5d31e83b8c..7cc68074e1 100644 --- a/library/src/main/java/com/google/android/exoplayer/SimpleExoPlayer.java +++ b/library/src/main/java/com/google/android/exoplayer/SimpleExoPlayer.java @@ -90,7 +90,6 @@ public final class SimpleExoPlayer implements ExoPlayer { } private static final String TAG = "SimpleExoPlayer"; - private static final long ALLOWED_VIDEO_JOINING_TIME_MS = 5000; private static final int MAX_DROPPED_VIDEO_FRAME_COUNT_TO_NOTIFY = 50; private final ExoPlayer player; @@ -113,7 +112,7 @@ public final class SimpleExoPlayer implements ExoPlayer { /* package */ SimpleExoPlayer(Context context, TrackSelector trackSelector, DrmSessionManager drmSessionManager, boolean preferExtensionDecoders, int minBufferMs, - int minRebufferMs) { + int minRebufferMs, long allowedVideoJoiningTimeMs) { mainHandler = new Handler(); bandwidthMeter = new DefaultBandwidthMeter(); componentListener = new ComponentListener(); @@ -121,11 +120,11 @@ public final class SimpleExoPlayer implements ExoPlayer { // Build the renderers. ArrayList renderersList = new ArrayList<>(); if (preferExtensionDecoders) { - buildExtensionRenderers(renderersList); - buildRenderers(context, drmSessionManager, renderersList); + buildExtensionRenderers(renderersList, allowedVideoJoiningTimeMs); + buildRenderers(context, drmSessionManager, renderersList, allowedVideoJoiningTimeMs); } else { - buildRenderers(context, drmSessionManager, renderersList); - buildExtensionRenderers(renderersList); + buildRenderers(context, drmSessionManager, renderersList, allowedVideoJoiningTimeMs); + buildExtensionRenderers(renderersList, allowedVideoJoiningTimeMs); } renderers = renderersList.toArray(new TrackRenderer[renderersList.size()]); @@ -387,10 +386,10 @@ public final class SimpleExoPlayer implements ExoPlayer { // Internal methods. private void buildRenderers(Context context, DrmSessionManager drmSessionManager, - ArrayList renderersList) { + ArrayList renderersList, long allowedVideoJoiningTimeMs) { MediaCodecVideoTrackRenderer videoRenderer = new MediaCodecVideoTrackRenderer(context, MediaCodecSelector.DEFAULT, MediaCodec.VIDEO_SCALING_MODE_SCALE_TO_FIT, - ALLOWED_VIDEO_JOINING_TIME_MS, drmSessionManager, false, mainHandler, componentListener, + allowedVideoJoiningTimeMs, drmSessionManager, false, mainHandler, componentListener, MAX_DROPPED_VIDEO_FRAME_COUNT_TO_NOTIFY); renderersList.add(videoRenderer); @@ -407,7 +406,8 @@ public final class SimpleExoPlayer implements ExoPlayer { renderersList.add(id3Renderer); } - private void buildExtensionRenderers(ArrayList renderersList) { + private void buildExtensionRenderers(ArrayList renderersList, + long allowedVideoJoiningTimeMs) { // Load extension renderers using reflection so that demo app doesn't depend on them. // Class.forName() appears for each renderer so that automated tools like proguard // can detect the use of reflection (see http://proguard.sourceforge.net/FAQ.html#forname). @@ -416,7 +416,7 @@ public final class SimpleExoPlayer implements ExoPlayer { Class.forName("com.google.android.exoplayer.ext.vp9.LibvpxVideoTrackRenderer"); Constructor constructor = clazz.getConstructor(boolean.class, long.class, Handler.class, VideoTrackRendererEventListener.class, int.class); - renderersList.add((TrackRenderer) constructor.newInstance(true, ALLOWED_VIDEO_JOINING_TIME_MS, + renderersList.add((TrackRenderer) constructor.newInstance(true, allowedVideoJoiningTimeMs, mainHandler, componentListener, MAX_DROPPED_VIDEO_FRAME_COUNT_TO_NOTIFY)); Log.i(TAG, "Loaded LibvpxVideoTrackRenderer."); } catch (ClassNotFoundException e) { diff --git a/playbacktests/src/main/java/com/google/android/exoplayer/playbacktests/util/ExoHostedTest.java b/playbacktests/src/main/java/com/google/android/exoplayer/playbacktests/util/ExoHostedTest.java index 1d7c7dda64..b584f4f201 100644 --- a/playbacktests/src/main/java/com/google/android/exoplayer/playbacktests/util/ExoHostedTest.java +++ b/playbacktests/src/main/java/com/google/android/exoplayer/playbacktests/util/ExoHostedTest.java @@ -140,6 +140,11 @@ public abstract class ExoHostedTest implements HostedTest, ExoPlayer.EventListen } } + @Override + public final boolean canStop() { + return playerFinished; + } + @Override public final void onStop() { actionHandler.removeCallbacksAndMessages(null); @@ -148,11 +153,6 @@ public abstract class ExoHostedTest implements HostedTest, ExoPlayer.EventListen player = null; } - @Override - public final boolean isFinished() { - return playerFinished; - } - @Override public final void onFinished() { if (failOnPlayerError && playerError != null) { @@ -271,7 +271,8 @@ public abstract class ExoHostedTest implements HostedTest, ExoPlayer.EventListen @SuppressWarnings("unused") protected SimpleExoPlayer buildExoPlayer(HostActivity host, Surface surface, DefaultTrackSelector trackSelector) { - SimpleExoPlayer player = ExoPlayerFactory.newSimpleInstance(host, trackSelector); + SimpleExoPlayer player = ExoPlayerFactory.newSimpleInstance(host, trackSelector, null, false, + ExoPlayerFactory.DEFAULT_MIN_BUFFER_MS, ExoPlayerFactory.DEFAULT_MIN_REBUFFER_MS, 0); player.setSurface(surface); return player; } diff --git a/playbacktests/src/main/java/com/google/android/exoplayer/playbacktests/util/HostActivity.java b/playbacktests/src/main/java/com/google/android/exoplayer/playbacktests/util/HostActivity.java index 4bf9de1db1..05a13293d1 100644 --- a/playbacktests/src/main/java/com/google/android/exoplayer/playbacktests/util/HostActivity.java +++ b/playbacktests/src/main/java/com/google/android/exoplayer/playbacktests/util/HostActivity.java @@ -58,23 +58,23 @@ public final class HostActivity extends Activity implements SurfaceHolder.Callba */ void onStart(HostActivity host, Surface surface); + /** + * Called on the main thread to check whether the test is ready to be stopped. + * + * @return True if the test is ready to be stopped. False otherwise. + */ + boolean canStop(); + /** * Called on the main thread when the test is stopped. *

- * The test will be stopped if it has finished, if the {@link HostActivity} has been paused, or - * if the {@link HostActivity}'s {@link Surface} has been destroyed. + * The test will be stopped if {@link #canStop()} returns true, if the {@link HostActivity} has + * been paused, or if the {@link HostActivity}'s {@link Surface} has been destroyed. */ void onStop(); /** - * Called on the main thread to check whether the test has finished. - * - * @return True if the test has finished. False otherwise. - */ - boolean isFinished(); - - /** - * Called on the main thread after the test has finished and been stopped. + * Called on the test thread after the test has finished and been stopped. *

* Implementations may use this method to assert that test criteria were met. */ @@ -88,7 +88,7 @@ public final class HostActivity extends Activity implements SurfaceHolder.Callba private WifiLock wifiLock; private SurfaceView surfaceView; private Handler mainHandler; - private CheckFinishedRunnable checkFinishedRunnable; + private CheckCanStopRunnable checkCanStopRunnable; private HostedTest hostedTest; private ConditionVariable hostedTestStoppedCondition; @@ -147,7 +147,7 @@ public final class HostActivity extends Activity implements SurfaceHolder.Callba surfaceView = (SurfaceView) findViewById(R.id.surface_view); surfaceView.getHolder().addCallback(this); mainHandler = new Handler(); - checkFinishedRunnable = new CheckFinishedRunnable(); + checkCanStopRunnable = new CheckCanStopRunnable(); } @Override @@ -211,7 +211,7 @@ public final class HostActivity extends Activity implements SurfaceHolder.Callba hostedTestStarted = true; Log.d(TAG, "Starting test."); hostedTest.onStart(this, surface); - checkFinishedRunnable.startChecking(); + checkCanStopRunnable.startChecking(); } } @@ -219,8 +219,16 @@ public final class HostActivity extends Activity implements SurfaceHolder.Callba if (hostedTest != null && hostedTestStarted) { hostedTest.onStop(); hostedTest = null; - mainHandler.removeCallbacks(checkFinishedRunnable); - hostedTestStoppedCondition.open(); + mainHandler.removeCallbacks(checkCanStopRunnable); + // We post opening of the stopped condition so that any events posted to the main thread as a + // result of hostedTest.onStop() are guaranteed to be handled before hostedTest.onFinished() + // is invoked from runTest. + mainHandler.post(new Runnable() { + @Override + public void run() { + hostedTestStoppedCondition.open(); + } + }); } } @@ -229,7 +237,7 @@ public final class HostActivity extends Activity implements SurfaceHolder.Callba return Util.SDK_INT < 12 ? WifiManager.WIFI_MODE_FULL : WifiManager.WIFI_MODE_FULL_HIGH_PERF; } - private final class CheckFinishedRunnable implements Runnable { + private final class CheckCanStopRunnable implements Runnable { private static final long CHECK_INTERVAL_MS = 1000; @@ -239,7 +247,7 @@ public final class HostActivity extends Activity implements SurfaceHolder.Callba @Override public void run() { - if (hostedTest.isFinished()) { + if (hostedTest.canStop()) { hostedTestFinished = true; maybeStopHostedTest(); } else {