From e96e6180463da64d5c7473bb062bd99a39004dad Mon Sep 17 00:00:00 2001 From: Oliver Woodman Date: Mon, 28 Sep 2015 20:23:45 +0100 Subject: [PATCH] Clean up expansion of sample buffers. - The old approach was technically incorrect, because the checks were "capacity < sampleSize" and hence neglected the fact that the buffer position may be greater than 0 (e.g. if the caller wants to prefix the sample with some additional data). - Also proactively throw an exception if the buffer is too small, rather than wait for the failure when we actually do the write. --- .../ext/opus/OpusDecoderWrapper.java | 2 +- .../exoplayer/ext/vp9/VpxDecoderWrapper.java | 2 +- .../exoplayer/MediaCodecTrackRenderer.java | 8 +-- .../android/exoplayer/SampleHolder.java | 58 ++++++++++++++----- .../android/exoplayer/SingleSampleSource.java | 4 +- .../extractor/RollingSampleBuffer.java | 8 +-- .../metadata/MetadataTrackRenderer.java | 2 +- 7 files changed, 54 insertions(+), 30 deletions(-) diff --git a/extensions/opus/src/main/java/com/google/android/exoplayer/ext/opus/OpusDecoderWrapper.java b/extensions/opus/src/main/java/com/google/android/exoplayer/ext/opus/OpusDecoderWrapper.java index 3c18fb3712..3687bde27b 100644 --- a/extensions/opus/src/main/java/com/google/android/exoplayer/ext/opus/OpusDecoderWrapper.java +++ b/extensions/opus/src/main/java/com/google/android/exoplayer/ext/opus/OpusDecoderWrapper.java @@ -324,7 +324,7 @@ import java.util.LinkedList; } public void reset() { - sampleHolder.data.clear(); + sampleHolder.clearData(); flags = 0; } diff --git a/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/VpxDecoderWrapper.java b/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/VpxDecoderWrapper.java index b108ca7eb2..49e063b52a 100644 --- a/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/VpxDecoderWrapper.java +++ b/extensions/vp9/src/main/java/com/google/android/exoplayer/ext/vp9/VpxDecoderWrapper.java @@ -72,7 +72,7 @@ import java.util.LinkedList; } InputBuffer inputBuffer = availableInputBuffers[--availableInputBufferCount]; inputBuffer.flags = 0; - inputBuffer.sampleHolder.data.clear(); + inputBuffer.sampleHolder.clearData(); return inputBuffer; } } diff --git a/library/src/main/java/com/google/android/exoplayer/MediaCodecTrackRenderer.java b/library/src/main/java/com/google/android/exoplayer/MediaCodecTrackRenderer.java index 6f92d0dc56..9970758c90 100644 --- a/library/src/main/java/com/google/android/exoplayer/MediaCodecTrackRenderer.java +++ b/library/src/main/java/com/google/android/exoplayer/MediaCodecTrackRenderer.java @@ -531,7 +531,7 @@ public abstract class MediaCodecTrackRenderer extends SampleSourceTrackRenderer return false; } sampleHolder.data = inputBuffers[inputIndex]; - sampleHolder.data.clear(); + sampleHolder.clearData(); } if (codecReinitializationState == REINITIALIZATION_STATE_SIGNAL_END_OF_STREAM) { @@ -579,7 +579,7 @@ public abstract class MediaCodecTrackRenderer extends SampleSourceTrackRenderer if (codecReconfigurationState == RECONFIGURATION_STATE_QUEUE_PENDING) { // We received two formats in a row. Clear the current buffer of any reconfiguration data // associated with the first format. - sampleHolder.data.clear(); + sampleHolder.clearData(); codecReconfigurationState = RECONFIGURATION_STATE_WRITE_PENDING; } onInputFormatChanged(formatHolder); @@ -590,7 +590,7 @@ public abstract class MediaCodecTrackRenderer extends SampleSourceTrackRenderer // We received a new format immediately before the end of the stream. We need to clear // the corresponding reconfiguration data from the current buffer, but re-write it into // a subsequent buffer if there are any (e.g. if the user seeks backwards). - sampleHolder.data.clear(); + sampleHolder.clearData(); codecReconfigurationState = RECONFIGURATION_STATE_WRITE_PENDING; } inputStreamEnded = true; @@ -616,7 +616,7 @@ public abstract class MediaCodecTrackRenderer extends SampleSourceTrackRenderer // TODO: Find out if it's possible to supply samples prior to the first sync // frame for HE-AAC. if (!sampleHolder.isSyncFrame()) { - sampleHolder.data.clear(); + sampleHolder.clearData(); if (codecReconfigurationState == RECONFIGURATION_STATE_QUEUE_PENDING) { // The buffer we just cleared contained reconfiguration data. We need to re-write this // data into a subsequent buffer (if there is one). diff --git a/library/src/main/java/com/google/android/exoplayer/SampleHolder.java b/library/src/main/java/com/google/android/exoplayer/SampleHolder.java index 59873e5921..be2c3f6189 100644 --- a/library/src/main/java/com/google/android/exoplayer/SampleHolder.java +++ b/library/src/main/java/com/google/android/exoplayer/SampleHolder.java @@ -63,8 +63,8 @@ public final class SampleHolder { private final int bufferReplacementMode; /** - * @param bufferReplacementMode Determines the behavior of {@link #replaceBuffer(int)}. One of - * {@link #BUFFER_REPLACEMENT_MODE_DISABLED}, {@link #BUFFER_REPLACEMENT_MODE_NORMAL} and + * @param bufferReplacementMode Determines the behavior of {@link #ensureSpaceForWrite(int)}. One + * of {@link #BUFFER_REPLACEMENT_MODE_DISABLED}, {@link #BUFFER_REPLACEMENT_MODE_NORMAL} and * {@link #BUFFER_REPLACEMENT_MODE_DIRECT}. */ public SampleHolder(int bufferReplacementMode) { @@ -73,21 +73,39 @@ public final class SampleHolder { } /** - * Attempts to replace {@link #data} with a {@link ByteBuffer} of the specified capacity. + * Ensures that {@link #data} is large enough to accommodate a write of a given length at its + * current position. + *

+ * If the capacity of {@link #data} is sufficient this method does nothing. If the capacity is + * insufficient then an attempt is made to replace {@link #data} with a new {@link ByteBuffer} + * whose capacity is sufficient. Data up to the current position is copied to the new buffer. * - * @param capacity The capacity of the replacement buffer, in bytes. - * @return True if the buffer was replaced. False otherwise. + * @param length The length of the write that must be accommodated, in bytes. + * @throws IllegalStateException If there is insufficient capacity to accommodate the write and + * the buffer replacement mode of the holder is {@link #BUFFER_REPLACEMENT_MODE_DISABLED}. */ - public boolean replaceBuffer(int capacity) { - switch (bufferReplacementMode) { - case BUFFER_REPLACEMENT_MODE_NORMAL: - data = ByteBuffer.allocate(capacity); - return true; - case BUFFER_REPLACEMENT_MODE_DIRECT: - data = ByteBuffer.allocateDirect(capacity); - return true; + public void ensureSpaceForWrite(int length) throws IllegalStateException { + if (data == null) { + data = createReplacementBuffer(length); + return; } - return false; + // Check whether the current buffer is sufficient. + int capacity = data.capacity(); + int position = data.position(); + int requiredCapacity = position + length; + if (capacity >= requiredCapacity) { + return; + } + // Instantiate a new buffer if possible. + ByteBuffer newData = createReplacementBuffer(requiredCapacity); + // Copy data up to the current position from the old buffer to the new one. + if (position > 0) { + data.position(0); + data.limit(position); + newData.put(data); + } + // Set the new buffer. + data = newData; } /** @@ -120,4 +138,16 @@ public final class SampleHolder { } } + private ByteBuffer createReplacementBuffer(int requiredCapacity) { + if (bufferReplacementMode == BUFFER_REPLACEMENT_MODE_NORMAL) { + return ByteBuffer.allocate(requiredCapacity); + } else if (bufferReplacementMode == BUFFER_REPLACEMENT_MODE_DIRECT) { + return ByteBuffer.allocateDirect(requiredCapacity); + } else { + int currentCapacity = data == null ? 0 : data.capacity(); + throw new IllegalStateException("Buffer too small (" + currentCapacity + " < " + + requiredCapacity + ")"); + } + } + } diff --git a/library/src/main/java/com/google/android/exoplayer/SingleSampleSource.java b/library/src/main/java/com/google/android/exoplayer/SingleSampleSource.java index 0eb8a9e6b1..55fde5057b 100644 --- a/library/src/main/java/com/google/android/exoplayer/SingleSampleSource.java +++ b/library/src/main/java/com/google/android/exoplayer/SingleSampleSource.java @@ -139,9 +139,7 @@ public final class SingleSampleSource implements SampleSource, SampleSourceReade sampleHolder.timeUs = 0; sampleHolder.size = sampleSize; sampleHolder.flags = C.SAMPLE_FLAG_SYNC; - if (sampleHolder.data == null || sampleHolder.data.capacity() < sampleSize) { - sampleHolder.replaceBuffer(sampleHolder.size); - } + sampleHolder.ensureSpaceForWrite(sampleHolder.size); sampleHolder.data.put(sampleData, 0, sampleSize); state = STATE_END_OF_STREAM; return SAMPLE_READ; diff --git a/library/src/main/java/com/google/android/exoplayer/extractor/RollingSampleBuffer.java b/library/src/main/java/com/google/android/exoplayer/extractor/RollingSampleBuffer.java index c6cdb8b1c4..4a349494b7 100644 --- a/library/src/main/java/com/google/android/exoplayer/extractor/RollingSampleBuffer.java +++ b/library/src/main/java/com/google/android/exoplayer/extractor/RollingSampleBuffer.java @@ -186,12 +186,8 @@ import java.util.concurrent.LinkedBlockingDeque; readEncryptionData(sampleHolder, extrasHolder); } // Write the sample data into the holder. - if (sampleHolder.data == null || sampleHolder.data.capacity() < sampleHolder.size) { - sampleHolder.replaceBuffer(sampleHolder.size); - } - if (sampleHolder.data != null) { - readData(extrasHolder.offset, sampleHolder.data, sampleHolder.size); - } + sampleHolder.ensureSpaceForWrite(sampleHolder.size); + readData(extrasHolder.offset, sampleHolder.data, sampleHolder.size); // Advance the read head. long nextOffset = infoQueue.moveToNextSample(); dropDownstreamTo(nextOffset); diff --git a/library/src/main/java/com/google/android/exoplayer/metadata/MetadataTrackRenderer.java b/library/src/main/java/com/google/android/exoplayer/metadata/MetadataTrackRenderer.java index 19c28c262a..a59519a7e6 100644 --- a/library/src/main/java/com/google/android/exoplayer/metadata/MetadataTrackRenderer.java +++ b/library/src/main/java/com/google/android/exoplayer/metadata/MetadataTrackRenderer.java @@ -114,6 +114,7 @@ public final class MetadataTrackRenderer extends SampleSourceTrackRenderer im protected void doSomeWork(long positionUs, long elapsedRealtimeUs) throws ExoPlaybackException { continueBufferingSource(positionUs); if (!inputStreamEnded && pendingMetadata == null) { + sampleHolder.clearData(); int result = readSource(positionUs, formatHolder, sampleHolder, false); if (result == SampleSource.SAMPLE_READ) { pendingMetadataTimestamp = sampleHolder.timeUs; @@ -122,7 +123,6 @@ public final class MetadataTrackRenderer extends SampleSourceTrackRenderer im } catch (IOException e) { throw new ExoPlaybackException(e); } - sampleHolder.data.clear(); } else if (result == SampleSource.END_OF_STREAM) { inputStreamEnded = true; }