From faf0e2c1471179f0dfaa3a547719c5121920a962 Mon Sep 17 00:00:00 2001 From: Oliver Woodman Date: Fri, 17 Apr 2015 20:10:31 +0100 Subject: [PATCH] Improve retry logic. 1. Reset retry count to 0 if a Loadable makes progress. 2. Handle resume correctly in the case of live streams. Issue: #227 Issue: #389 --- .../extractor/ExtractorSampleSource.java | 101 ++++++++++++++---- .../extractor/mp3/ConstantBitrateSeeker.java | 12 ++- .../exoplayer/extractor/mp3/Mp3Extractor.java | 15 +-- .../exoplayer/extractor/mp3/VbriSeeker.java | 12 ++- .../exoplayer/extractor/mp3/XingSeeker.java | 12 ++- 5 files changed, 112 insertions(+), 40 deletions(-) diff --git a/library/src/main/java/com/google/android/exoplayer/extractor/ExtractorSampleSource.java b/library/src/main/java/com/google/android/exoplayer/extractor/ExtractorSampleSource.java index 11b2da0397..e546fcf319 100644 --- a/library/src/main/java/com/google/android/exoplayer/extractor/ExtractorSampleSource.java +++ b/library/src/main/java/com/google/android/exoplayer/extractor/ExtractorSampleSource.java @@ -42,12 +42,17 @@ import java.io.IOException; public class ExtractorSampleSource implements SampleSource, ExtractorOutput, Loader.Callback { /** - * The default minimum number of times to retry loading data prior to failing. + * The default minimum number of times to retry loading prior to failing for on-demand streams. */ - public static final int DEFAULT_MIN_LOADABLE_RETRY_COUNT = 3; + public static final int DEFAULT_MIN_LOADABLE_RETRY_COUNT_ON_DEMAND = 3; - private static final int BUFFER_LENGTH = 256 * 1024; + /** + * The default minimum number of times to retry loading prior to failing for live streams. + */ + public static final int DEFAULT_MIN_LOADABLE_RETRY_COUNT_LIVE = 6; + private static final int BUFFER_FRAGMENT_LENGTH = 256 * 1024; + private static final int MIN_RETRY_COUNT_DEFAULT_FOR_MEDIA = -1; private static final int NO_RESET_PENDING = -1; private final Extractor extractor; @@ -94,14 +99,30 @@ public class ExtractorSampleSource implements SampleSource, ExtractorOutput, Loa */ public ExtractorSampleSource(Uri uri, DataSource dataSource, Extractor extractor, int downstreamRendererCount, int requestedBufferSize) { + this(uri, dataSource, extractor, downstreamRendererCount, requestedBufferSize, + MIN_RETRY_COUNT_DEFAULT_FOR_MEDIA); + } + + /** + * @param uri The {@link Uri} of the media stream. + * @param dataSource A data source to read the media stream. + * @param extractor An {@link Extractor} to extract the media stream. + * @param downstreamRendererCount Number of track renderers dependent on this sample source. + * @param requestedBufferSize The requested total buffer size for storing sample data, in bytes. + * The actual allocated size may exceed the value passed in if the implementation requires it. + * @param minLoadableRetryCount The minimum number of times that the sample source will retry + * if a loading error occurs. + */ + public ExtractorSampleSource(Uri uri, DataSource dataSource, Extractor extractor, + int downstreamRendererCount, int requestedBufferSize, int minLoadableRetryCount) { this.uri = uri; this.dataSource = dataSource; this.extractor = extractor; - remainingReleaseCount = downstreamRendererCount; + this.remainingReleaseCount = downstreamRendererCount; this.requestedBufferSize = requestedBufferSize; + this.minLoadableRetryCount = minLoadableRetryCount; sampleQueues = new SparseArray(); - bufferPool = new BufferPool(BUFFER_LENGTH); - minLoadableRetryCount = DEFAULT_MIN_LOADABLE_RETRY_COUNT; + bufferPool = new BufferPool(BUFFER_FRAGMENT_LENGTH); pendingResetPositionUs = NO_RESET_PENDING; frameAccurateSeeking = true; extractor.init(this); @@ -132,9 +153,6 @@ public class ExtractorSampleSource implements SampleSource, ExtractorOutput, Loa trackInfos[i] = new TrackInfo(format.mimeType, format.durationUs); } prepared = true; - if (isPendingReset()) { - restartFrom(pendingResetPositionUs); - } return true; } else { maybeThrowLoadableException(); @@ -232,6 +250,11 @@ public class ExtractorSampleSource implements SampleSource, ExtractorOutput, Loa public void seekToUs(long positionUs) { Assertions.checkState(prepared); Assertions.checkState(enabledTrackCount > 0); + if (!seekMap.isSeekable()) { + // Treat all seeks into non-seekable media as seeks to the start. + positionUs = 0; + } + lastSeekPositionUs = positionUs; if ((isPendingReset() ? pendingResetPositionUs : downstreamPositionUs) == positionUs) { return; @@ -300,9 +323,9 @@ public class ExtractorSampleSource implements SampleSource, ExtractorOutput, Loa } @Override - public void onLoadError(Loadable loadable, IOException e) { + public void onLoadError(Loadable ignored, IOException e) { currentLoadableException = e; - currentLoadableExceptionCount++; + currentLoadableExceptionCount = loadable.madeProgress() ? 1 : currentLoadableExceptionCount + 1; currentLoadableExceptionTimestamp = SystemClock.elapsedRealtime(); maybeStartLoading(); } @@ -369,28 +392,62 @@ public class ExtractorSampleSource implements SampleSource, ExtractorOutput, Loa long elapsedMillis = SystemClock.elapsedRealtime() - currentLoadableExceptionTimestamp; if (elapsedMillis >= getRetryDelayMillis(currentLoadableExceptionCount)) { currentLoadableException = null; + if (!prepared || !seekMap.isSeekable()) { + // One of two cases applies: + // 1. We're not prepared. We don't know whether we're playing an on-demand or a live + // stream. Play it safe and start from scratch. + // 2. We're playing a non-seekable stream. Assume it's a live stream. In such cases it's + // best to discard the pending buffer and start from scratch. + for (int i = 0; i < sampleQueues.size(); i++) { + sampleQueues.valueAt(i).clear(); + } + loadable = createPreparationLoadable(); + } else { + // We're playing a seekable on-demand stream. Resume the current loadable, which will + // request data starting from the point it left off. + } loader.startLoading(loadable, this); } return; } if (!prepared) { - loadable = new ExtractingLoadable(uri, dataSource, extractor, bufferPool, requestedBufferSize, - 0); + loadable = createPreparationLoadable(); } else { Assertions.checkState(isPendingReset()); - loadable = new ExtractingLoadable(uri, dataSource, extractor, bufferPool, requestedBufferSize, - seekMap.getPosition(pendingResetPositionUs)); + loadable = createLoadableForPosition(pendingResetPositionUs); pendingResetPositionUs = NO_RESET_PENDING; } loader.startLoading(loadable, this); } private void maybeThrowLoadableException() throws IOException { - if (currentLoadableException != null && (currentLoadableExceptionFatal - || currentLoadableExceptionCount > minLoadableRetryCount)) { + if (currentLoadableException == null) { + return; + } + if (currentLoadableExceptionFatal) { throw currentLoadableException; } + int minLoadableRetryCountForMedia; + if (minLoadableRetryCount != MIN_RETRY_COUNT_DEFAULT_FOR_MEDIA) { + minLoadableRetryCountForMedia = minLoadableRetryCount; + } else { + minLoadableRetryCountForMedia = seekMap != null && !seekMap.isSeekable() + ? DEFAULT_MIN_LOADABLE_RETRY_COUNT_LIVE + : DEFAULT_MIN_LOADABLE_RETRY_COUNT_ON_DEMAND; + } + if (currentLoadableExceptionCount > minLoadableRetryCountForMedia) { + throw currentLoadableException; + } + } + + private ExtractingLoadable createPreparationLoadable() { + return new ExtractingLoadable(uri, dataSource, extractor, bufferPool, requestedBufferSize, 0); + } + + private ExtractingLoadable createLoadableForPosition(long positionUs) { + return new ExtractingLoadable(uri, dataSource, extractor, bufferPool, requestedBufferSize, + seekMap.getPosition(positionUs)); } private boolean haveFormatsForAllTracks() { @@ -452,6 +509,7 @@ public class ExtractorSampleSource implements SampleSource, ExtractorOutput, Loa private volatile boolean loadCanceled; private boolean pendingExtractorSeek; + private boolean madeProgress; public ExtractingLoadable(Uri uri, DataSource dataSource, Extractor extractor, BufferPool bufferPool, int bufferPoolSizeLimit, long position) { @@ -465,6 +523,10 @@ public class ExtractorSampleSource implements SampleSource, ExtractorOutput, Loa pendingExtractorSeek = true; } + public boolean madeProgress() { + return madeProgress; + } + @Override public void cancelLoad() { loadCanceled = true; @@ -498,9 +560,12 @@ public class ExtractorSampleSource implements SampleSource, ExtractorOutput, Loa } } finally { if (result == Extractor.RESULT_SEEK) { + madeProgress |= true; result = Extractor.RESULT_CONTINUE; } else if (input != null) { - positionHolder.position = input.getPosition(); + long newPosition = input.getPosition(); + madeProgress |= newPosition > positionHolder.position; + positionHolder.position = newPosition; } dataSource.close(); } diff --git a/library/src/main/java/com/google/android/exoplayer/extractor/mp3/ConstantBitrateSeeker.java b/library/src/main/java/com/google/android/exoplayer/extractor/mp3/ConstantBitrateSeeker.java index a196c9bba9..90fdfce85e 100644 --- a/library/src/main/java/com/google/android/exoplayer/extractor/mp3/ConstantBitrateSeeker.java +++ b/library/src/main/java/com/google/android/exoplayer/extractor/mp3/ConstantBitrateSeeker.java @@ -20,7 +20,7 @@ import com.google.android.exoplayer.C; /** * MP3 seeker that doesn't rely on metadata and seeks assuming the source has a constant bitrate. */ -/* package */ final class ConstantBitrateSeeker extends Mp3Extractor.Seeker { +/* package */ final class ConstantBitrateSeeker implements Mp3Extractor.Seeker { private static final int MICROSECONDS_PER_SECOND = 1000000; private static final int BITS_PER_BYTE = 8; @@ -32,14 +32,18 @@ import com.google.android.exoplayer.C; public ConstantBitrateSeeker(long firstFramePosition, int bitrate, long inputLength) { this.firstFramePosition = firstFramePosition; this.bitrate = bitrate; + durationUs = inputLength == C.LENGTH_UNBOUNDED ? C.UNKNOWN_TIME_US : getTimeUs(inputLength); + } - durationUs = - inputLength == C.LENGTH_UNBOUNDED ? C.UNKNOWN_TIME_US : getTimeUs(inputLength); + @Override + public boolean isSeekable() { + return durationUs != C.UNKNOWN_TIME_US; } @Override public long getPosition(long timeUs) { - return firstFramePosition + (timeUs * bitrate) / (MICROSECONDS_PER_SECOND * BITS_PER_BYTE); + return durationUs == C.UNKNOWN_TIME_US ? 0 + : firstFramePosition + (timeUs * bitrate) / (MICROSECONDS_PER_SECOND * BITS_PER_BYTE); } @Override diff --git a/library/src/main/java/com/google/android/exoplayer/extractor/mp3/Mp3Extractor.java b/library/src/main/java/com/google/android/exoplayer/extractor/mp3/Mp3Extractor.java index 186f9d520f..ddcdf19f3b 100644 --- a/library/src/main/java/com/google/android/exoplayer/extractor/mp3/Mp3Extractor.java +++ b/library/src/main/java/com/google/android/exoplayer/extractor/mp3/Mp3Extractor.java @@ -248,8 +248,8 @@ public final class Mp3Extractor implements Extractor { } if (seeker == null) { inputBuffer.returnToMark(); - seeker = new ConstantBitrateSeeker( - headerPosition, synchronizedHeader.bitrate * 1000, extractorInput.getLength()); + seeker = new ConstantBitrateSeeker(headerPosition, synchronizedHeader.bitrate * 1000, + extractorInput.getLength()); } else { // Discard the frame that was parsed for seeking metadata. inputBuffer.mark(); @@ -273,12 +273,7 @@ public final class Mp3Extractor implements Extractor { * {@link SeekMap} that also allows mapping from position (byte offset) back to time, which can be * used to work out the new sample basis timestamp after seeking and resynchronization. */ - /* package */ abstract static class Seeker implements SeekMap { - - @Override - public final boolean isSeekable() { - return true; - } + /* package */ interface Seeker extends SeekMap { /** * Maps a position (byte offset) to a corresponding sample timestamp. @@ -286,10 +281,10 @@ public final class Mp3Extractor implements Extractor { * @param position A seek position (byte offset) relative to the start of the stream. * @return The corresponding timestamp of the next sample to be read, in microseconds. */ - abstract long getTimeUs(long position); + long getTimeUs(long position); /** Returns the duration of the source, in microseconds. */ - abstract long getDurationUs(); + long getDurationUs(); } diff --git a/library/src/main/java/com/google/android/exoplayer/extractor/mp3/VbriSeeker.java b/library/src/main/java/com/google/android/exoplayer/extractor/mp3/VbriSeeker.java index ee516c3a1e..4413edaaa7 100644 --- a/library/src/main/java/com/google/android/exoplayer/extractor/mp3/VbriSeeker.java +++ b/library/src/main/java/com/google/android/exoplayer/extractor/mp3/VbriSeeker.java @@ -21,15 +21,14 @@ import com.google.android.exoplayer.util.Util; /** * MP3 seeker that uses metadata from a VBRI header. */ -/* package */ final class VbriSeeker extends Mp3Extractor.Seeker { +/* package */ final class VbriSeeker implements Mp3Extractor.Seeker { private static final int VBRI_HEADER = Util.getIntegerCodeForString("VBRI"); /** * If {@code frame} contains a VBRI header and it is usable for seeking, returns a - * {@link Mp3Extractor.Seeker} for seeking in the containing stream. Otherwise, returns - * {@code null}, which indicates that the information in the frame was not a VBRI header, or was - * unusable for seeking. + * {@link VbriSeeker} for seeking in the containing stream. Otherwise, returns {@code null}, which + * indicates that the information in the frame was not a VBRI header, or was unusable for seeking. */ public static VbriSeeker create( MpegAudioHeader mpegAudioHeader, ParsableByteArray frame, long position) { @@ -99,6 +98,11 @@ import com.google.android.exoplayer.util.Util; this.durationUs = durationUs; } + @Override + public boolean isSeekable() { + return true; + } + @Override public long getPosition(long timeUs) { int index = Util.binarySearchFloor(timesUs, timeUs, false, false); diff --git a/library/src/main/java/com/google/android/exoplayer/extractor/mp3/XingSeeker.java b/library/src/main/java/com/google/android/exoplayer/extractor/mp3/XingSeeker.java index 73c1fb0c4c..25fd30f89e 100644 --- a/library/src/main/java/com/google/android/exoplayer/extractor/mp3/XingSeeker.java +++ b/library/src/main/java/com/google/android/exoplayer/extractor/mp3/XingSeeker.java @@ -22,16 +22,15 @@ import com.google.android.exoplayer.util.Util; /** * MP3 seeker that uses metadata from a XING header. */ -/* package */ final class XingSeeker extends Mp3Extractor.Seeker { +/* package */ final class XingSeeker implements Mp3Extractor.Seeker { private static final int XING_HEADER = Util.getIntegerCodeForString("Xing"); private static final int INFO_HEADER = Util.getIntegerCodeForString("Info"); /** * If {@code frame} contains a XING header and it is usable for seeking, returns a - * {@link Mp3Extractor.Seeker} for seeking in the containing stream. Otherwise, returns - * {@code null}, which indicates that the information in the frame was not a XING header, or was - * unusable for seeking. + * {@link XingSeeker} for seeking in the containing stream. Otherwise, returns {@code null}, which + * indicates that the information in the frame was not a XING header, or was unusable for seeking. */ public static XingSeeker create(MpegAudioHeader mpegAudioHeader, ParsableByteArray frame, long position, long inputLength) { @@ -108,6 +107,11 @@ import com.google.android.exoplayer.util.Util; this.inputLength = inputLength; } + @Override + public boolean isSeekable() { + return true; + } + @Override public long getPosition(long timeUs) { float percent = timeUs * 100f / durationUs;