From 9290b468d07e8177a32b0befba83f6ade70a1f17 Mon Sep 17 00:00:00 2001 From: ibaker Date: Fri, 10 Jul 2020 14:10:14 +0100 Subject: [PATCH] Stop sharing a Handler between EPI and EPII Sharing the Handler has led to it being accidentally used for purposes beyond the original intention. Instead for EPII -> EPI communication: Call methods directly on ExoPlayerImpl that then post from the playback thread to the application thread. And for the MediaSourceList and Queue initialization, create a dedicated Handler based on the same applicationLooper. PiperOrigin-RevId: 320590527 --- .../android/exoplayer2/ExoPlayerImpl.java | 87 +++++++----- .../exoplayer2/ExoPlayerImplInternal.java | 124 +++++++++--------- 2 files changed, 119 insertions(+), 92 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java index c8640e56af..2ddf09295e 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java @@ -66,7 +66,7 @@ import java.util.concurrent.TimeoutException; private final Renderer[] renderers; private final TrackSelector trackSelector; - private final Handler applicationHandler; + private final PlaybackUpdateListenerImpl playbackUpdateListener; private final ExoPlayerImplInternal internalPlayer; private final Handler internalPlayerHandler; private final CopyOnWriteArrayList listeners; @@ -76,6 +76,7 @@ import java.util.concurrent.TimeoutException; private final boolean useLazyPreparation; private final MediaSourceFactory mediaSourceFactory; @Nullable private final AnalyticsCollector analyticsCollector; + private final Looper applicationLooper; private final BandwidthMeter bandwidthMeter; @RepeatMode private int repeatMode; @@ -142,6 +143,7 @@ import java.util.concurrent.TimeoutException; this.useLazyPreparation = useLazyPreparation; this.seekParameters = seekParameters; this.pauseAtEndOfMediaItems = pauseAtEndOfMediaItems; + this.applicationLooper = applicationLooper; repeatMode = Player.REPEAT_MODE_OFF; listeners = new CopyOnWriteArrayList<>(); mediaSourceHolderSnapshots = new ArrayList<>(); @@ -154,19 +156,13 @@ import java.util.concurrent.TimeoutException; period = new Timeline.Period(); playbackSpeed = Player.DEFAULT_PLAYBACK_SPEED; maskingWindowIndex = C.INDEX_UNSET; - applicationHandler = - new Handler(applicationLooper) { - @Override - public void handleMessage(Message msg) { - ExoPlayerImpl.this.handleEvent(msg); - } - }; + playbackUpdateListener = new PlaybackUpdateListenerImpl(applicationLooper); playbackInfo = PlaybackInfo.createDummy(emptyTrackSelectorResult); pendingListenerNotifications = new ArrayDeque<>(); if (analyticsCollector != null) { analyticsCollector.setPlayer(this); addListener(analyticsCollector); - bandwidthMeter.addEventListener(applicationHandler, analyticsCollector); + bandwidthMeter.addEventListener(new Handler(applicationLooper), analyticsCollector); } internalPlayer = new ExoPlayerImplInternal( @@ -180,8 +176,9 @@ import java.util.concurrent.TimeoutException; analyticsCollector, seekParameters, pauseAtEndOfMediaItems, - applicationHandler, - clock); + applicationLooper, + clock, + playbackUpdateListener); internalPlayerHandler = new Handler(internalPlayer.getPlaybackLooper()); } @@ -251,7 +248,7 @@ import java.util.concurrent.TimeoutException; @Override public Looper getApplicationLooper() { - return applicationHandler.getLooper(); + return applicationLooper; } @Override @@ -588,11 +585,8 @@ import java.util.concurrent.TimeoutException; // general because the midroll ad preceding the seek destination must be played before the // content position can be played, if a different ad is playing at the moment. Log.w(TAG, "seekTo ignored because an ad is playing"); - applicationHandler - .obtainMessage( - ExoPlayerImplInternal.MSG_PLAYBACK_INFO_CHANGED, - new ExoPlayerImplInternal.PlaybackInfoUpdate(playbackInfo)) - .sendToTarget(); + playbackUpdateListener.onPlaybackInfoUpdate( + new ExoPlayerImplInternal.PlaybackInfoUpdate(playbackInfo)); return; } maskWindowIndexAndPositionForSeek(timeline, windowIndex, positionMs); @@ -715,7 +709,7 @@ import java.util.concurrent.TimeoutException; ExoPlaybackException.createForUnexpected( new RuntimeException(new TimeoutException("Player release timed out."))))); } - applicationHandler.removeCallbacksAndMessages(null); + playbackUpdateListener.handler.removeCallbacksAndMessages(null); if (analyticsCollector != null) { bandwidthMeter.removeEventListener(analyticsCollector); } @@ -869,20 +863,6 @@ import java.util.concurrent.TimeoutException; return playbackInfo.timeline; } - // Not private so it can be called from an inner class without going through a thunk method. - /* package */ void handleEvent(Message msg) { - switch (msg.what) { - case ExoPlayerImplInternal.MSG_PLAYBACK_INFO_CHANGED: - handlePlaybackInfo((ExoPlayerImplInternal.PlaybackInfoUpdate) msg.obj); - break; - case ExoPlayerImplInternal.MSG_PLAYBACK_SPEED_CHANGED: - handlePlaybackSpeed((Float) msg.obj, /* operationAck= */ msg.arg1 != 0); - break; - default: - throw new IllegalStateException(); - } - } - private int getCurrentWindowIndexInternal() { if (shouldMaskPosition()) { return maskingWindowIndex; @@ -1282,6 +1262,49 @@ import java.util.concurrent.TimeoutException; return playbackInfo.timeline.isEmpty() || pendingOperationAcks > 0; } + private final class PlaybackUpdateListenerImpl + implements ExoPlayerImplInternal.PlaybackUpdateListener, Handler.Callback { + private static final int MSG_PLAYBACK_INFO_CHANGED = 0; + private static final int MSG_PLAYBACK_SPEED_CHANGED = 1; + + private final Handler handler; + + private PlaybackUpdateListenerImpl(Looper applicationLooper) { + handler = Util.createHandler(applicationLooper, /* callback= */ this); + } + + @Override + public void onPlaybackInfoUpdate(ExoPlayerImplInternal.PlaybackInfoUpdate playbackInfo) { + handler.obtainMessage(MSG_PLAYBACK_INFO_CHANGED, playbackInfo).sendToTarget(); + } + + @Override + public void onPlaybackSpeedChange(float playbackSpeed, boolean acknowledgeCommand) { + handler + .obtainMessage( + MSG_PLAYBACK_SPEED_CHANGED, + /* arg1= */ acknowledgeCommand ? 1 : 0, + /* arg2= */ 0, + /* obj= */ playbackSpeed) + .sendToTarget(); + } + + @Override + public boolean handleMessage(Message msg) { + switch (msg.what) { + case MSG_PLAYBACK_INFO_CHANGED: + handlePlaybackInfo((ExoPlayerImplInternal.PlaybackInfoUpdate) msg.obj); + break; + case MSG_PLAYBACK_SPEED_CHANGED: + handlePlaybackSpeed((Float) msg.obj, /* operationAck= */ msg.arg1 != 0); + break; + default: + throw new IllegalStateException(); + } + return true; + } + } + private static final class PlaybackInfoUpdate implements Runnable { private final PlaybackInfo playbackInfo; diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java index 637c418635..bff541ce18 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java @@ -63,9 +63,57 @@ import java.util.concurrent.atomic.AtomicBoolean; private static final String TAG = "ExoPlayerImplInternal"; - // External messages - public static final int MSG_PLAYBACK_INFO_CHANGED = 0; - public static final int MSG_PLAYBACK_SPEED_CHANGED = 1; + public static final class PlaybackInfoUpdate { + + private boolean hasPendingChange; + + public PlaybackInfo playbackInfo; + public int operationAcks; + public boolean positionDiscontinuity; + @DiscontinuityReason public int discontinuityReason; + public boolean hasPlayWhenReadyChangeReason; + @PlayWhenReadyChangeReason public int playWhenReadyChangeReason; + + public PlaybackInfoUpdate(PlaybackInfo playbackInfo) { + this.playbackInfo = playbackInfo; + } + + public void incrementPendingOperationAcks(int operationAcks) { + hasPendingChange |= operationAcks > 0; + this.operationAcks += operationAcks; + } + + public void setPlaybackInfo(PlaybackInfo playbackInfo) { + hasPendingChange |= this.playbackInfo != playbackInfo; + this.playbackInfo = playbackInfo; + } + + public void setPositionDiscontinuity(@DiscontinuityReason int discontinuityReason) { + if (positionDiscontinuity + && this.discontinuityReason != Player.DISCONTINUITY_REASON_INTERNAL) { + // We always prefer non-internal discontinuity reasons. We also assume that we won't report + // more than one non-internal discontinuity per message iteration. + Assertions.checkArgument(discontinuityReason == Player.DISCONTINUITY_REASON_INTERNAL); + return; + } + hasPendingChange = true; + positionDiscontinuity = true; + this.discontinuityReason = discontinuityReason; + } + + public void setPlayWhenReadyChangeReason( + @PlayWhenReadyChangeReason int playWhenReadyChangeReason) { + hasPendingChange = true; + this.hasPlayWhenReadyChangeReason = true; + this.playWhenReadyChangeReason = playWhenReadyChangeReason; + } + } + + public interface PlaybackUpdateListener { + void onPlaybackInfoUpdate(ExoPlayerImplInternal.PlaybackInfoUpdate playbackInfo); + + void onPlaybackSpeedChange(float playbackSpeed, boolean acknowledgeCommand); + } // Internal messages private static final int MSG_PREPARE = 0; @@ -113,7 +161,6 @@ import java.util.concurrent.atomic.AtomicBoolean; private final BandwidthMeter bandwidthMeter; private final HandlerWrapper handler; private final HandlerThread internalPlaybackThread; - private final Handler eventHandler; private final Timeline.Window window; private final Timeline.Period period; private final long backBufferDurationUs; @@ -121,6 +168,7 @@ import java.util.concurrent.atomic.AtomicBoolean; private final DefaultMediaClock mediaClock; private final ArrayList pendingMessages; private final Clock clock; + private final PlaybackUpdateListener playbackUpdateListener; private final MediaPeriodQueue queue; private final MediaSourceList mediaSourceList; @@ -160,8 +208,10 @@ import java.util.concurrent.atomic.AtomicBoolean; @Nullable AnalyticsCollector analyticsCollector, SeekParameters seekParameters, boolean pauseAtEndOfWindow, - Handler eventHandler, - Clock clock) { + Looper applicationLooper, + Clock clock, + PlaybackUpdateListener playbackUpdateListener) { + this.playbackUpdateListener = playbackUpdateListener; this.renderers = renderers; this.trackSelector = trackSelector; this.emptyTrackSelectorResult = emptyTrackSelectorResult; @@ -171,9 +221,7 @@ import java.util.concurrent.atomic.AtomicBoolean; this.shuffleModeEnabled = shuffleModeEnabled; this.seekParameters = seekParameters; this.pauseAtEndOfWindow = pauseAtEndOfWindow; - this.eventHandler = eventHandler; this.clock = clock; - this.queue = new MediaPeriodQueue(analyticsCollector, eventHandler); backBufferDurationUs = loadControl.getBackBufferDurationUs(); retainBackBufferFromKeyframe = loadControl.retainBackBufferFromKeyframe(); @@ -191,13 +239,17 @@ import java.util.concurrent.atomic.AtomicBoolean; period = new Timeline.Period(); trackSelector.init(/* listener= */ this, bandwidthMeter); + deliverPendingMessageAtStartPositionRequired = true; + + Handler eventHandler = new Handler(applicationLooper); + queue = new MediaPeriodQueue(analyticsCollector, eventHandler); + mediaSourceList = new MediaSourceList(/* listener= */ this, analyticsCollector, eventHandler); + // Note: The documentation for Process.THREAD_PRIORITY_AUDIO that states "Applications can // not normally change to this priority" is incorrect. internalPlaybackThread = new HandlerThread("ExoPlayer:Playback", Process.THREAD_PRIORITY_AUDIO); internalPlaybackThread.start(); handler = clock.createHandler(internalPlaybackThread.getLooper(), this); - deliverPendingMessageAtStartPositionRequired = true; - mediaSourceList = new MediaSourceList(/* listener= */ this, analyticsCollector, eventHandler); } public void experimental_setReleaseTimeoutMs(long releaseTimeoutMs) { @@ -571,7 +623,7 @@ import java.util.concurrent.atomic.AtomicBoolean; private void maybeNotifyPlaybackInfoChanged() { playbackInfoUpdate.setPlaybackInfo(playbackInfo); if (playbackInfoUpdate.hasPendingChange) { - eventHandler.obtainMessage(MSG_PLAYBACK_INFO_CHANGED, playbackInfoUpdate).sendToTarget(); + playbackUpdateListener.onPlaybackInfoUpdate(playbackInfoUpdate); playbackInfoUpdate = new PlaybackInfoUpdate(playbackInfo); } } @@ -1938,9 +1990,7 @@ import java.util.concurrent.atomic.AtomicBoolean; private void handlePlaybackSpeed(float playbackSpeed, boolean acknowledgeCommand) throws ExoPlaybackException { - eventHandler - .obtainMessage(MSG_PLAYBACK_SPEED_CHANGED, acknowledgeCommand ? 1 : 0, 0, playbackSpeed) - .sendToTarget(); + playbackUpdateListener.onPlaybackSpeedChange(playbackSpeed, acknowledgeCommand); updateTrackSelectionPlaybackSpeed(playbackSpeed); for (Renderer renderer : renderers) { if (renderer != null) { @@ -2650,50 +2700,4 @@ import java.util.concurrent.atomic.AtomicBoolean; this.shuffleOrder = shuffleOrder; } } - - /* package */ static final class PlaybackInfoUpdate { - - private boolean hasPendingChange; - - public PlaybackInfo playbackInfo; - public int operationAcks; - public boolean positionDiscontinuity; - @DiscontinuityReason public int discontinuityReason; - public boolean hasPlayWhenReadyChangeReason; - @PlayWhenReadyChangeReason public int playWhenReadyChangeReason; - - public PlaybackInfoUpdate(PlaybackInfo playbackInfo) { - this.playbackInfo = playbackInfo; - } - - public void incrementPendingOperationAcks(int operationAcks) { - hasPendingChange |= operationAcks > 0; - this.operationAcks += operationAcks; - } - - public void setPlaybackInfo(PlaybackInfo playbackInfo) { - hasPendingChange |= this.playbackInfo != playbackInfo; - this.playbackInfo = playbackInfo; - } - - public void setPositionDiscontinuity(@DiscontinuityReason int discontinuityReason) { - if (positionDiscontinuity - && this.discontinuityReason != Player.DISCONTINUITY_REASON_INTERNAL) { - // We always prefer non-internal discontinuity reasons. We also assume that we won't report - // more than one non-internal discontinuity per message iteration. - Assertions.checkArgument(discontinuityReason == Player.DISCONTINUITY_REASON_INTERNAL); - return; - } - hasPendingChange = true; - positionDiscontinuity = true; - this.discontinuityReason = discontinuityReason; - } - - public void setPlayWhenReadyChangeReason( - @PlayWhenReadyChangeReason int playWhenReadyChangeReason) { - hasPendingChange = true; - this.hasPlayWhenReadyChangeReason = true; - this.playWhenReadyChangeReason = playWhenReadyChangeReason; - } - } }