From 68c401b53e43c4891e37a4f8a1e5ceefdfd0c288 Mon Sep 17 00:00:00 2001 From: tonihei Date: Fri, 28 Feb 2020 12:11:48 +0000 Subject: [PATCH] Mark initial progressive source Timeline as placeholder This is more accurate since it's just a placeholder and none of the values is provided by the media. It also allows to fix a problem in ClippingMediaSource where we couldn't detect a clipping error because we didn't know if the timeline is a placeholder or not. Issue:#5924 PiperOrigin-RevId: 297813606 --- .../source/ClippingMediaSource.java | 6 +-- .../source/ProgressiveMediaSource.java | 38 ++++++++++++++----- .../source/ClippingMediaSourceTest.java | 25 ++++++++++-- 3 files changed, 54 insertions(+), 15 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ClippingMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ClippingMediaSource.java index 4780c075d5..d4ede3e59e 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ClippingMediaSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ClippingMediaSource.java @@ -319,14 +319,14 @@ public final class ClippingMediaSource extends CompositeMediaSource { } Window window = timeline.getWindow(0, new Window()); startUs = Math.max(0, startUs); + if (!window.isPlaceholder && startUs != 0 && !window.isSeekable) { + throw new IllegalClippingException(IllegalClippingException.REASON_NOT_SEEKABLE_TO_START); + } long resolvedEndUs = endUs == C.TIME_END_OF_SOURCE ? window.durationUs : Math.max(0, endUs); if (window.durationUs != C.TIME_UNSET) { if (resolvedEndUs > window.durationUs) { resolvedEndUs = window.durationUs; } - if (startUs != 0 && !window.isSeekable) { - throw new IllegalClippingException(IllegalClippingException.REASON_NOT_SEEKABLE_TO_START); - } if (startUs > resolvedEndUs) { throw new IllegalClippingException(IllegalClippingException.REASON_START_EXCEEDS_END); } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ProgressiveMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ProgressiveMediaSource.java index 16c4cc22c6..a1611d6723 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ProgressiveMediaSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ProgressiveMediaSource.java @@ -18,6 +18,7 @@ package com.google.android.exoplayer2.source; import android.net.Uri; import androidx.annotation.Nullable; import com.google.android.exoplayer2.C; +import com.google.android.exoplayer2.Timeline; import com.google.android.exoplayer2.drm.DrmSession; import com.google.android.exoplayer2.drm.DrmSessionManager; import com.google.android.exoplayer2.extractor.DefaultExtractorsFactory; @@ -204,6 +205,7 @@ public final class ProgressiveMediaSource extends BaseMediaSource private final int continueLoadingCheckIntervalBytes; @Nullable private final Object tag; + private boolean timelineIsPlaceholder; private long timelineDurationUs; private boolean timelineIsSeekable; private boolean timelineIsLive; @@ -226,6 +228,7 @@ public final class ProgressiveMediaSource extends BaseMediaSource this.loadableLoadErrorHandlingPolicy = loadableLoadErrorHandlingPolicy; this.customCacheKey = customCacheKey; this.continueLoadingCheckIntervalBytes = continueLoadingCheckIntervalBytes; + this.timelineIsPlaceholder = true; this.timelineDurationUs = C.TIME_UNSET; this.tag = tag; } @@ -240,7 +243,7 @@ public final class ProgressiveMediaSource extends BaseMediaSource protected void prepareSourceInternal(@Nullable TransferListener mediaTransferListener) { transferListener = mediaTransferListener; drmSessionManager.prepare(); - notifySourceInfoRefreshed(timelineDurationUs, timelineIsSeekable, timelineIsLive); + notifySourceInfoRefreshed(); } @Override @@ -283,30 +286,47 @@ public final class ProgressiveMediaSource extends BaseMediaSource public void onSourceInfoRefreshed(long durationUs, boolean isSeekable, boolean isLive) { // If we already have the duration from a previous source info refresh, use it. durationUs = durationUs == C.TIME_UNSET ? timelineDurationUs : durationUs; - if (timelineDurationUs == durationUs + if (!timelineIsPlaceholder + && timelineDurationUs == durationUs && timelineIsSeekable == isSeekable && timelineIsLive == isLive) { // Suppress no-op source info changes. return; } - notifySourceInfoRefreshed(durationUs, isSeekable, isLive); + timelineDurationUs = durationUs; + timelineIsSeekable = isSeekable; + timelineIsLive = isLive; + timelineIsPlaceholder = false; + notifySourceInfoRefreshed(); } // Internal methods. - private void notifySourceInfoRefreshed(long durationUs, boolean isSeekable, boolean isLive) { - timelineDurationUs = durationUs; - timelineIsSeekable = isSeekable; - timelineIsLive = isLive; + private void notifySourceInfoRefreshed() { // TODO: Split up isDynamic into multiple fields to indicate which values may change. Then // indicate that the duration may change until it's known. See [internal: b/69703223]. - refreshSourceInfo( + Timeline timeline = new SinglePeriodTimeline( timelineDurationUs, timelineIsSeekable, /* isDynamic= */ false, /* isLive= */ timelineIsLive, /* manifest= */ null, - tag)); + tag); + if (timelineIsPlaceholder) { + // TODO: Actually prepare the extractors during prepatation so that we don't need a + // placeholder. See https://github.com/google/ExoPlayer/issues/4727. + timeline = + new ForwardingTimeline(timeline) { + @Override + public Window getWindow( + int windowIndex, Window window, long defaultPositionProjectionUs) { + super.getWindow(windowIndex, window, defaultPositionProjectionUs); + window.isPlaceholder = true; + return window; + } + }; + } + refreshSourceInfo(timeline); } } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/source/ClippingMediaSourceTest.java b/library/core/src/test/java/com/google/android/exoplayer2/source/ClippingMediaSourceTest.java index d93665e735..ae0c431bd3 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/source/ClippingMediaSourceTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/source/ClippingMediaSourceTest.java @@ -26,6 +26,7 @@ import com.google.android.exoplayer2.Timeline; import com.google.android.exoplayer2.Timeline.Period; import com.google.android.exoplayer2.Timeline.Window; import com.google.android.exoplayer2.source.ClippingMediaSource.IllegalClippingException; +import com.google.android.exoplayer2.source.MaskingMediaSource.DummyTimeline; import com.google.android.exoplayer2.source.MediaSource.MediaPeriodId; import com.google.android.exoplayer2.source.MediaSourceEventListener.EventDispatcher; import com.google.android.exoplayer2.testutil.FakeMediaPeriod; @@ -100,6 +101,26 @@ public final class ClippingMediaSourceTest { } } + @Test + public void clippingUnseekableWindowWithUnknownDurationThrows() throws IOException { + Timeline timeline = + new SinglePeriodTimeline( + /* durationUs= */ C.TIME_UNSET, + /* isSeekable= */ false, + /* isDynamic= */ false, + /* isLive= */ false); + + // If the unseekable window isn't clipped, clipping succeeds. + getClippedTimeline(timeline, /* startUs= */ 0, TEST_PERIOD_DURATION_US); + try { + // If the unseekable window is clipped, clipping fails. + getClippedTimeline(timeline, /* startUs= */ 1, TEST_PERIOD_DURATION_US); + fail("Expected clipping to fail."); + } catch (IllegalClippingException e) { + assertThat(e.reason).isEqualTo(IllegalClippingException.REASON_NOT_SEEKABLE_TO_START); + } + } + @Test public void clippingStart() throws IOException { Timeline timeline = @@ -139,9 +160,7 @@ public final class ClippingMediaSourceTest { // Timeline that's dynamic and not seekable. A child source might report such a timeline prior // to it having loaded sufficient data to establish its duration and seekability. Such timelines // should not result in clipping failure. - Timeline timeline = - new SinglePeriodTimeline( - C.TIME_UNSET, /* isSeekable= */ false, /* isDynamic= */ true, /* isLive= */ true); + Timeline timeline = new DummyTimeline(/* tag= */ null); Timeline clippedTimeline = getClippedTimeline(