Commit 76a05ce3 by aquilescanta Committed by Oliver Woodman

Fix race condition in timestamp adjustment for HLS

If a Webvtt HlsChunkSource got to schedule its chunk load before the
master HlsChunkSource (the one that downloads the TS or the fMP4
chunks), the player would never get past the buffering state.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=132985792
parent 20757a19
...@@ -59,13 +59,6 @@ public final class TimestampAdjuster { ...@@ -59,13 +59,6 @@ public final class TimestampAdjuster {
} }
/** /**
* Whether this adjuster has been initialized with a first MPEG-2 TS presentation timestamp.
*/
public boolean isInitialized() {
return lastSampleTimestamp != C.TIME_UNSET;
}
/**
* Scales and offsets an MPEG-2 TS presentation timestamp considering wraparound. * Scales and offsets an MPEG-2 TS presentation timestamp considering wraparound.
* *
* @param pts The MPEG-2 TS presentation timestamp. * @param pts The MPEG-2 TS presentation timestamp.
...@@ -92,16 +85,35 @@ public final class TimestampAdjuster { ...@@ -92,16 +85,35 @@ public final class TimestampAdjuster {
* @return The adjusted timestamp in microseconds. * @return The adjusted timestamp in microseconds.
*/ */
public long adjustSampleTimestamp(long timeUs) { public long adjustSampleTimestamp(long timeUs) {
if (firstSampleTimestampUs != DO_NOT_OFFSET && lastSampleTimestamp == C.TIME_UNSET) {
// Calculate the timestamp offset.
timestampOffsetUs = firstSampleTimestampUs - timeUs;
}
// Record the adjusted PTS to adjust for wraparound next time. // Record the adjusted PTS to adjust for wraparound next time.
lastSampleTimestamp = timeUs; if (lastSampleTimestamp != C.TIME_UNSET) {
lastSampleTimestamp = timeUs;
} else {
if (firstSampleTimestampUs != DO_NOT_OFFSET) {
// Calculate the timestamp offset.
timestampOffsetUs = firstSampleTimestampUs - timeUs;
}
synchronized (this) {
lastSampleTimestamp = timeUs;
// Notify threads waiting for this adjuster to be initialized.
notifyAll();
}
}
return timeUs + timestampOffsetUs; 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 (lastSampleTimestamp == C.TIME_UNSET) {
wait();
}
}
/**
* Converts a value in MPEG-2 timestamp units to the corresponding value in microseconds. * Converts a value in MPEG-2 timestamp units to the corresponding value in microseconds.
* *
* @param pts A value in MPEG-2 timestamp units. * @param pts A value in MPEG-2 timestamp units.
......
...@@ -329,6 +329,8 @@ import java.util.Locale; ...@@ -329,6 +329,8 @@ import java.util.Locale;
// Configure the extractor that will read the chunk. // Configure the extractor that will read the chunk.
Extractor extractor; Extractor extractor;
boolean extractorNeedsInit = true; boolean extractorNeedsInit = true;
boolean isTimestampMaster = false;
TimestampAdjuster timestampAdjuster = null;
String lastPathSegment = chunkUri.getLastPathSegment(); String lastPathSegment = chunkUri.getLastPathSegment();
if (lastPathSegment.endsWith(AAC_FILE_EXTENSION)) { if (lastPathSegment.endsWith(AAC_FILE_EXTENSION)) {
// TODO: Inject a timestamp adjuster and use it along with ID3 PRIV tag values with owner // TODO: Inject a timestamp adjuster and use it along with ID3 PRIV tag values with owner
...@@ -342,25 +344,16 @@ import java.util.Locale; ...@@ -342,25 +344,16 @@ import java.util.Locale;
extractor = new Mp3Extractor(startTimeUs); extractor = new Mp3Extractor(startTimeUs);
} else if (lastPathSegment.endsWith(WEBVTT_FILE_EXTENSION) } else if (lastPathSegment.endsWith(WEBVTT_FILE_EXTENSION)
|| lastPathSegment.endsWith(VTT_FILE_EXTENSION)) { || lastPathSegment.endsWith(VTT_FILE_EXTENSION)) {
TimestampAdjuster timestampAdjuster = timestampAdjusterProvider.getAdjuster(false, timestampAdjuster = timestampAdjusterProvider.getAdjuster(segment.discontinuitySequenceNumber,
segment.discontinuitySequenceNumber, startTimeUs); startTimeUs);
if (timestampAdjuster == null) {
// The master source has yet to instantiate an adjuster for the discontinuity sequence.
// TODO: There's probably an edge case if the master starts playback at a chunk belonging to
// a discontinuity sequence greater than the one that this source is trying to start at.
return;
}
extractor = new WebvttExtractor(format.language, timestampAdjuster); extractor = new WebvttExtractor(format.language, timestampAdjuster);
} else if (previous == null } else if (previous == null
|| previous.discontinuitySequenceNumber != segment.discontinuitySequenceNumber || previous.discontinuitySequenceNumber != segment.discontinuitySequenceNumber
|| format != previous.trackFormat) { || format != previous.trackFormat) {
// MPEG-2 TS segments, but we need a new extractor. // MPEG-2 TS segments, but we need a new extractor.
TimestampAdjuster timestampAdjuster = timestampAdjusterProvider.getAdjuster(true, isTimestampMaster = true;
segment.discontinuitySequenceNumber, startTimeUs); timestampAdjuster = timestampAdjusterProvider.getAdjuster(segment.discontinuitySequenceNumber,
if (timestampAdjuster == null) { startTimeUs);
// The master source has yet to instantiate an adjuster for the discontinuity sequence.
return;
}
// This flag ensures the change of pid between streams does not affect the sample queues. // This flag ensures the change of pid between streams does not affect the sample queues.
int workaroundFlags = TsExtractor.WORKAROUND_MAP_BY_TYPE; int workaroundFlags = TsExtractor.WORKAROUND_MAP_BY_TYPE;
String codecs = variants[newVariantIndex].format.codecs; String codecs = variants[newVariantIndex].format.codecs;
...@@ -384,8 +377,9 @@ import java.util.Locale; ...@@ -384,8 +377,9 @@ import java.util.Locale;
out.chunk = new HlsMediaChunk(dataSource, dataSpec, format, out.chunk = new HlsMediaChunk(dataSource, dataSpec, format,
trackSelection.getSelectionReason(), trackSelection.getSelectionData(), trackSelection.getSelectionReason(), trackSelection.getSelectionData(),
startTimeUs, endTimeUs, chunkMediaSequence, segment.discontinuitySequenceNumber, extractor, startTimeUs, endTimeUs, chunkMediaSequence, segment.discontinuitySequenceNumber,
extractorNeedsInit, switchingVariant, encryptionKey, encryptionIv); isTimestampMaster, timestampAdjuster, extractor, extractorNeedsInit, switchingVariant,
encryptionKey, encryptionIv);
} }
/** /**
......
...@@ -19,6 +19,7 @@ import com.google.android.exoplayer2.Format; ...@@ -19,6 +19,7 @@ import com.google.android.exoplayer2.Format;
import com.google.android.exoplayer2.extractor.DefaultExtractorInput; import com.google.android.exoplayer2.extractor.DefaultExtractorInput;
import com.google.android.exoplayer2.extractor.Extractor; import com.google.android.exoplayer2.extractor.Extractor;
import com.google.android.exoplayer2.extractor.ExtractorInput; import com.google.android.exoplayer2.extractor.ExtractorInput;
import com.google.android.exoplayer2.extractor.ts.TimestampAdjuster;
import com.google.android.exoplayer2.source.chunk.MediaChunk; import com.google.android.exoplayer2.source.chunk.MediaChunk;
import com.google.android.exoplayer2.upstream.DataSource; import com.google.android.exoplayer2.upstream.DataSource;
import com.google.android.exoplayer2.upstream.DataSpec; import com.google.android.exoplayer2.upstream.DataSpec;
...@@ -51,6 +52,8 @@ import java.util.concurrent.atomic.AtomicInteger; ...@@ -51,6 +52,8 @@ import java.util.concurrent.atomic.AtomicInteger;
private final boolean isEncrypted; private final boolean isEncrypted;
private final boolean extractorNeedsInit; private final boolean extractorNeedsInit;
private final boolean shouldSpliceIn; private final boolean shouldSpliceIn;
private final boolean isMasterTimestampSource;
private final TimestampAdjuster timestampAdjuster;
private int bytesLoaded; private int bytesLoaded;
private HlsSampleStreamWrapper extractorOutput; private HlsSampleStreamWrapper extractorOutput;
...@@ -68,6 +71,8 @@ import java.util.concurrent.atomic.AtomicInteger; ...@@ -68,6 +71,8 @@ import java.util.concurrent.atomic.AtomicInteger;
* @param endTimeUs The end time of the media contained by the chunk, in microseconds. * @param endTimeUs The end time of the media contained by the chunk, in microseconds.
* @param chunkIndex The media sequence number of the chunk. * @param chunkIndex The media sequence number of the chunk.
* @param discontinuitySequenceNumber The discontinuity sequence number of the chunk. * @param discontinuitySequenceNumber The discontinuity sequence number of the chunk.
* @param isMasterTimestampSource True if the chunk can initialize the timestamp adjuster.
* @param timestampAdjuster Adjuster corresponding to the provided discontinuity sequence number.
* @param extractor The extractor to decode samples from the data. * @param extractor The extractor to decode samples from the data.
* @param extractorNeedsInit Whether the extractor needs initializing with the target * @param extractorNeedsInit Whether the extractor needs initializing with the target
* {@link HlsSampleStreamWrapper}. * {@link HlsSampleStreamWrapper}.
...@@ -78,12 +83,14 @@ import java.util.concurrent.atomic.AtomicInteger; ...@@ -78,12 +83,14 @@ import java.util.concurrent.atomic.AtomicInteger;
*/ */
public HlsMediaChunk(DataSource dataSource, DataSpec dataSpec, Format trackFormat, public HlsMediaChunk(DataSource dataSource, DataSpec dataSpec, Format trackFormat,
int trackSelectionReason, Object trackSelectionData, long startTimeUs, long endTimeUs, int trackSelectionReason, Object trackSelectionData, long startTimeUs, long endTimeUs,
int chunkIndex, int discontinuitySequenceNumber, Extractor extractor, int chunkIndex, int discontinuitySequenceNumber, boolean isMasterTimestampSource,
boolean extractorNeedsInit, boolean shouldSpliceIn, byte[] encryptionKey, TimestampAdjuster timestampAdjuster, Extractor extractor, boolean extractorNeedsInit,
byte[] encryptionIv) { boolean shouldSpliceIn, byte[] encryptionKey, byte[] encryptionIv) {
super(buildDataSource(dataSource, encryptionKey, encryptionIv), dataSpec, trackFormat, super(buildDataSource(dataSource, encryptionKey, encryptionIv), dataSpec, trackFormat,
trackSelectionReason, trackSelectionData, startTimeUs, endTimeUs, chunkIndex); trackSelectionReason, trackSelectionData, startTimeUs, endTimeUs, chunkIndex);
this.discontinuitySequenceNumber = discontinuitySequenceNumber; this.discontinuitySequenceNumber = discontinuitySequenceNumber;
this.isMasterTimestampSource = isMasterTimestampSource;
this.timestampAdjuster = timestampAdjuster;
this.extractor = extractor; this.extractor = extractor;
this.extractorNeedsInit = extractorNeedsInit; this.extractorNeedsInit = extractorNeedsInit;
this.shouldSpliceIn = shouldSpliceIn; this.shouldSpliceIn = shouldSpliceIn;
...@@ -166,6 +173,9 @@ import java.util.concurrent.atomic.AtomicInteger; ...@@ -166,6 +173,9 @@ import java.util.concurrent.atomic.AtomicInteger;
} }
try { try {
int result = Extractor.RESULT_CONTINUE; int result = Extractor.RESULT_CONTINUE;
if (!isMasterTimestampSource && timestampAdjuster != null) {
timestampAdjuster.waitUntilInitialized();
}
while (result == Extractor.RESULT_CONTINUE && !loadCanceled) { while (result == Extractor.RESULT_CONTINUE && !loadCanceled) {
result = extractor.read(input, null); result = extractor.read(input, null);
} }
......
...@@ -34,22 +34,18 @@ public final class TimestampAdjusterProvider { ...@@ -34,22 +34,18 @@ public final class TimestampAdjusterProvider {
/** /**
* Returns a {@link TimestampAdjuster} suitable for adjusting the pts timestamps contained in * Returns a {@link TimestampAdjuster} suitable for adjusting the pts timestamps contained in
* a chunk with a given discontinuity sequence. * a chunk with a given discontinuity sequence.
* <p>
* This method may return null if the master source has yet to initialize a suitable adjuster.
* *
* @param isMasterSource True if the calling chunk source is the master.
* @param discontinuitySequence The chunk's discontinuity sequence. * @param discontinuitySequence The chunk's discontinuity sequence.
* @param startTimeUs The chunk's start time. * @param startTimeUs The chunk's start time.
* @return A {@link TimestampAdjuster}. * @return A {@link TimestampAdjuster}.
*/ */
public TimestampAdjuster getAdjuster(boolean isMasterSource, int discontinuitySequence, public TimestampAdjuster getAdjuster(int discontinuitySequence, long startTimeUs) {
long startTimeUs) {
TimestampAdjuster adjuster = timestampAdjusters.get(discontinuitySequence); TimestampAdjuster adjuster = timestampAdjusters.get(discontinuitySequence);
if (isMasterSource && adjuster == null) { if (adjuster == null) {
adjuster = new TimestampAdjuster(startTimeUs); adjuster = new TimestampAdjuster(startTimeUs);
timestampAdjusters.put(discontinuitySequence, adjuster); timestampAdjusters.put(discontinuitySequence, adjuster);
} }
return isMasterSource || (adjuster != null && adjuster.isInitialized()) ? adjuster : null; return adjuster;
} }
/** /**
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment