From 22599a6d6cb581ddc578f485c8ba08c51a4a09a5 Mon Sep 17 00:00:00 2001 From: tonihei Date: Fri, 4 Jan 2019 17:55:49 +0000 Subject: [PATCH] Pass Handler together with Runnable callbacks for playlist commands. We currently either use the app thread returned by the player or the thread the commands are called on depending on whether the media source is already prepared or not. This change lets the application decide which callback thread to use. As a side effect, we also don't longer need access the player instance passed to MediaSource.prepare. PiperOrigin-RevId: 227871111 --- RELEASENOTES.md | 2 + .../source/ConcatenatingMediaSource.java | 308 +++++++++++------- .../source/ConcatenatingMediaSourceTest.java | 34 +- 3 files changed, 208 insertions(+), 136 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index cff58e4b9a..271f2d94b9 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -2,6 +2,8 @@ ### 2.9.5 ### +* Add `Handler` parameter to `ConcatenatingMediaSource` methods which take a + callback `Runnable`. * HLS: Parse `CHANNELS` attribute from `EXT-X-MEDIA` tag. * ExtractorMediaSource: Fix issue that could cause the player to get stuck buffering at the end of the media. diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java index c93afdb249..fd567234e9 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java @@ -17,6 +17,7 @@ package com.google.android.exoplayer2.source; import android.os.Handler; import android.os.Message; +import android.support.annotation.GuardedBy; import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.util.Pair; @@ -28,6 +29,7 @@ import com.google.android.exoplayer2.source.ShuffleOrder.DefaultShuffleOrder; import com.google.android.exoplayer2.upstream.Allocator; import com.google.android.exoplayer2.upstream.TransferListener; import com.google.android.exoplayer2.util.Assertions; +import com.google.android.exoplayer2.util.EventDispatcher; import com.google.android.exoplayer2.util.Util; import java.io.IOException; import java.util.ArrayList; @@ -53,22 +55,21 @@ public class ConcatenatingMediaSource extends CompositeMediaSource mediaSourcesPublic; + @Nullable private Handler playbackThreadHandler; - // Accessed on the playback thread. + // Accessed on the playback thread only. private final List mediaSourceHolders; private final Map mediaSourceByMediaPeriod; private final Map mediaSourceByUid; - private final List pendingOnCompletionActions; private final boolean isAtomic; private final boolean useLazyPreparation; private final Timeline.Window window; private final Timeline.Period period; - @Nullable private Handler playbackThreadHandler; - @Nullable private Handler applicationThreadHandler; private boolean listenerNotificationScheduled; + private EventDispatcher pendingOnCompletionActions; private ShuffleOrder shuffleOrder; private int windowCount; private int periodCount; @@ -127,7 +128,7 @@ public class ConcatenatingMediaSource extends CompositeMediaSource(); this.mediaSourcesPublic = new ArrayList<>(); this.mediaSourceHolders = new ArrayList<>(); - this.pendingOnCompletionActions = new ArrayList<>(); + this.pendingOnCompletionActions = new EventDispatcher<>(); this.isAtomic = isAtomic; this.useLazyPreparation = useLazyPreparation; window = new Timeline.Window(); @@ -141,19 +142,20 @@ public class ConcatenatingMediaSource extends CompositeMediaSource mediaSources) { - addMediaSources(mediaSourcesPublic.size(), mediaSources, null); + addPublicMediaSources( + mediaSourcesPublic.size(), + mediaSources, + /* handler= */ null, + /* actionOnCompletion= */ null); } /** @@ -197,12 +209,13 @@ public class ConcatenatingMediaSource extends CompositeMediaSource mediaSources, @Nullable Runnable actionOnCompletion) { - addMediaSources(mediaSourcesPublic.size(), mediaSources, actionOnCompletion); + Collection mediaSources, Handler handler, Runnable actionOnCompletion) { + addPublicMediaSources(mediaSourcesPublic.size(), mediaSources, handler, actionOnCompletion); } /** @@ -214,7 +227,7 @@ public class ConcatenatingMediaSource extends CompositeMediaSource mediaSources) { - addMediaSources(index, mediaSources, null); + addPublicMediaSources(index, mediaSources, /* handler= */ null, /* actionOnCompletion= */ null); } /** @@ -224,26 +237,16 @@ public class ConcatenatingMediaSource extends CompositeMediaSource mediaSources, @Nullable Runnable actionOnCompletion) { - for (MediaSource mediaSource : mediaSources) { - Assertions.checkNotNull(mediaSource); - } - List mediaSourceHolders = new ArrayList<>(mediaSources.size()); - for (MediaSource mediaSource : mediaSources) { - mediaSourceHolders.add(new MediaSourceHolder(mediaSource)); - } - mediaSourcesPublic.addAll(index, mediaSourceHolders); - if (playbackThreadHandler != null && !mediaSources.isEmpty()) { - playbackThreadHandler - .obtainMessage(MSG_ADD, new MessageData<>(index, mediaSourceHolders, actionOnCompletion)) - .sendToTarget(); - } else if (actionOnCompletion != null) { - actionOnCompletion.run(); - } + int index, + Collection mediaSources, + Handler handler, + Runnable actionOnCompletion) { + addPublicMediaSources(index, mediaSources, handler, actionOnCompletion); } /** @@ -259,26 +262,27 @@ public class ConcatenatingMediaSource extends CompositeMediaSourceNote: If you want to move the instance, it's preferable to use {@link #moveMediaSource(int, - * int, Runnable)} instead. + * int, Handler, Runnable)} instead. * *

Note: If you want to remove a set of contiguous sources, it's preferable to use {@link - * #removeMediaSourceRange(int, int, Runnable)} instead. + * #removeMediaSourceRange(int, int, Handler, Runnable)} instead. * * @param index The index at which the media source will be removed. This index must be in the * range of 0 <= index < {@link #getSize()}. + * @param handler The {@link Handler} to run {@code actionOnCompletion}. * @param actionOnCompletion A {@link Runnable} which is executed immediately after the media * source has been removed from the playlist. */ public final synchronized void removeMediaSource( - int index, @Nullable Runnable actionOnCompletion) { - removeMediaSourceRange(index, index + 1, actionOnCompletion); + int index, Handler handler, Runnable actionOnCompletion) { + removePublicMediaSources(index, index + 1, handler, actionOnCompletion); } /** @@ -296,7 +300,8 @@ public class ConcatenatingMediaSource extends CompositeMediaSource(fromIndex, toIndex, actionOnCompletion)) - .sendToTarget(); - } else if (actionOnCompletion != null) { - actionOnCompletion.run(); - } + int fromIndex, int toIndex, Handler handler, Runnable actionOnCompletion) { + removePublicMediaSources(fromIndex, toIndex, handler, actionOnCompletion); } /** @@ -342,7 +335,8 @@ public class ConcatenatingMediaSource extends CompositeMediaSource(currentIndex, newIndex, actionOnCompletion)) - .sendToTarget(); - } else if (actionOnCompletion != null) { - actionOnCompletion.run(); - } + int currentIndex, int newIndex, Handler handler, Runnable actionOnCompletion) { + movePublicMediaSource(currentIndex, newIndex, handler, actionOnCompletion); } /** Clears the playlist. */ public final synchronized void clear() { - clear(/* actionOnCompletion= */ null); + removeMediaSourceRange(0, getSize()); } /** * Clears the playlist and executes a custom action on completion. * + * @param handler The {@link Handler} to run {@code actionOnCompletion}. * @param actionOnCompletion A {@link Runnable} which is executed immediately after the playlist * has been cleared. */ - public final synchronized void clear(@Nullable Runnable actionOnCompletion) { - removeMediaSourceRange(0, getSize(), actionOnCompletion); + public final synchronized void clear(Handler handler, Runnable actionOnCompletion) { + removeMediaSourceRange(0, getSize(), handler, actionOnCompletion); } /** Returns the number of media sources in the playlist. */ @@ -410,41 +393,24 @@ public class ConcatenatingMediaSource extends CompositeMediaSource(/* index= */ 0, shuffleOrder, actionOnCompletion)) - .sendToTarget(); - } else { - this.shuffleOrder = - shuffleOrder.getLength() > 0 ? shuffleOrder.cloneAndClear() : shuffleOrder; - if (actionOnCompletion != null) { - actionOnCompletion.run(); - } - } + ShuffleOrder shuffleOrder, Handler handler, Runnable actionOnCompletion) { + setPublicShuffleOrder(shuffleOrder, handler, actionOnCompletion); } + // CompositeMediaSource implementation. + @Override @Nullable public Object getTag() { @@ -458,13 +424,12 @@ public class ConcatenatingMediaSource extends CompositeMediaSource mediaSources, + @Nullable Handler handler, + @Nullable Runnable actionOnCompletion) { + Assertions.checkArgument((handler == null) == (actionOnCompletion == null)); + for (MediaSource mediaSource : mediaSources) { + Assertions.checkNotNull(mediaSource); + } + List mediaSourceHolders = new ArrayList<>(mediaSources.size()); + for (MediaSource mediaSource : mediaSources) { + mediaSourceHolders.add(new MediaSourceHolder(mediaSource)); + } + mediaSourcesPublic.addAll(index, mediaSourceHolders); + if (playbackThreadHandler != null && !mediaSources.isEmpty()) { + playbackThreadHandler + .obtainMessage( + MSG_ADD, new MessageData<>(index, mediaSourceHolders, handler, actionOnCompletion)) + .sendToTarget(); + } else if (actionOnCompletion != null && handler != null) { + handler.post(actionOnCompletion); + } + } + + @GuardedBy("this") + private void removePublicMediaSources( + int fromIndex, + int toIndex, + @Nullable Handler handler, + @Nullable Runnable actionOnCompletion) { + Assertions.checkArgument((handler == null) == (actionOnCompletion == null)); + Util.removeRange(mediaSourcesPublic, fromIndex, toIndex); + if (playbackThreadHandler != null) { + playbackThreadHandler + .obtainMessage( + MSG_REMOVE, new MessageData<>(fromIndex, toIndex, handler, actionOnCompletion)) + .sendToTarget(); + } else if (actionOnCompletion != null && handler != null) { + handler.post(actionOnCompletion); + } + } + + @GuardedBy("this") + private void movePublicMediaSource( + int currentIndex, + int newIndex, + @Nullable Handler handler, + @Nullable Runnable actionOnCompletion) { + Assertions.checkArgument((handler == null) == (actionOnCompletion == null)); + mediaSourcesPublic.add(newIndex, mediaSourcesPublic.remove(currentIndex)); + if (playbackThreadHandler != null) { + playbackThreadHandler + .obtainMessage( + MSG_MOVE, new MessageData<>(currentIndex, newIndex, handler, actionOnCompletion)) + .sendToTarget(); + } else if (actionOnCompletion != null && handler != null) { + handler.post(actionOnCompletion); + } + } + + @GuardedBy("this") + private void setPublicShuffleOrder( + ShuffleOrder shuffleOrder, @Nullable Handler handler, @Nullable Runnable actionOnCompletion) { + Assertions.checkArgument((handler == null) == (actionOnCompletion == null)); + Handler playbackThreadHandler = this.playbackThreadHandler; + if (playbackThreadHandler != null) { + int size = getSize(); + if (shuffleOrder.getLength() != size) { + shuffleOrder = + shuffleOrder + .cloneAndClear() + .cloneAndInsert(/* insertionIndex= */ 0, /* insertionCount= */ size); + } + playbackThreadHandler + .obtainMessage( + MSG_SET_SHUFFLE_ORDER, + new MessageData<>(/* index= */ 0, shuffleOrder, handler, actionOnCompletion)) + .sendToTarget(); + } else { + this.shuffleOrder = + shuffleOrder.getLength() > 0 ? shuffleOrder.cloneAndClear() : shuffleOrder; + if (actionOnCompletion != null && handler != null) { + handler.post(actionOnCompletion); + } + } + } + + // Internal methods. Called on the playback thread. + @SuppressWarnings("unchecked") private boolean handleMessage(Message msg) { if (playbackThreadHandler == null) { @@ -562,7 +618,7 @@ public class ConcatenatingMediaSource extends CompositeMediaSource>) Util.castNonNull(msg.obj); shuffleOrder = shuffleOrder.cloneAndInsert(addMessage.index, addMessage.customData.size()); addMediaSourcesInternal(addMessage.index, addMessage.customData); - scheduleListenerNotification(addMessage.actionOnCompletion); + scheduleListenerNotification(addMessage.handler, addMessage.actionOnCompletion); break; case MSG_REMOVE: MessageData removeMessage = (MessageData) Util.castNonNull(msg.obj); @@ -576,30 +632,29 @@ public class ConcatenatingMediaSource extends CompositeMediaSource= fromIndex; index--) { removeMediaSourceInternal(index); } - scheduleListenerNotification(removeMessage.actionOnCompletion); + scheduleListenerNotification(removeMessage.handler, removeMessage.actionOnCompletion); break; case MSG_MOVE: MessageData moveMessage = (MessageData) Util.castNonNull(msg.obj); shuffleOrder = shuffleOrder.cloneAndRemove(moveMessage.index, moveMessage.index + 1); shuffleOrder = shuffleOrder.cloneAndInsert(moveMessage.customData, 1); moveMediaSourceInternal(moveMessage.index, moveMessage.customData); - scheduleListenerNotification(moveMessage.actionOnCompletion); + scheduleListenerNotification(moveMessage.handler, moveMessage.actionOnCompletion); break; case MSG_SET_SHUFFLE_ORDER: MessageData shuffleOrderMessage = (MessageData) Util.castNonNull(msg.obj); shuffleOrder = shuffleOrderMessage.customData; - scheduleListenerNotification(shuffleOrderMessage.actionOnCompletion); + scheduleListenerNotification( + shuffleOrderMessage.handler, shuffleOrderMessage.actionOnCompletion); break; case MSG_NOTIFY_LISTENER: notifyListener(); break; case MSG_ON_COMPLETION: - List actionsOnCompletion = (List) Util.castNonNull(msg.obj); - Handler handler = Assertions.checkNotNull(applicationThreadHandler); - for (int i = 0; i < actionsOnCompletion.size(); i++) { - handler.post(actionsOnCompletion.get(i)); - } + EventDispatcher actionsOnCompletion = + (EventDispatcher) Util.castNonNull(msg.obj); + actionsOnCompletion.dispatch(Runnable::run); break; default: throw new IllegalStateException(); @@ -607,34 +662,34 @@ public class ConcatenatingMediaSource extends CompositeMediaSource actionsOnCompletion = - pendingOnCompletionActions.isEmpty() - ? Collections.emptyList() - : new ArrayList<>(pendingOnCompletionActions); - pendingOnCompletionActions.clear(); + EventDispatcher actionsOnCompletion = pendingOnCompletionActions; + pendingOnCompletionActions = new EventDispatcher<>(); refreshSourceInfo( new ConcatenatedTimeline( mediaSourceHolders, windowCount, periodCount, shuffleOrder, isAtomic), /* manifest= */ null); - if (!actionsOnCompletion.isEmpty()) { - Assertions.checkNotNull(playbackThreadHandler) - .obtainMessage(MSG_ON_COMPLETION, actionsOnCompletion) - .sendToTarget(); - } + Assertions.checkNotNull(playbackThreadHandler) + .obtainMessage(MSG_ON_COMPLETION, actionsOnCompletion) + .sendToTarget(); } private void addMediaSourcesInternal( @@ -733,7 +788,7 @@ public class ConcatenatingMediaSource extends CompositeMediaSource mediaSource.addMediaSource(createFakeMediaSource(), timelineGrabber)); + () -> + mediaSource.addMediaSource(createFakeMediaSource(), new Handler(), timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.getWindowCount()).isEqualTo(1); } finally { @@ -495,6 +499,7 @@ public final class ConcatenatingMediaSourceTest { mediaSource.addMediaSources( Arrays.asList( new MediaSource[] {createFakeMediaSource(), createFakeMediaSource()}), + new Handler(), timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.getWindowCount()).isEqualTo(2); @@ -511,7 +516,8 @@ public final class ConcatenatingMediaSourceTest { final TimelineGrabber timelineGrabber = new TimelineGrabber(testRunner); dummyMainThread.runOnMainThread( () -> - mediaSource.addMediaSource(/* index */ 0, createFakeMediaSource(), timelineGrabber)); + mediaSource.addMediaSource( + /* index */ 0, createFakeMediaSource(), new Handler(), timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.getWindowCount()).isEqualTo(1); } finally { @@ -531,6 +537,7 @@ public final class ConcatenatingMediaSourceTest { /* index */ 0, Arrays.asList( new MediaSource[] {createFakeMediaSource(), createFakeMediaSource()}), + new Handler(), timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.getWindowCount()).isEqualTo(2); @@ -549,7 +556,7 @@ public final class ConcatenatingMediaSourceTest { final TimelineGrabber timelineGrabber = new TimelineGrabber(testRunner); dummyMainThread.runOnMainThread( - () -> mediaSource.removeMediaSource(/* index */ 0, timelineGrabber)); + () -> mediaSource.removeMediaSource(/* index */ 0, new Handler(), timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.getWindowCount()).isEqualTo(0); } finally { @@ -571,7 +578,9 @@ public final class ConcatenatingMediaSourceTest { final TimelineGrabber timelineGrabber = new TimelineGrabber(testRunner); dummyMainThread.runOnMainThread( - () -> mediaSource.moveMediaSource(/* fromIndex */ 1, /* toIndex */ 0, timelineGrabber)); + () -> + mediaSource.moveMediaSource( + /* fromIndex */ 1, /* toIndex */ 0, new Handler(), timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.getWindowCount()).isEqualTo(2); } finally { @@ -819,7 +828,7 @@ public final class ConcatenatingMediaSourceTest { testRunner.prepareSource(); final TimelineGrabber timelineGrabber = new TimelineGrabber(testRunner); - dummyMainThread.runOnMainThread(() -> mediaSource.clear(timelineGrabber)); + dummyMainThread.runOnMainThread(() -> mediaSource.clear(new Handler(), timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.isEmpty()).isTrue(); @@ -965,7 +974,8 @@ public final class ConcatenatingMediaSourceTest { @Test public void testCustomCallbackBeforePreparationSetShuffleOrder() throws Exception { Runnable runnable = Mockito.mock(Runnable.class); - mediaSource.setShuffleOrder(new ShuffleOrder.UnshuffledShuffleOrder(/* length= */ 0), runnable); + mediaSource.setShuffleOrder( + new ShuffleOrder.UnshuffledShuffleOrder(/* length= */ 0), new Handler(), runnable); verify(runnable).run(); } @@ -981,7 +991,9 @@ public final class ConcatenatingMediaSourceTest { dummyMainThread.runOnMainThread( () -> mediaSource.setShuffleOrder( - new ShuffleOrder.UnshuffledShuffleOrder(/* length= */ 3), timelineGrabber)); + new ShuffleOrder.UnshuffledShuffleOrder(/* length= */ 3), + new Handler(), + timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.getFirstWindowIndex(/* shuffleModeEnabled= */ true)).isEqualTo(0); } finally {