From f0bb6bb263e39e8e37a4c96b6fb01592c2dfd8ea Mon Sep 17 00:00:00 2001 From: aquilescanta Date: Tue, 27 Mar 2018 04:13:18 -0700 Subject: [PATCH] Make MediaPeriod implementations compliant with its specification Currently, MediaPeriod states that continueLoading may be called during preparation. Some implementations would throw an error if this happened. Also make MediaPeriod documentation clearer. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=190596870 --- .../exoplayer2/source/MediaPeriod.java | 24 ++++++------- .../exoplayer2/source/MergingMediaPeriod.java | 28 ++++++++++----- .../exoplayer2/source/hls/HlsMediaPeriod.java | 35 +++++++------------ .../exoplayer2/testutil/FakeMediaPeriod.java | 1 - 4 files changed, 44 insertions(+), 44 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/MediaPeriod.java b/library/core/src/main/java/com/google/android/exoplayer2/source/MediaPeriod.java index 8d7bd768bb..997f94bbfe 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/MediaPeriod.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/MediaPeriod.java @@ -67,7 +67,7 @@ public interface MediaPeriod extends SequenceableLoader { * Throws an error that's preventing the period from becoming prepared. Does nothing if no such * error exists. * - *

This method should only be called before the period has completed preparation. + *

This method is only called before the period has completed preparation. * * @throws IOException The underlying error. */ @@ -76,7 +76,7 @@ public interface MediaPeriod extends SequenceableLoader { /** * Returns the {@link TrackGroup}s exposed by the period. * - *

This method should only be called after the period has been prepared. + *

This method is only called after the period has been prepared. * * @return The {@link TrackGroup}s. */ @@ -93,7 +93,7 @@ public interface MediaPeriod extends SequenceableLoader { * corresponding flag in {@code streamResetFlags} will be set to true. This flag will also be set * if a new sample stream is created. * - *

This method should only be called after the period has been prepared. + *

This method is only called after the period has been prepared. * * @param selections The renderer track selections. * @param mayRetainStreamFlags Flags indicating whether the existing sample stream can be retained @@ -117,7 +117,7 @@ public interface MediaPeriod extends SequenceableLoader { /** * Discards buffered media up to the specified position. * - *

This method should only be called after the period has been prepared. + *

This method is only called after the period has been prepared. * * @param positionUs The position in microseconds. * @param toKeyframe If true then for each track discards samples up to the keyframe before or at @@ -131,8 +131,8 @@ public interface MediaPeriod extends SequenceableLoader { *

After this method has returned a value other than {@link C#TIME_UNSET}, all {@link * SampleStream}s provided by the period are guaranteed to start from a key frame. * - *

This method should only be called after the period has been prepared. It must be called - * before attempting to read from any {@link SampleStream}s provided by the period. + *

This method is only called after the period has been prepared and before reading from any + * {@link SampleStream}s provided by the period. * * @return If a discontinuity was read then the playback position in microseconds after the * discontinuity. Else {@link C#TIME_UNSET}. @@ -145,7 +145,7 @@ public interface MediaPeriod extends SequenceableLoader { *

After this method has been called, all {@link SampleStream}s provided by the period are * guaranteed to start from a key frame. * - *

This method should only be called when at least one track is selected. + *

This method is only called when at least one track is selected. * * @param positionUs The seek position in microseconds. * @return The actual position to which the period was seeked, in microseconds. @@ -156,7 +156,7 @@ public interface MediaPeriod extends SequenceableLoader { * Returns the position to which a seek will be performed, given the specified seek position and * {@link SeekParameters}. * - *

This method should only be called after the period has been prepared. + *

This method is only called after the period has been prepared. * * @param positionUs The seek position in microseconds. * @param seekParameters Parameters that control how the seek is performed. Implementations may @@ -170,7 +170,7 @@ public interface MediaPeriod extends SequenceableLoader { /** * Returns an estimate of the position up to which data is buffered for the enabled tracks. * - *

This method should only be called when at least one track is selected. + *

This method is only called when at least one track is selected. * * @return An estimate of the absolute position in microseconds up to which data is buffered, or * {@link C#TIME_END_OF_SOURCE} if the track is fully buffered. @@ -181,8 +181,8 @@ public interface MediaPeriod extends SequenceableLoader { /** * Returns the next load time, or {@link C#TIME_END_OF_SOURCE} if loading has finished. * - *

This method should only be called after the period has been prepared. It may be called when - * no tracks are selected. + *

This method is only called after the period has been prepared. It may be called when no + * tracks are selected. */ @Override long getNextLoadPositionUs(); @@ -209,7 +209,7 @@ public interface MediaPeriod extends SequenceableLoader { /** * Re-evaluates the buffer given the playback position. * - *

This method should only be called after the period has been prepared. + *

This method is only called after the period has been prepared. * *

A period may choose to discard buffered media so that it can be re-buffered in a different * quality. diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/MergingMediaPeriod.java b/library/core/src/main/java/com/google/android/exoplayer2/source/MergingMediaPeriod.java index d8b64fff7e..a4fc8c6b00 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/MergingMediaPeriod.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/MergingMediaPeriod.java @@ -21,6 +21,7 @@ import com.google.android.exoplayer2.trackselection.TrackSelection; import com.google.android.exoplayer2.util.Assertions; import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.IdentityHashMap; /** @@ -32,9 +33,9 @@ import java.util.IdentityHashMap; private final IdentityHashMap streamPeriodIndices; private final CompositeSequenceableLoaderFactory compositeSequenceableLoaderFactory; + private final ArrayList childrenPendingPreparation; private Callback callback; - private int pendingChildPrepareCount; private TrackGroupArray trackGroups; private MediaPeriod[] enabledPeriods; @@ -44,13 +45,16 @@ import java.util.IdentityHashMap; MediaPeriod... periods) { this.compositeSequenceableLoaderFactory = compositeSequenceableLoaderFactory; this.periods = periods; + childrenPendingPreparation = new ArrayList<>(); + compositeSequenceableLoader = + compositeSequenceableLoaderFactory.createCompositeSequenceableLoader(); streamPeriodIndices = new IdentityHashMap<>(); } @Override public void prepare(Callback callback, long positionUs) { this.callback = callback; - pendingChildPrepareCount = periods.length; + Collections.addAll(childrenPendingPreparation, periods); for (MediaPeriod period : periods) { period.prepare(this, positionUs); } @@ -147,7 +151,16 @@ import java.util.IdentityHashMap; @Override public boolean continueLoading(long positionUs) { - return compositeSequenceableLoader.continueLoading(positionUs); + if (!childrenPendingPreparation.isEmpty()) { + // Preparation is still going on. + int childrenPendingPreparationSize = childrenPendingPreparation.size(); + for (int i = 0; i < childrenPendingPreparationSize; i++) { + childrenPendingPreparation.get(i).continueLoading(positionUs); + } + return false; + } else { + return compositeSequenceableLoader.continueLoading(positionUs); + } } @Override @@ -201,8 +214,9 @@ import java.util.IdentityHashMap; // MediaPeriod.Callback implementation @Override - public void onPrepared(MediaPeriod ignored) { - if (--pendingChildPrepareCount > 0) { + public void onPrepared(MediaPeriod preparedPeriod) { + childrenPendingPreparation.remove(preparedPeriod); + if (!childrenPendingPreparation.isEmpty()) { return; } int totalTrackGroupCount = 0; @@ -224,10 +238,6 @@ import java.util.IdentityHashMap; @Override public void onContinueLoadingRequested(MediaPeriod ignored) { - if (trackGroups == null) { - // Still preparing. - return; - } callback.onContinueLoadingRequested(this); } diff --git a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsMediaPeriod.java b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsMediaPeriod.java index 8076fd8359..01b042e7a5 100644 --- a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsMediaPeriod.java +++ b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsMediaPeriod.java @@ -15,7 +15,6 @@ */ package com.google.android.exoplayer2.source.hls; -import android.os.Handler; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.Format; import com.google.android.exoplayer2.SeekParameters; @@ -55,7 +54,6 @@ public final class HlsMediaPeriod implements MediaPeriod, HlsSampleStreamWrapper private final Allocator allocator; private final IdentityHashMap streamWrapperIndices; private final TimestampAdjusterProvider timestampAdjusterProvider; - private final Handler continueLoadingHandler; private final CompositeSequenceableLoaderFactory compositeSequenceableLoaderFactory; private final boolean allowChunklessPreparation; @@ -84,9 +82,10 @@ public final class HlsMediaPeriod implements MediaPeriod, HlsSampleStreamWrapper this.allocator = allocator; this.compositeSequenceableLoaderFactory = compositeSequenceableLoaderFactory; this.allowChunklessPreparation = allowChunklessPreparation; + compositeSequenceableLoader = + compositeSequenceableLoaderFactory.createCompositeSequenceableLoader(); streamWrapperIndices = new IdentityHashMap<>(); timestampAdjusterProvider = new TimestampAdjusterProvider(); - continueLoadingHandler = new Handler(); sampleStreamWrappers = new HlsSampleStreamWrapper[0]; enabledSampleStreamWrappers = new HlsSampleStreamWrapper[0]; eventDispatcher.mediaPeriodCreated(); @@ -94,7 +93,6 @@ public final class HlsMediaPeriod implements MediaPeriod, HlsSampleStreamWrapper public void release() { playlistTracker.removeListener(this); - continueLoadingHandler.removeCallbacksAndMessages(null); for (HlsSampleStreamWrapper sampleStreamWrapper : sampleStreamWrappers) { sampleStreamWrapper.release(); } @@ -214,7 +212,15 @@ public final class HlsMediaPeriod implements MediaPeriod, HlsSampleStreamWrapper @Override public boolean continueLoading(long positionUs) { - return compositeSequenceableLoader.continueLoading(positionUs); + if (trackGroups == null) { + // Preparation is still going on. + for (HlsSampleStreamWrapper wrapper : sampleStreamWrappers) { + wrapper.continuePreparing(); + } + return false; + } else { + return compositeSequenceableLoader.continueLoading(positionUs); + } } @Override @@ -288,10 +294,6 @@ public final class HlsMediaPeriod implements MediaPeriod, HlsSampleStreamWrapper @Override public void onContinueLoadingRequested(HlsSampleStreamWrapper sampleStreamWrapper) { - if (trackGroups == null) { - // Still preparing. - return; - } callback.onContinueLoadingRequested(this); } @@ -299,7 +301,7 @@ public final class HlsMediaPeriod implements MediaPeriod, HlsSampleStreamWrapper @Override public void onPlaylistChanged() { - continuePreparingOrLoading(); + callback.onContinueLoadingRequested(this); } @Override @@ -307,7 +309,7 @@ public final class HlsMediaPeriod implements MediaPeriod, HlsSampleStreamWrapper for (HlsSampleStreamWrapper streamWrapper : sampleStreamWrappers) { streamWrapper.onPlaylistBlacklisted(url, blacklistMs); } - continuePreparingOrLoading(); + callback.onContinueLoadingRequested(this); } // Internal methods. @@ -475,17 +477,6 @@ public final class HlsMediaPeriod implements MediaPeriod, HlsSampleStreamWrapper muxedAudioFormat, minLoadableRetryCount, eventDispatcher); } - private void continuePreparingOrLoading() { - if (trackGroups != null) { - callback.onContinueLoadingRequested(this); - } else { - // Some of the wrappers were waiting for their media playlist to prepare. - for (HlsSampleStreamWrapper wrapper : sampleStreamWrappers) { - wrapper.continuePreparing(); - } - } - } - private static Format deriveVideoFormat(Format variantFormat) { String codecs = Util.getCodecsOfType(variantFormat.codecs, C.TRACK_TYPE_VIDEO); String mimeType = MimeTypes.getMediaMimeType(codecs); diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaPeriod.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaPeriod.java index 40d9b1ef9f..a251bd5ef0 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaPeriod.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaPeriod.java @@ -219,7 +219,6 @@ public class FakeMediaPeriod implements MediaPeriod { @Override public boolean continueLoading(long positionUs) { - assertThat(prepared).isTrue(); return false; }