From 81e2c9f0d3ea0fd934fbb4f01ae20bb81c7adf66 Mon Sep 17 00:00:00 2001 From: Oliver Woodman Date: Thu, 20 Nov 2014 14:59:22 +0000 Subject: [PATCH] Fix https://github.com/google/ExoPlayer/issues/159. The actual fix here is to not call discardExtractors in HlsSampleSource whilst the loading thread that's pushing data into it is still running. It's required to wait for that thread to have exited before doing this. Issue: #159 --- .../exoplayer/hls/HlsSampleSource.java | 29 ++++++------- .../android/exoplayer/hls/TsExtractor.java | 43 +++++++++++-------- 2 files changed, 39 insertions(+), 33 deletions(-) diff --git a/library/src/main/java/com/google/android/exoplayer/hls/HlsSampleSource.java b/library/src/main/java/com/google/android/exoplayer/hls/HlsSampleSource.java index cad1cdc03d..693af382f1 100644 --- a/library/src/main/java/com/google/android/exoplayer/hls/HlsSampleSource.java +++ b/library/src/main/java/com/google/android/exoplayer/hls/HlsSampleSource.java @@ -138,9 +138,7 @@ public class HlsSampleSource implements SampleSource, Loader.Callback { if (loader.isLoading()) { loader.cancelLoading(); } else { - discardExtractors(); - clearCurrentLoadable(); - previousTsLoadable = null; + clearState(); } } } @@ -186,7 +184,7 @@ public class HlsSampleSource implements SampleSource, Loader.Callback { TsExtractor extractor = extractors.getFirst(); while (extractors.size() > 1 && !extractor.hasSamples()) { // We're finished reading from the extractor for all tracks, and so can discard it. - extractors.removeFirst().clear(); + extractors.removeFirst().release(); extractor = extractors.getFirst(); } @@ -294,11 +292,10 @@ public class HlsSampleSource implements SampleSource, Loader.Callback { @Override public void onLoadCanceled(Loadable loadable) { - clearCurrentLoadable(); if (enabledTrackCount > 0) { restartFrom(pendingResetPositionUs); } else { - previousTsLoadable = null; + clearState(); } } @@ -312,17 +309,24 @@ public class HlsSampleSource implements SampleSource, Loader.Callback { private void restartFrom(long positionUs) { pendingResetPositionUs = positionUs; - previousTsLoadable = null; loadingFinished = false; - discardExtractors(); if (loader.isLoading()) { loader.cancelLoading(); } else { - clearCurrentLoadable(); + clearState(); maybeStartLoading(); } } + private void clearState() { + for (int i = 0; i < extractors.size(); i++) { + extractors.get(i).release(); + } + extractors.clear(); + clearCurrentLoadable(); + previousTsLoadable = null; + } + private void clearCurrentLoadable() { currentLoadable = null; currentLoadableException = null; @@ -370,13 +374,6 @@ public class HlsSampleSource implements SampleSource, Loader.Callback { loader.startLoading(currentLoadable, this); } - private void discardExtractors() { - for (int i = 0; i < extractors.size(); i++) { - extractors.get(i).clear(); - } - extractors.clear(); - } - private boolean isTsChunk(HlsChunk chunk) { return chunk instanceof TsChunk; } diff --git a/library/src/main/java/com/google/android/exoplayer/hls/TsExtractor.java b/library/src/main/java/com/google/android/exoplayer/hls/TsExtractor.java index 6a00873eae..d574078162 100644 --- a/library/src/main/java/com/google/android/exoplayer/hls/TsExtractor.java +++ b/library/src/main/java/com/google/android/exoplayer/hls/TsExtractor.java @@ -58,12 +58,16 @@ public final class TsExtractor { private final SamplePool samplePool; /* package */ final long firstSampleTimestamp; - private boolean prepared; + // Accessed only by the consuming thread. private boolean spliceConfigured; + // Accessed only by the loading thread. /* package */ boolean pendingFirstSampleTimestampAdjustment; /* package */ long sampleTimestampOffsetUs; - /* package */ long largestParsedTimestampUs; + + // Accessed by both the loading and consuming threads. + private volatile boolean prepared; + /* package */ volatile long largestParsedTimestampUs; public TsExtractor(long firstSampleTimestamp, SamplePool samplePool) { this.firstSampleTimestamp = firstSampleTimestamp; @@ -111,11 +115,13 @@ public final class TsExtractor { } /** - * Flushes any pending or incomplete samples, returning them to the sample pool. + * Releases the extractor, recycling any pending or incomplete samples to the sample pool. + *

+ * This method should not be called whilst {@link #read(DataSource)} is also being invoked. */ - public void clear() { + public void release() { for (int i = 0; i < sampleQueues.size(); i++) { - sampleQueues.valueAt(i).clear(); + sampleQueues.valueAt(i).release(); } } @@ -504,16 +510,19 @@ public final class TsExtractor { private final SamplePool samplePool; private final ConcurrentLinkedQueue internalQueue; - private MediaFormat mediaFormat; - private long spliceOutTimeUs; - private long lastParsedTimestampUs; + // Accessed only by the consuming thread. private boolean readFirstFrame; + private long lastReadTimeUs; + private long spliceOutTimeUs; + + // Accessed by both the loading and consuming threads. + private volatile MediaFormat mediaFormat; protected SampleQueue(SamplePool samplePool) { this.samplePool = samplePool; internalQueue = new ConcurrentLinkedQueue(); spliceOutTimeUs = Long.MIN_VALUE; - lastParsedTimestampUs = Long.MIN_VALUE; + lastReadTimeUs = Long.MIN_VALUE; } public boolean hasMediaFormat() { @@ -541,6 +550,7 @@ public final class TsExtractor { if (head != null) { internalQueue.remove(); readFirstFrame = true; + lastReadTimeUs = head.timeUs; } return head; } @@ -575,7 +585,7 @@ public final class TsExtractor { /** * Clears the queue. */ - public void clear() { + public void release() { Sample toRecycle = internalQueue.poll(); while (toRecycle != null) { recycle(toRecycle); @@ -608,7 +618,7 @@ public final class TsExtractor { if (nextSample != null) { firstPossibleSpliceTime = nextSample.timeUs; } else { - firstPossibleSpliceTime = lastParsedTimestampUs + 1; + firstPossibleSpliceTime = lastReadTimeUs + 1; } ConcurrentLinkedQueue nextInternalQueue = nextQueue.internalQueue; Sample nextQueueSample = nextInternalQueue.peek(); @@ -656,7 +666,6 @@ public final class TsExtractor { protected void addSample(Sample sample) { adjustTimestamp(sample); - lastParsedTimestampUs = sample.timeUs; largestParsedTimestampUs = Math.max(largestParsedTimestampUs, sample.timeUs); internalQueue.add(sample); } @@ -712,8 +721,8 @@ public final class TsExtractor { } @Override - public void clear() { - super.clear(); + public void release() { + super.release(); if (currentSample != null) { recycle(currentSample); currentSample = null; @@ -861,7 +870,7 @@ public final class TsExtractor { cropUnitX = subWidthC; cropUnitY = subHeightC * (2 - (frameMbsOnlyFlag ? 1 : 0)); } - frameWidth -= (frameCropLeftOffset + frameCropRightOffset) * cropUnitX; + frameWidth -= (frameCropLeftOffset + frameCropRightOffset) * cropUnitX; frameHeight -= (frameCropTopOffset + frameCropBottomOffset) * cropUnitY; } @@ -1057,8 +1066,8 @@ public final class TsExtractor { } @Override - public void clear() { - super.clear(); + public void release() { + super.release(); adtsBuffer.reset(); }