From b8df8ecb47d838f4e3c184cbcbb64a397b1384a2 Mon Sep 17 00:00:00 2001 From: Oliver Woodman Date: Fri, 19 Jun 2015 19:55:12 +0100 Subject: [PATCH] Final fixes for correctly handling chunk load failures in HLS. An accumulation of several fixes: 1. Change to HlsExtractorWrapper is just a move + documentating things that were already true + adding a precondition in the configureSpliceTo method. 2. Change in HlsSampleSource.readData ensures that configureSpliceTo and hasSamples aren't called on an extractor that isn't prepared. 3. The other change in HlsSampleSource ensures the correct "previous" TsChunk is used. If a TsChunk fails to load and is replaced, the previous chunk should be the one before that whose load completed successfully. 4. Determine switchingVariantSpliced based on the actual format of the previous chunk, so it's set correctly in the case of a TsChunk load failure and subsequent replacement. --- .../android/exoplayer/hls/HlsChunkSource.java | 3 +- .../exoplayer/hls/HlsExtractorWrapper.java | 119 ++++++++++-------- .../exoplayer/hls/HlsSampleSource.java | 27 ++-- 3 files changed, 86 insertions(+), 63 deletions(-) diff --git a/library/src/main/java/com/google/android/exoplayer/hls/HlsChunkSource.java b/library/src/main/java/com/google/android/exoplayer/hls/HlsChunkSource.java index 42fe6d3e37..00b02025e9 100644 --- a/library/src/main/java/com/google/android/exoplayer/hls/HlsChunkSource.java +++ b/library/src/main/java/com/google/android/exoplayer/hls/HlsChunkSource.java @@ -264,7 +264,8 @@ public class HlsChunkSource { switchingVariantSpliced = false; } else { nextVariantIndex = getNextVariantIndex(previousTsChunk, playbackPositionUs); - switchingVariantSpliced = nextVariantIndex != selectedVariantIndex + switchingVariantSpliced = previousTsChunk != null + && !variants[nextVariantIndex].format.equals(previousTsChunk.format) && adaptiveMode == ADAPTIVE_MODE_SPLICE; } diff --git a/library/src/main/java/com/google/android/exoplayer/hls/HlsExtractorWrapper.java b/library/src/main/java/com/google/android/exoplayer/hls/HlsExtractorWrapper.java index b802d9fb9b..b1d1a8e10e 100644 --- a/library/src/main/java/com/google/android/exoplayer/hls/HlsExtractorWrapper.java +++ b/library/src/main/java/com/google/android/exoplayer/hls/HlsExtractorWrapper.java @@ -73,60 +73,6 @@ public final class HlsExtractorWrapper implements ExtractorOutput { extractor.init(this); } - /** - * Attempts to configure a splice from this extractor to the next. - *

- * The splice is performed such that for each track the samples read from the next extractor - * start with a keyframe, and continue from where the samples read from this extractor finish. - * A successful splice may discard samples from either or both extractors. - *

- * Splice configuration may fail if the next extractor is not yet in a state that allows the - * splice to be performed. Calling this method is a noop if the splice has already been - * configured. Hence this method should be called repeatedly during the window within which a - * splice can be performed. - * - * @param nextExtractor The extractor being spliced to. - */ - public final void configureSpliceTo(HlsExtractorWrapper nextExtractor) { - if (spliceConfigured || !nextExtractor.shouldSpliceIn || !nextExtractor.isPrepared()) { - // The splice is already configured, or the next extractor doesn't want to be spliced in, or - // the next extractor isn't ready to be spliced in. - return; - } - boolean spliceConfigured = true; - int trackCount = getTrackCount(); - for (int i = 0; i < trackCount; i++) { - DefaultTrackOutput currentSampleQueue = sampleQueues.valueAt(i); - DefaultTrackOutput nextSampleQueue = nextExtractor.sampleQueues.valueAt(i); - spliceConfigured &= currentSampleQueue.configureSpliceTo(nextSampleQueue); - } - this.spliceConfigured = spliceConfigured; - return; - } - - /** - * Gets the number of available tracks. - *

- * This method should only be called after the extractor has been prepared. - * - * @return The number of available tracks. - */ - public int getTrackCount() { - return sampleQueues.size(); - } - - /** - * Gets the {@link MediaFormat} of the specified track. - *

- * This method must only be called after the extractor has been prepared. - * - * @param track The track index. - * @return The corresponding format. - */ - public MediaFormat getMediaFormat(int track) { - return sampleQueues.valueAt(track).getFormat(); - } - /** * Whether the extractor is prepared. * @@ -167,8 +113,69 @@ public final class HlsExtractorWrapper implements ExtractorOutput { return largestParsedTimestampUs; } + /** + * Attempts to configure a splice from this extractor to the next. + *

+ * The splice is performed such that for each track the samples read from the next extractor + * start with a keyframe, and continue from where the samples read from this extractor finish. + * A successful splice may discard samples from either or both extractors. + *

+ * Splice configuration may fail if the next extractor is not yet in a state that allows the + * splice to be performed. Calling this method is a noop if the splice has already been + * configured. Hence this method should be called repeatedly during the window within which a + * splice can be performed. + *

+ * This method must only be called after the extractor has been prepared. + * + * @param nextExtractor The extractor being spliced to. + */ + public final void configureSpliceTo(HlsExtractorWrapper nextExtractor) { + Assertions.checkState(isPrepared()); + if (spliceConfigured || !nextExtractor.shouldSpliceIn || !nextExtractor.isPrepared()) { + // The splice is already configured, or the next extractor doesn't want to be spliced in, or + // the next extractor isn't ready to be spliced in. + return; + } + boolean spliceConfigured = true; + int trackCount = getTrackCount(); + for (int i = 0; i < trackCount; i++) { + DefaultTrackOutput currentSampleQueue = sampleQueues.valueAt(i); + DefaultTrackOutput nextSampleQueue = nextExtractor.sampleQueues.valueAt(i); + spliceConfigured &= currentSampleQueue.configureSpliceTo(nextSampleQueue); + } + this.spliceConfigured = spliceConfigured; + return; + } + + /** + * Gets the number of available tracks. + *

+ * This method must only be called after the extractor has been prepared. + * + * @return The number of available tracks. + */ + public int getTrackCount() { + Assertions.checkState(isPrepared()); + return sampleQueues.size(); + } + + /** + * Gets the {@link MediaFormat} of the specified track. + *

+ * This method must only be called after the extractor has been prepared. + * + * @param track The track index. + * @return The corresponding format. + */ + public MediaFormat getMediaFormat(int track) { + Assertions.checkState(isPrepared()); + return sampleQueues.valueAt(track).getFormat(); + } + /** * Gets the next sample for the specified track. + *

+ * This method must only be called after the extractor has been prepared. * * @param track The track from which to read. * @param holder A {@link SampleHolder} into which the sample should be read. @@ -181,6 +188,8 @@ public final class HlsExtractorWrapper implements ExtractorOutput { /** * Discards samples for the specified track up to the specified time. + *

+ * This method must only be called after the extractor has been prepared. * * @param track The track from which samples should be discarded. * @param timeUs The time up to which samples should be discarded, in microseconds. @@ -193,6 +202,8 @@ public final class HlsExtractorWrapper implements ExtractorOutput { /** * Whether samples are available for reading from {@link #getSample(int, SampleHolder)} for the * specified track. + *

+ * This method must only be called after the extractor has been prepared. * * @return True if samples are available for reading from {@link #getSample(int, SampleHolder)} * for the specified track. False otherwise. 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 85b52757f2..e96b9e3d07 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 @@ -80,9 +80,10 @@ public class HlsSampleSource implements SampleSource, SampleSourceReader, Loader private long lastSeekPositionUs; private long pendingResetPositionUs; - private TsChunk previousTsLoadable; - private Chunk currentLoadable; private boolean loadingFinished; + private Chunk currentLoadable; + private TsChunk currentTsLoadable; + private TsChunk previousTsLoadable; private Loader loader; private IOException currentLoadableException; @@ -259,6 +260,10 @@ public class HlsSampleSource implements SampleSource, SampleSourceReader, Loader } HlsExtractorWrapper extractor = getCurrentExtractor(); + if (!extractor.isPrepared()) { + maybeThrowLoadableException(); + return NOTHING_READ; + } if (downstreamFormat == null || !downstreamFormat.equals(extractor.format)) { // Notify a change in the downstream format. @@ -277,11 +282,10 @@ public class HlsSampleSource implements SampleSource, SampleSourceReader, Loader // We're finished reading from the extractor for this particular track, so advance to the // next one for the current read. extractor = extractors.get(++extractorIndex); - } - - if (!extractor.isPrepared()) { - maybeThrowLoadableException(); - return NOTHING_READ; + if (!extractor.isPrepared()) { + maybeThrowLoadableException(); + return NOTHING_READ; + } } MediaFormat mediaFormat = extractor.getMediaFormat(track); @@ -351,6 +355,10 @@ public class HlsSampleSource implements SampleSource, SampleSourceReader, Loader public void onLoadCompleted(Loadable loadable) { long now = SystemClock.elapsedRealtime(); long loadDurationMs = now - currentLoadStartTimeMs; + if (currentTsLoadable != null) { + previousTsLoadable = currentTsLoadable; + currentTsLoadable = null; + } chunkSource.onChunkLoadCompleted(currentLoadable); if (isTsChunk(currentLoadable)) { TsChunk tsChunk = (TsChunk) loadable; @@ -462,6 +470,7 @@ public class HlsSampleSource implements SampleSource, SampleSourceReader, Loader } private void clearCurrentLoadable() { + currentTsLoadable = null; currentLoadable = null; currentLoadableException = null; currentLoadableExceptionCount = 0; @@ -510,7 +519,7 @@ public class HlsSampleSource implements SampleSource, SampleSourceReader, Loader } notifyLoadStarted(tsChunk.dataSpec.length, tsChunk.type, tsChunk.trigger, tsChunk.format, tsChunk.startTimeUs, tsChunk.endTimeUs); - previousTsLoadable = tsChunk; + currentTsLoadable = tsChunk; } else { notifyLoadStarted(currentLoadable.dataSpec.length, currentLoadable.type, currentLoadable.trigger, currentLoadable.format, -1, -1); @@ -525,6 +534,8 @@ public class HlsSampleSource implements SampleSource, SampleSourceReader, Loader private long getNextLoadPositionUs() { if (isPendingReset()) { return pendingResetPositionUs; + } else if (loader.isLoading()) { + return currentTsLoadable.isLastChunk ? -1 : currentTsLoadable.endTimeUs; } else { return previousTsLoadable.isLastChunk ? -1 : previousTsLoadable.endTimeUs; }