From 418edda2850ea51c1b804b94d99124110bf8124d Mon Sep 17 00:00:00 2001 From: tonihei Date: Fri, 11 Aug 2023 15:31:55 +0000 Subject: [PATCH] Fix capacity overflow logic in findSampleAfter The method was recently introduced and only searched for matching samples up to index==length. However, SampleQueue uses a circular array to index its data and the search should continue until relativeStartIndex+length, while also handling the overflow in the circular array. The tests for seeking didn't cover these overflow cases yet because they always started writing data in an empty SampleQueue. This can be fixed by prewarming the queue with placeholder data to force using the overflow logic in the various seek methods. PiperOrigin-RevId: 555963011 --- .../media3/exoplayer/source/SampleQueue.java | 11 +++- .../exoplayer/source/SampleQueueTest.java | 55 ++++++++++++++----- 2 files changed, 49 insertions(+), 17 deletions(-) diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/SampleQueue.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/SampleQueue.java index 5ae69704b0..b00545ee6b 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/SampleQueue.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/SampleQueue.java @@ -1009,9 +1009,14 @@ public class SampleQueue implements TrackOutput { */ private int findSampleAfter( int relativeStartIndex, int length, long timeUs, boolean allowTimeBeyondBuffer) { - for (int i = relativeStartIndex; i < length; i++) { - if (timesUs[i] >= timeUs) { - return i - relativeStartIndex; + int searchIndex = relativeStartIndex; + for (int i = 0; i < length; i++) { + if (timesUs[searchIndex] >= timeUs) { + return i; + } + searchIndex++; + if (searchIndex == capacity) { + searchIndex = 0; } } return allowTimeBeyondBuffer ? length : -1; diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/SampleQueueTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/SampleQueueTest.java index 9be2f6009c..b990923b73 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/SampleQueueTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/SampleQueueTest.java @@ -37,6 +37,7 @@ import androidx.media3.common.Format; import androidx.media3.common.MimeTypes; import androidx.media3.common.PlaybackException; import androidx.media3.common.util.ParsableByteArray; +import androidx.media3.common.util.Util; import androidx.media3.decoder.DecoderInputBuffer; import androidx.media3.exoplayer.FormatHolder; import androidx.media3.exoplayer.analytics.PlayerId; @@ -159,6 +160,8 @@ public final class SampleQueueTest { private static final TrackOutput.CryptoData CRYPTO_DATA = new TrackOutput.CryptoData(C.CRYPTO_MODE_AES_CTR, new byte[16], 0, 0); + private static final int CLOSE_TO_CAPACITY_SIZE = SampleQueue.SAMPLE_CAPACITY_INCREMENT - 1; + private Allocator allocator; private MockDrmSessionManager mockDrmSessionManager; private DrmSession mockDrmSession; @@ -737,37 +740,40 @@ public final class SampleQueueTest { @Test public void seekToBeforeBuffer_notAllSamplesAreSyncSamples() { + writeAndDiscardPlaceholderSamples(CLOSE_TO_CAPACITY_SIZE); writeTestData(); boolean success = sampleQueue.seekTo(SAMPLE_TIMESTAMPS[0] - 1, /* allowTimeBeyondBuffer= */ false); assertThat(success).isFalse(); - assertThat(sampleQueue.getReadIndex()).isEqualTo(0); + assertThat(sampleQueue.getReadIndex()).isEqualTo(CLOSE_TO_CAPACITY_SIZE); assertReadTestData(); assertNoSamplesToRead(FORMAT_2); } @Test public void seekToStartOfBuffer_notAllSamplesAreSyncSamples() { + writeAndDiscardPlaceholderSamples(CLOSE_TO_CAPACITY_SIZE); writeTestData(); boolean success = sampleQueue.seekTo(SAMPLE_TIMESTAMPS[0], /* allowTimeBeyondBuffer= */ false); assertThat(success).isTrue(); - assertThat(sampleQueue.getReadIndex()).isEqualTo(0); + assertThat(sampleQueue.getReadIndex()).isEqualTo(CLOSE_TO_CAPACITY_SIZE); assertReadTestData(); assertNoSamplesToRead(FORMAT_2); } @Test public void seekToEndOfBuffer_notAllSamplesAreSyncSamples() { + writeAndDiscardPlaceholderSamples(CLOSE_TO_CAPACITY_SIZE); writeTestData(); boolean success = sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP, /* allowTimeBeyondBuffer= */ false); assertThat(success).isTrue(); - assertThat(sampleQueue.getReadIndex()).isEqualTo(4); + assertThat(sampleQueue.getReadIndex()).isEqualTo(CLOSE_TO_CAPACITY_SIZE + 4); assertReadTestData( /* startFormat= */ null, DATA_SECOND_KEYFRAME_INDEX, @@ -779,26 +785,28 @@ public final class SampleQueueTest { @Test public void seekToAfterBuffer_notAllSamplesAreSyncSamples() { + writeAndDiscardPlaceholderSamples(CLOSE_TO_CAPACITY_SIZE); writeTestData(); boolean success = sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP + 1, /* allowTimeBeyondBuffer= */ false); assertThat(success).isFalse(); - assertThat(sampleQueue.getReadIndex()).isEqualTo(0); + assertThat(sampleQueue.getReadIndex()).isEqualTo(CLOSE_TO_CAPACITY_SIZE); assertReadTestData(); assertNoSamplesToRead(FORMAT_2); } @Test public void seekToAfterBufferAllowed_notAllSamplesAreSyncSamples() { + writeAndDiscardPlaceholderSamples(CLOSE_TO_CAPACITY_SIZE); writeTestData(); boolean success = sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP + 1, /* allowTimeBeyondBuffer= */ true); assertThat(success).isTrue(); - assertThat(sampleQueue.getReadIndex()).isEqualTo(4); + assertThat(sampleQueue.getReadIndex()).isEqualTo(CLOSE_TO_CAPACITY_SIZE + 4); assertReadTestData( /* startFormat= */ null, DATA_SECOND_KEYFRAME_INDEX, @@ -810,12 +818,13 @@ public final class SampleQueueTest { @Test public void seekToEndAndBackToStart_notAllSamplesAreSyncSamples() { + writeAndDiscardPlaceholderSamples(CLOSE_TO_CAPACITY_SIZE); writeTestData(); boolean success = sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP, /* allowTimeBeyondBuffer= */ false); assertThat(success).isTrue(); - assertThat(sampleQueue.getReadIndex()).isEqualTo(4); + assertThat(sampleQueue.getReadIndex()).isEqualTo(CLOSE_TO_CAPACITY_SIZE + 4); assertReadTestData( /* startFormat= */ null, DATA_SECOND_KEYFRAME_INDEX, @@ -828,44 +837,48 @@ public final class SampleQueueTest { success = sampleQueue.seekTo(SAMPLE_TIMESTAMPS[0], /* allowTimeBeyondBuffer= */ false); assertThat(success).isTrue(); - assertThat(sampleQueue.getReadIndex()).isEqualTo(0); + assertThat(sampleQueue.getReadIndex()).isEqualTo(CLOSE_TO_CAPACITY_SIZE); assertReadTestData(); assertNoSamplesToRead(FORMAT_2); } @Test public void seekToBeforeBuffer_allSamplesAreSyncSamples() { + writeAndDiscardPlaceholderSamples(CLOSE_TO_CAPACITY_SIZE); writeSyncSamplesOnlyTestData(); boolean success = sampleQueue.seekTo(SAMPLE_TIMESTAMPS[0] - 1, /* allowTimeBeyondBuffer= */ false); assertThat(success).isFalse(); - assertThat(sampleQueue.getReadIndex()).isEqualTo(0); + assertThat(sampleQueue.getReadIndex()).isEqualTo(CLOSE_TO_CAPACITY_SIZE); assertReadSyncSampleOnlyTestData(); assertNoSamplesToRead(FORMAT_SYNC_SAMPLE_ONLY_2); } @Test public void seekToStartOfBuffer_allSamplesAreSyncSamples() { + writeAndDiscardPlaceholderSamples(CLOSE_TO_CAPACITY_SIZE); writeSyncSamplesOnlyTestData(); boolean success = sampleQueue.seekTo(SAMPLE_TIMESTAMPS[0], /* allowTimeBeyondBuffer= */ false); assertThat(success).isTrue(); - assertThat(sampleQueue.getReadIndex()).isEqualTo(0); + assertThat(sampleQueue.getReadIndex()).isEqualTo(CLOSE_TO_CAPACITY_SIZE); assertReadSyncSampleOnlyTestData(); assertNoSamplesToRead(FORMAT_SYNC_SAMPLE_ONLY_2); } @Test public void seekToEndOfBuffer_allSamplesAreSyncSamples() { + writeAndDiscardPlaceholderSamples(CLOSE_TO_CAPACITY_SIZE); writeSyncSamplesOnlyTestData(); boolean success = sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP, /* allowTimeBeyondBuffer= */ false); assertThat(success).isTrue(); - assertThat(sampleQueue.getReadIndex()).isEqualTo(SAMPLE_TIMESTAMPS.length - 1); + assertThat(sampleQueue.getReadIndex()) + .isEqualTo(CLOSE_TO_CAPACITY_SIZE + SAMPLE_TIMESTAMPS.length - 1); assertReadSyncSampleOnlyTestData( /* firstSampleIndex= */ SAMPLE_TIMESTAMPS.length - 1, /* sampleCount= */ 1); assertNoSamplesToRead(FORMAT_SYNC_SAMPLE_ONLY_2); @@ -873,38 +886,43 @@ public final class SampleQueueTest { @Test public void seekToAfterBuffer_allSamplesAreSyncSamples() { + writeAndDiscardPlaceholderSamples(CLOSE_TO_CAPACITY_SIZE); writeSyncSamplesOnlyTestData(); boolean success = sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP + 1, /* allowTimeBeyondBuffer= */ false); assertThat(success).isFalse(); - assertThat(sampleQueue.getReadIndex()).isEqualTo(0); + assertThat(sampleQueue.getReadIndex()).isEqualTo(CLOSE_TO_CAPACITY_SIZE); assertReadSyncSampleOnlyTestData(); assertNoSamplesToRead(FORMAT_SYNC_SAMPLE_ONLY_2); } @Test public void seekToAfterBufferAllowed_allSamplesAreSyncSamples() { + writeAndDiscardPlaceholderSamples(CLOSE_TO_CAPACITY_SIZE); writeSyncSamplesOnlyTestData(); boolean success = sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP + 1, /* allowTimeBeyondBuffer= */ true); assertThat(success).isTrue(); - assertThat(sampleQueue.getReadIndex()).isEqualTo(SAMPLE_TIMESTAMPS.length); + assertThat(sampleQueue.getReadIndex()) + .isEqualTo(CLOSE_TO_CAPACITY_SIZE + SAMPLE_TIMESTAMPS.length); assertReadFormat(/* formatRequired= */ false, FORMAT_SYNC_SAMPLE_ONLY_2); assertNoSamplesToRead(FORMAT_SYNC_SAMPLE_ONLY_2); } @Test public void seekToEndAndBackToStart_allSamplesAreSyncSamples() { + writeAndDiscardPlaceholderSamples(CLOSE_TO_CAPACITY_SIZE); writeSyncSamplesOnlyTestData(); boolean success = sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP, /* allowTimeBeyondBuffer= */ false); assertThat(success).isTrue(); - assertThat(sampleQueue.getReadIndex()).isEqualTo(SAMPLE_TIMESTAMPS.length - 1); + assertThat(sampleQueue.getReadIndex()) + .isEqualTo(CLOSE_TO_CAPACITY_SIZE + SAMPLE_TIMESTAMPS.length - 1); assertReadSyncSampleOnlyTestData( /* firstSampleIndex= */ SAMPLE_TIMESTAMPS.length - 1, /* sampleCount= */ 1); assertNoSamplesToRead(FORMAT_SYNC_SAMPLE_ONLY_2); @@ -913,7 +931,7 @@ public final class SampleQueueTest { success = sampleQueue.seekTo(SAMPLE_TIMESTAMPS[0], /* allowTimeBeyondBuffer= */ false); assertThat(success).isTrue(); - assertThat(sampleQueue.getReadIndex()).isEqualTo(0); + assertThat(sampleQueue.getReadIndex()).isEqualTo(CLOSE_TO_CAPACITY_SIZE); assertReadSyncSampleOnlyTestData(); assertNoSamplesToRead(FORMAT_SYNC_SAMPLE_ONLY_2); } @@ -1574,6 +1592,15 @@ public final class SampleQueueTest { (sampleFlags & C.BUFFER_FLAG_ENCRYPTED) != 0 ? CRYPTO_DATA : null); } + private void writeAndDiscardPlaceholderSamples(int sampleCount) { + writeFormat(FORMAT_SYNC_SAMPLE_ONLY_1); + for (int i = 0; i < sampleCount; i++) { + writeSample( + Util.EMPTY_BYTE_ARRAY, /* timestampUs= */ 0, /* sampleFlags= */ C.BUFFER_FLAG_KEY_FRAME); + } + sampleQueue.discardToEnd(); + } + /** Asserts correct reading of the sync-sample-only test data from {@code sampleQueue}. */ private void assertReadSyncSampleOnlyTestData() { assertReadSyncSampleOnlyTestData(