From 0d052e0399e82355f53c77779a2053aa264cbac3 Mon Sep 17 00:00:00 2001 From: olly Date: Mon, 15 Mar 2021 10:42:34 +0000 Subject: [PATCH] HLS: Allow audio variants to initialize the timestamp adjuster This makes HLS playback less liable to become stuck if discontinuity tags are inserted at different times across media playlists. Issue: #8700 Issue: #8372 PiperOrigin-RevId: 362903428 --- RELEASENOTES.md | 7 +- .../exoplayer2/util/TimestampAdjuster.java | 110 +++++++++++------- .../exoplayer2/extractor/ts/PsExtractor.java | 3 +- .../exoplayer2/extractor/ts/TsExtractor.java | 3 +- .../exoplayer2/source/hls/HlsMediaChunk.java | 13 +-- .../exoplayer2/source/hls/HlsMediaPeriod.java | 14 ++- 6 files changed, 94 insertions(+), 56 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 12ff74f4236..b96ae729c82 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -24,7 +24,7 @@ * `DebugTextViewHelper` moved from `ui` package to `util` package. * Spherical UI components moved from `video.spherical` package to `ui.spherical` package, and made package private. -* Core +* Core: * Move `getRendererCount` and `getRendererType` methods from `Player` to `ExoPlayer`. * Reset playback speed when live playback speed control becomes unused @@ -32,6 +32,11 @@ * Fix playback position issue when re-preparing playback after a BehindLiveWindowException ([#8675](https://github.com/google/ExoPlayer/issues/8675)). +* HLS: + * Fix issue that could cause playback to become stuck if corresponding + `EXT-X-DISCONTINUITY` tags in different media playlists occur at + different positions in time + ([#8372](https://github.com/google/ExoPlayer/issues/8372)). * Remove deprecated symbols: * Remove `Player.DefaultEventListener`. Use `Player.EventListener` instead. diff --git a/library/common/src/main/java/com/google/android/exoplayer2/util/TimestampAdjuster.java b/library/common/src/main/java/com/google/android/exoplayer2/util/TimestampAdjuster.java index 65f88b19832..a76cf9b5123 100644 --- a/library/common/src/main/java/com/google/android/exoplayer2/util/TimestampAdjuster.java +++ b/library/common/src/main/java/com/google/android/exoplayer2/util/TimestampAdjuster.java @@ -15,6 +15,7 @@ */ package com.google.android.exoplayer2.util; +import androidx.annotation.GuardedBy; import com.google.android.exoplayer2.C; /** @@ -35,34 +36,73 @@ public final class TimestampAdjuster { */ private static final long MAX_PTS_PLUS_ONE = 0x200000000L; + @GuardedBy("this") + private boolean sharedInitializationStarted; + + @GuardedBy("this") private long firstSampleTimestampUs; + + @GuardedBy("this") private long timestampOffsetUs; - // Volatile to allow isInitialized to be called on a different thread to adjustSampleTimestamp. - private volatile long lastSampleTimestampUs; + @GuardedBy("this") + private long lastSampleTimestampUs; /** - * @param firstSampleTimestampUs See {@link #setFirstSampleTimestampUs(long)}. + * @param firstSampleTimestampUs The desired value of the first adjusted sample timestamp in + * microseconds, or {@link #DO_NOT_OFFSET} if timestamps should not be offset. */ public TimestampAdjuster(long firstSampleTimestampUs) { + this.firstSampleTimestampUs = firstSampleTimestampUs; lastSampleTimestampUs = C.TIME_UNSET; - setFirstSampleTimestampUs(firstSampleTimestampUs); } /** - * Sets the desired result of the first call to {@link #adjustSampleTimestamp(long)}. Can only be - * called before any timestamps have been adjusted. + * For shared timestamp adjusters, performs necessary initialization actions for a caller. * - * @param firstSampleTimestampUs The first adjusted sample timestamp in microseconds, or - * {@link #DO_NOT_OFFSET} if presentation timestamps should not be offset. + * + * + * @param canInitialize Whether the caller is able to initialize the adjuster, if needed. + * @param startTimeUs The desired first sample timestamp of the caller, in microseconds. Only used + * if {@code canInitialize} is {@code true}. + * @throws InterruptedException If the thread is interrupted whilst blocked waiting for + * initialization to complete. */ - public synchronized void setFirstSampleTimestampUs(long firstSampleTimestampUs) { - Assertions.checkState(lastSampleTimestampUs == C.TIME_UNSET); - this.firstSampleTimestampUs = firstSampleTimestampUs; + public synchronized void sharedInitializeOrWait(boolean canInitialize, long startTimeUs) + throws InterruptedException { + if (canInitialize && !sharedInitializationStarted) { + firstSampleTimestampUs = startTimeUs; + sharedInitializationStarted = true; + } + if (!canInitialize || startTimeUs != firstSampleTimestampUs) { + while (lastSampleTimestampUs == C.TIME_UNSET) { + wait(); + } + } } - /** Returns the last value passed to {@link #setFirstSampleTimestampUs(long)}. */ - public long getFirstSampleTimestampUs() { + /** + * Returns the value of the first adjusted sample timestamp in microseconds, or {@link + * #DO_NOT_OFFSET} if timestamps will not be offset. + */ + public synchronized long getFirstSampleTimestampUs() { return firstSampleTimestampUs; } @@ -72,22 +112,22 @@ public long getFirstSampleTimestampUs() { * #getFirstSampleTimestampUs()}. If this value is {@link #DO_NOT_OFFSET}, returns {@link * C#TIME_UNSET}. */ - public long getLastAdjustedTimestampUs() { + public synchronized long getLastAdjustedTimestampUs() { return lastSampleTimestampUs != C.TIME_UNSET ? (lastSampleTimestampUs + timestampOffsetUs) : firstSampleTimestampUs != DO_NOT_OFFSET ? firstSampleTimestampUs : C.TIME_UNSET; } /** - * Returns the offset between the input of {@link #adjustSampleTimestamp(long)} and its output. - * If {@link #DO_NOT_OFFSET} was provided to the constructor, 0 is returned. If the timestamp + * Returns the offset between the input of {@link #adjustSampleTimestamp(long)} and its output. If + * {@link #DO_NOT_OFFSET} was provided to the constructor, 0 is returned. If the timestamp * adjuster is yet not initialized, {@link C#TIME_UNSET} is returned. * - * @return The offset between {@link #adjustSampleTimestamp(long)}'s input and output. - * {@link C#TIME_UNSET} if the adjuster is not yet initialized and 0 if timestamps should not - * be offset. + * @return The offset between {@link #adjustSampleTimestamp(long)}'s input and output. {@link + * C#TIME_UNSET} if the adjuster is not yet initialized and 0 if timestamps should not be + * offset. */ - public long getTimestampOffsetUs() { + public synchronized long getTimestampOffsetUs() { return firstSampleTimestampUs == DO_NOT_OFFSET ? 0 : lastSampleTimestampUs == C.TIME_UNSET ? C.TIME_UNSET : timestampOffsetUs; @@ -95,9 +135,14 @@ public long getTimestampOffsetUs() { /** * Resets the instance to its initial state. + * + * @param firstSampleTimestampUs The desired value of the first adjusted sample timestamp after + * this reset, in microseconds, or {@link #DO_NOT_OFFSET} if timestamps should not be offset. */ - public void reset() { + public synchronized void reset(long firstSampleTimestampUs) { + this.firstSampleTimestampUs = firstSampleTimestampUs; lastSampleTimestampUs = C.TIME_UNSET; + sharedInitializationStarted = false; } /** @@ -106,7 +151,7 @@ public void reset() { * @param pts90Khz A 90 kHz clock MPEG-2 TS presentation timestamp. * @return The adjusted timestamp in microseconds. */ - public long adjustTsTimestamp(long pts90Khz) { + public synchronized long adjustTsTimestamp(long pts90Khz) { if (pts90Khz == C.TIME_UNSET) { return C.TIME_UNSET; } @@ -131,7 +176,7 @@ public long adjustTsTimestamp(long pts90Khz) { * @param timeUs The timestamp to adjust in microseconds. * @return The adjusted timestamp in microseconds. */ - public long adjustSampleTimestamp(long timeUs) { + public synchronized long adjustSampleTimestamp(long timeUs) { if (timeUs == C.TIME_UNSET) { return C.TIME_UNSET; } @@ -143,26 +188,13 @@ public long adjustSampleTimestamp(long timeUs) { // Calculate the timestamp offset. timestampOffsetUs = firstSampleTimestampUs - timeUs; } - synchronized (this) { - lastSampleTimestampUs = timeUs; - // Notify threads waiting for this adjuster to be initialized. - notifyAll(); - } + lastSampleTimestampUs = timeUs; + // Notify threads waiting for this adjuster to be initialized. + notifyAll(); } return timeUs + timestampOffsetUs; } - /** - * Blocks the calling thread until this adjuster is initialized. - * - * @throws InterruptedException If the thread was interrupted. - */ - public synchronized void waitUntilInitialized() throws InterruptedException { - while (lastSampleTimestampUs == C.TIME_UNSET) { - wait(); - } - } - /** * Converts a 90 kHz clock timestamp to a timestamp in microseconds. * diff --git a/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ts/PsExtractor.java b/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ts/PsExtractor.java index 4ead98febbf..ec6a8cca65d 100644 --- a/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ts/PsExtractor.java +++ b/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ts/PsExtractor.java @@ -144,8 +144,7 @@ public void seek(long position, long timeUs) { // we have to set the first sample timestamp manually. // - If the timestamp adjuster has its timestamp set manually before, and now we seek to a // different position, we need to set the first sample timestamp manually again. - timestampAdjuster.reset(); - timestampAdjuster.setFirstSampleTimestampUs(timeUs); + timestampAdjuster.reset(timeUs); } if (psBinarySearchSeeker != null) { diff --git a/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ts/TsExtractor.java b/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ts/TsExtractor.java index 2a9613f7f40..8266400971d 100644 --- a/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ts/TsExtractor.java +++ b/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ts/TsExtractor.java @@ -268,8 +268,7 @@ public void seek(long position, long timeUs) { // sample timestamp for that track manually. // - If the timestamp adjuster has its timestamp set manually before, and now we seek to a // different position, we need to set the first sample timestamp manually again. - timestampAdjuster.reset(); - timestampAdjuster.setFirstSampleTimestampUs(timeUs); + timestampAdjuster.reset(timeUs); } } if (timeUs != 0 && tsBinarySearchSeeker != null) { diff --git a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsMediaChunk.java b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsMediaChunk.java index 30e8350982b..81cfcd2ef76 100644 --- a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsMediaChunk.java +++ b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/HlsMediaChunk.java @@ -391,15 +391,10 @@ private void maybeLoadInitData() throws IOException { @RequiresNonNull("output") private void loadMedia() throws IOException { - if (!isMasterTimestampSource) { - try { - timestampAdjuster.waitUntilInitialized(); - } catch (InterruptedException e) { - throw new InterruptedIOException(); - } - } else if (timestampAdjuster.getFirstSampleTimestampUs() == TimestampAdjuster.DO_NOT_OFFSET) { - // We're the master and we haven't set the desired first sample timestamp yet. - timestampAdjuster.setFirstSampleTimestampUs(startTimeUs); + try { + timestampAdjuster.sharedInitializeOrWait(isMasterTimestampSource, startTimeUs); + } catch (InterruptedException e) { + throw new InterruptedIOException(); } feedDataToExtractor(dataSource, dataSpec, mediaSegmentEncrypted); } 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 9d643ea9264..a4db3d9c528 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 @@ -88,6 +88,7 @@ public final class HlsMediaPeriod implements MediaPeriod, HlsSampleStreamWrapper private HlsSampleStreamWrapper[] enabledSampleStreamWrappers; // Maps sample stream wrappers to variant/rendition index by matching array positions. private int[][] manifestUrlIndicesPerWrapper; + private int audioVideoSampleStreamWrapperCount; private SequenceableLoader compositeSequenceableLoader; /** @@ -315,8 +316,9 @@ public long selectTracks( if (wrapperEnabled) { newEnabledSampleStreamWrappers[newEnabledSampleStreamWrapperCount] = sampleStreamWrapper; if (newEnabledSampleStreamWrapperCount++ == 0) { - // The first enabled wrapper is responsible for initializing timestamp adjusters. This - // way, if enabled, variants are responsible. Else audio renditions. Else text renditions. + // The first enabled wrapper is always allowed to initialize timestamp adjusters. Note + // that the first wrapper will correspond to a variant, or else an audio rendition, or + // else a text rendition, in that order. sampleStreamWrapper.setIsTimestampMaster(true); if (wasReset || enabledSampleStreamWrappers.length == 0 || sampleStreamWrapper != enabledSampleStreamWrappers[0]) { @@ -326,7 +328,11 @@ public long selectTracks( forceReset = true; } } else { - sampleStreamWrapper.setIsTimestampMaster(false); + // Additional wrappers are also allowed to initialize timestamp adjusters if they contain + // audio or video, since they are expected to contain dense samples. Text wrappers are not + // permitted except in the case above in which no variant or audio rendition wrappers are + // enabled. + sampleStreamWrapper.setIsTimestampMaster(i < audioVideoSampleStreamWrapperCount); } } } @@ -496,6 +502,8 @@ private void buildAndPrepareSampleStreamWrappers(long positionUs) { manifestUrlIndicesPerWrapper, overridingDrmInitData); + audioVideoSampleStreamWrapperCount = sampleStreamWrappers.size(); + // Subtitle stream wrappers. We can always use master playlist information to prepare these. for (int i = 0; i < subtitleRenditions.size(); i++) { Rendition subtitleRendition = subtitleRenditions.get(i);