Split SampleQueue.advanceTo for advance & seek use cases

This method has two use cases:

1. Seeking. Calls are immediately preceded by a call to rewind(), and
   the returned value isn't important unless it's ADVANCED_FAILED (i.e.
   the caller is only interested in success and failure).
2. Advancing. The return value is important unless it's ADVANCED_FAILED,
   in which case the caller wants to treat it as 0.

This change creates separate methods for each use case. The new seekTo
methods automatically rewind and return a boolean. The updated advanceTo
method returns 0 directly in cases where ADVANCED_FAILED was returned.
Arguments that were always hard-coded to true by callers have also been
removed.

This change is a step toward one possible solution for #6155. How we'll
solve that issue is still up for discussion, but this change seems like
one we should make regardless!

Issue: #6155
PiperOrigin-RevId: 290053743
This commit is contained in:
olly 2020-01-16 14:17:28 +00:00 committed by Oliver Woodman
parent 51f2723b90
commit 775a17c498
5 changed files with 146 additions and 115 deletions

View File

@ -281,13 +281,13 @@ import org.checkerframework.checker.nullness.compatqual.NullableType;
// If there's still a chance of avoiding a seek, try and seek within the sample queue.
if (!seekRequired) {
SampleQueue sampleQueue = sampleQueues[track];
sampleQueue.rewind();
// A seek can be avoided if we're able to advance to the current playback position in the
// A seek can be avoided if we're able to seek to the current playback position in the
// sample queue, or if we haven't read anything from the queue since the previous seek
// (this case is common for sparse tracks such as metadata tracks). In all other cases a
// seek is required.
seekRequired = sampleQueue.advanceTo(positionUs, true, true) == SampleQueue.ADVANCE_FAILED
&& sampleQueue.getReadIndex() != 0;
seekRequired =
!sampleQueue.seekTo(positionUs, /* allowTimeBeyondBuffer= */ true)
&& sampleQueue.getReadIndex() != 0;
}
}
}
@ -493,10 +493,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType;
if (loadingFinished && positionUs > sampleQueue.getLargestQueuedTimestampUs()) {
skipCount = sampleQueue.advanceToEnd();
} else {
skipCount = sampleQueue.advanceTo(positionUs, true, true);
if (skipCount == SampleQueue.ADVANCE_FAILED) {
skipCount = 0;
}
skipCount = sampleQueue.advanceTo(positionUs);
}
if (skipCount == 0) {
maybeStartDeferredRetry(track);
@ -840,9 +837,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType;
int trackCount = sampleQueues.length;
for (int i = 0; i < trackCount; i++) {
SampleQueue sampleQueue = sampleQueues[i];
sampleQueue.rewind();
boolean seekInsideQueue = sampleQueue.advanceTo(positionUs, true, false)
!= SampleQueue.ADVANCE_FAILED;
boolean seekInsideQueue = sampleQueue.seekTo(positionUs, /* allowTimeBeyondBuffer= */ false);
// If we have AV tracks then an in-buffer seek is successful if the seek into every AV queue
// is successful. We ignore whether seeks within non-AV queues are successful in this case, as
// they may be sparse or poorly interleaved. If we only have non-AV tracks then a seek is

View File

@ -48,8 +48,6 @@ public class SampleQueue implements TrackOutput {
void onUpstreamFormatChanged(Format format);
}
public static final int ADVANCE_FAILED = -1;
@VisibleForTesting /* package */ static final int SAMPLE_CAPACITY_INCREMENT = 1000;
private final SampleDataQueue sampleDataQueue;
@ -271,6 +269,7 @@ public class SampleQueue implements TrackOutput {
* the end of stream has been reached. When false, this method returns false if the sample
* queue is empty.
*/
@SuppressWarnings("ReferenceEquality") // See comments in setUpstreamFormat
public synchronized boolean isReady(boolean loadingFinished) {
if (!hasNextSample()) {
return loadingFinished
@ -312,7 +311,6 @@ public class SampleQueue implements TrackOutput {
* @return The result, which can be {@link C#RESULT_NOTHING_READ}, {@link C#RESULT_FORMAT_READ} or
* {@link C#RESULT_BUFFER_READ}.
*/
@SuppressWarnings("ReferenceEquality")
public int read(
FormatHolder formatHolder,
DecoderInputBuffer buffer,
@ -328,36 +326,61 @@ public class SampleQueue implements TrackOutput {
return result;
}
/** Rewinds the read position to the first sample in the queue. */
public void rewind() {
rewindMetadata();
sampleDataQueue.rewind();
/**
* Attempts to seek the read position to the specified sample index.
*
* @param sampleIndex The sample index.
* @return Whether the seek was successful.
*/
public synchronized boolean seekTo(int sampleIndex) {
rewind();
if (sampleIndex < absoluteFirstIndex || sampleIndex > absoluteFirstIndex + length) {
return false;
}
readPosition = sampleIndex - absoluteFirstIndex;
return true;
}
/**
* Attempts to advance the read position to the sample before or at the specified time.
* Attempts to seek the read position to the keyframe before or at the specified time.
*
* @param timeUs The time to advance to.
* @param toKeyframe If true then attempts to advance to the keyframe before or at the specified
* time, rather than to any sample before or at that time.
* @param timeUs The time to seek to.
* @param allowTimeBeyondBuffer Whether the operation can succeed if {@code timeUs} is beyond the
* end of the queue, by advancing the read position to the last sample (or keyframe).
* @return The number of samples that were skipped if the operation was successful, which may be
* equal to 0, or {@link #ADVANCE_FAILED} if the operation was not successful. A successful
* advance is one in which the read position was unchanged or advanced, and is now at a sample
* meeting the specified criteria.
* end of the queue, by seeking to the last sample (or keyframe).
* @return Whether the seek was successful.
*/
public synchronized int advanceTo(
long timeUs, boolean toKeyframe, boolean allowTimeBeyondBuffer) {
public synchronized boolean seekTo(long timeUs, boolean allowTimeBeyondBuffer) {
rewind();
int relativeReadIndex = getRelativeIndex(readPosition);
if (!hasNextSample()
|| timeUs < timesUs[relativeReadIndex]
|| (timeUs > largestQueuedTimestampUs && !allowTimeBeyondBuffer)) {
return SampleQueue.ADVANCE_FAILED;
return false;
}
int offset = findSampleBefore(relativeReadIndex, length - readPosition, timeUs, toKeyframe);
int offset =
findSampleBefore(relativeReadIndex, length - readPosition, timeUs, /* keyframe= */ true);
if (offset == -1) {
return SampleQueue.ADVANCE_FAILED;
return false;
}
readPosition += offset;
return true;
}
/**
* Advances the read position to the keyframe before or at the specified time.
*
* @param timeUs The time to advance to.
* @return The number of samples that were skipped, which may be equal to 0.
*/
public synchronized int advanceTo(long timeUs) {
int relativeReadIndex = getRelativeIndex(readPosition);
if (!hasNextSample() || timeUs < timesUs[relativeReadIndex]) {
return 0;
}
int offset =
findSampleBefore(relativeReadIndex, length - readPosition, timeUs, /* keyframe= */ true);
if (offset == -1) {
return 0;
}
readPosition += offset;
return offset;
@ -374,22 +397,6 @@ public class SampleQueue implements TrackOutput {
return skipCount;
}
/**
* Attempts to set the read position to the specified sample index.
*
* @param sampleIndex The sample index.
* @return Whether the read position was set successfully. False is returned if the specified
* index is smaller than the index of the first sample in the queue, or larger than the index
* of the next sample that will be written.
*/
public synchronized boolean setReadPosition(int sampleIndex) {
if (absoluteFirstIndex <= sampleIndex && sampleIndex <= absoluteFirstIndex + length) {
readPosition = sampleIndex - absoluteFirstIndex;
return true;
}
return false;
}
/**
* Discards up to but not including the sample immediately before or at the specified time.
*
@ -486,10 +493,13 @@ public class SampleQueue implements TrackOutput {
// Internal methods.
private synchronized void rewindMetadata() {
/** Rewinds the read position to the first sample in the queue. */
private synchronized void rewind() {
readPosition = 0;
sampleDataQueue.rewind();
}
@SuppressWarnings("ReferenceEquality") // See comments in setUpstreamFormat
private synchronized int readSampleMetadata(
FormatHolder formatHolder,
DecoderInputBuffer buffer,
@ -543,13 +553,13 @@ public class SampleQueue implements TrackOutput {
upstreamFormatRequired = false;
if (Util.areEqual(format, upstreamFormat)) {
// The format is unchanged. If format and upstreamFormat are different objects, we keep the
// current upstreamFormat so we can detect format changes in read() using cheap referential
// equality.
// current upstreamFormat so we can detect format changes on the read side using cheap
// referential quality.
return false;
} else if (Util.areEqual(format, upstreamCommittedFormat)) {
// The format has changed back to the format of the last committed sample. If they are
// different objects, we revert back to using upstreamCommittedFormat as the upstreamFormat
// so we can detect format changes in read() using cheap referential equality.
// so we can detect format changes on the read side using cheap referential equality.
upstreamFormat = upstreamCommittedFormat;
return true;
} else {

View File

@ -186,8 +186,7 @@ public class ChunkSampleStream<T extends ChunkSource> implements SampleStream, S
if (embeddedTrackTypes[i] == trackType) {
Assertions.checkState(!embeddedTracksSelected[i]);
embeddedTracksSelected[i] = true;
embeddedSampleQueues[i].rewind();
embeddedSampleQueues[i].advanceTo(positionUs, true, true);
embeddedSampleQueues[i].seekTo(positionUs, /* allowTimeBeyondBuffer= */ true);
return new EmbeddedSampleStream(this, embeddedSampleQueues[i], i);
}
}
@ -267,21 +266,16 @@ public class ChunkSampleStream<T extends ChunkSource> implements SampleStream, S
// See if we can seek inside the primary sample queue.
boolean seekInsideBuffer;
primarySampleQueue.rewind();
if (seekToMediaChunk != null) {
// When seeking to the start of a chunk we use the index of the first sample in the chunk
// rather than the seek position. This ensures we seek to the keyframe at the start of the
// chunk even if the sample timestamps are slightly offset from the chunk start times.
seekInsideBuffer =
primarySampleQueue.setReadPosition(seekToMediaChunk.getFirstSampleIndex(0));
seekInsideBuffer = primarySampleQueue.seekTo(seekToMediaChunk.getFirstSampleIndex(0));
decodeOnlyUntilPositionUs = 0;
} else {
seekInsideBuffer =
primarySampleQueue.advanceTo(
positionUs,
/* toKeyframe= */ true,
/* allowTimeBeyondBuffer= */ positionUs < getNextLoadPositionUs())
!= SampleQueue.ADVANCE_FAILED;
primarySampleQueue.seekTo(
positionUs, /* allowTimeBeyondBuffer= */ positionUs < getNextLoadPositionUs());
decodeOnlyUntilPositionUs = lastSeekPositionUs;
}
@ -290,10 +284,9 @@ public class ChunkSampleStream<T extends ChunkSource> implements SampleStream, S
nextNotifyPrimaryFormatMediaChunkIndex =
primarySampleIndexToMediaChunkIndex(
primarySampleQueue.getReadIndex(), /* minChunkIndex= */ 0);
// Advance the embedded sample queues to the seek position.
// Seek the embedded sample queues.
for (SampleQueue embeddedSampleQueue : embeddedSampleQueues) {
embeddedSampleQueue.rewind();
embeddedSampleQueue.advanceTo(positionUs, true, false);
embeddedSampleQueue.seekTo(positionUs, /* allowTimeBeyondBuffer= */ true);
}
} else {
// We can't seek inside the buffer, and so need to reset.
@ -390,10 +383,7 @@ public class ChunkSampleStream<T extends ChunkSource> implements SampleStream, S
if (loadingFinished && positionUs > primarySampleQueue.getLargestQueuedTimestampUs()) {
skipCount = primarySampleQueue.advanceToEnd();
} else {
skipCount = primarySampleQueue.advanceTo(positionUs, true, true);
if (skipCount == SampleQueue.ADVANCE_FAILED) {
skipCount = 0;
}
skipCount = primarySampleQueue.advanceTo(positionUs);
}
maybeNotifyPrimaryTrackFormatChanged();
return skipCount;
@ -756,10 +746,7 @@ public class ChunkSampleStream<T extends ChunkSource> implements SampleStream, S
if (loadingFinished && positionUs > sampleQueue.getLargestQueuedTimestampUs()) {
skipCount = sampleQueue.advanceToEnd();
} else {
skipCount = sampleQueue.advanceTo(positionUs, true, true);
if (skipCount == SampleQueue.ADVANCE_FAILED) {
skipCount = 0;
}
skipCount = sampleQueue.advanceTo(positionUs);
}
return skipCount;
}

View File

@ -18,7 +18,6 @@ package com.google.android.exoplayer2.source;
import static com.google.android.exoplayer2.C.RESULT_BUFFER_READ;
import static com.google.android.exoplayer2.C.RESULT_FORMAT_READ;
import static com.google.android.exoplayer2.C.RESULT_NOTHING_READ;
import static com.google.android.exoplayer2.source.SampleQueue.ADVANCE_FAILED;
import static com.google.common.truth.Truth.assertThat;
import static java.lang.Long.MIN_VALUE;
import static java.util.Arrays.copyOfRange;
@ -320,14 +319,14 @@ public final class SampleQueueTest {
}
@Test
public void testReadMultiWithRewind() {
public void testReadMultiWithSeek() {
writeTestData();
assertReadTestData();
assertThat(sampleQueue.getFirstIndex()).isEqualTo(0);
assertThat(sampleQueue.getReadIndex()).isEqualTo(8);
assertAllocationCount(10);
// Rewind.
sampleQueue.rewind();
sampleQueue.seekTo(0);
assertAllocationCount(10);
// Read again.
assertThat(sampleQueue.getFirstIndex()).isEqualTo(0);
@ -496,15 +495,15 @@ public final class SampleQueueTest {
}
@Test
public void testRewindAfterDiscard() {
public void testSeekAfterDiscard() {
writeTestData();
assertReadTestData();
sampleQueue.discardToRead();
assertThat(sampleQueue.getFirstIndex()).isEqualTo(8);
assertThat(sampleQueue.getReadIndex()).isEqualTo(8);
assertAllocationCount(0);
// Rewind.
sampleQueue.rewind();
sampleQueue.seekTo(0);
assertAllocationCount(0);
// Can't read again.
assertThat(sampleQueue.getFirstIndex()).isEqualTo(8);
@ -553,9 +552,9 @@ public final class SampleQueueTest {
@Test
public void testAdvanceToBeforeBuffer() {
writeTestData();
int skipCount = sampleQueue.advanceTo(SAMPLE_TIMESTAMPS[0] - 1, true, false);
// Should fail and have no effect.
assertThat(skipCount).isEqualTo(ADVANCE_FAILED);
int skipCount = sampleQueue.advanceTo(SAMPLE_TIMESTAMPS[0] - 1);
// Should have no effect (we're already at the first frame).
assertThat(skipCount).isEqualTo(0);
assertReadTestData();
assertNoSamplesToRead(FORMAT_2);
}
@ -563,8 +562,8 @@ public final class SampleQueueTest {
@Test
public void testAdvanceToStartOfBuffer() {
writeTestData();
int skipCount = sampleQueue.advanceTo(SAMPLE_TIMESTAMPS[0], true, false);
// Should succeed but have no effect (we're already at the first frame).
int skipCount = sampleQueue.advanceTo(SAMPLE_TIMESTAMPS[0]);
// Should have no effect (we're already at the first frame).
assertThat(skipCount).isEqualTo(0);
assertReadTestData();
assertNoSamplesToRead(FORMAT_2);
@ -573,8 +572,8 @@ public final class SampleQueueTest {
@Test
public void testAdvanceToEndOfBuffer() {
writeTestData();
int skipCount = sampleQueue.advanceTo(LAST_SAMPLE_TIMESTAMP, true, false);
// Should succeed and skip to 2nd keyframe (the 4th frame).
int skipCount = sampleQueue.advanceTo(LAST_SAMPLE_TIMESTAMP);
// Should advance to 2nd keyframe (the 4th frame).
assertThat(skipCount).isEqualTo(4);
assertReadTestData(null, DATA_SECOND_KEYFRAME_INDEX);
assertNoSamplesToRead(FORMAT_2);
@ -583,23 +582,79 @@ public final class SampleQueueTest {
@Test
public void testAdvanceToAfterBuffer() {
writeTestData();
int skipCount = sampleQueue.advanceTo(LAST_SAMPLE_TIMESTAMP + 1, true, false);
// Should fail and have no effect.
assertThat(skipCount).isEqualTo(ADVANCE_FAILED);
int skipCount = sampleQueue.advanceTo(LAST_SAMPLE_TIMESTAMP + 1);
// Should advance to 2nd keyframe (the 4th frame).
assertThat(skipCount).isEqualTo(4);
assertReadTestData(null, DATA_SECOND_KEYFRAME_INDEX);
assertNoSamplesToRead(FORMAT_2);
}
@Test
public void testSeekToBeforeBuffer() {
writeTestData();
boolean success = sampleQueue.seekTo(SAMPLE_TIMESTAMPS[0] - 1, false);
assertThat(success).isFalse();
assertThat(sampleQueue.getReadIndex()).isEqualTo(0);
assertReadTestData();
assertNoSamplesToRead(FORMAT_2);
}
@Test
public void testAdvanceToAfterBufferAllowed() {
public void testSeekToStartOfBuffer() {
writeTestData();
int skipCount = sampleQueue.advanceTo(LAST_SAMPLE_TIMESTAMP + 1, true, true);
// Should succeed and skip to 2nd keyframe (the 4th frame).
assertThat(skipCount).isEqualTo(4);
boolean success = sampleQueue.seekTo(SAMPLE_TIMESTAMPS[0], false);
assertThat(success).isTrue();
assertThat(sampleQueue.getReadIndex()).isEqualTo(0);
assertReadTestData();
assertNoSamplesToRead(FORMAT_2);
}
@Test
public void testSeekToEndOfBuffer() {
writeTestData();
boolean success = sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP, false);
assertThat(success).isTrue();
assertThat(sampleQueue.getReadIndex()).isEqualTo(4);
assertReadTestData(null, DATA_SECOND_KEYFRAME_INDEX);
assertNoSamplesToRead(FORMAT_2);
}
@Test
public void testSeekToAfterBuffer() {
writeTestData();
boolean success = sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP + 1, false);
assertThat(success).isFalse();
assertThat(sampleQueue.getReadIndex()).isEqualTo(0);
assertReadTestData();
assertNoSamplesToRead(FORMAT_2);
}
@Test
public void testSeekToAfterBufferAllowed() {
writeTestData();
boolean success = sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP + 1, true);
assertThat(success).isTrue();
assertThat(sampleQueue.getReadIndex()).isEqualTo(4);
assertReadTestData(null, DATA_SECOND_KEYFRAME_INDEX);
assertNoSamplesToRead(FORMAT_2);
}
@Test
public void testSeekToEndAndBackToStart() {
writeTestData();
boolean success = sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP, false);
assertThat(success).isTrue();
assertThat(sampleQueue.getReadIndex()).isEqualTo(4);
assertReadTestData(null, DATA_SECOND_KEYFRAME_INDEX);
assertNoSamplesToRead(FORMAT_2);
// Seek back to the start.
success = sampleQueue.seekTo(SAMPLE_TIMESTAMPS[0], false);
assertThat(success).isTrue();
assertThat(sampleQueue.getReadIndex()).isEqualTo(0);
assertReadTestData();
assertNoSamplesToRead(FORMAT_2);
}
@Test
public void testDiscardToEnd() {
writeTestData();
@ -814,7 +869,7 @@ public final class SampleQueueTest {
assertReadTestData(SAMPLE_FORMATS[3], 4, 4);
assertReadEndOfStream(false);
sampleQueue.rewind();
sampleQueue.seekTo(0);
assertReadTestData(null, 0, 4);
sampleQueue.splice();
// Splice should succeed, replacing the last 4 samples with the sample being written
@ -1114,18 +1169,6 @@ public final class SampleQueueTest {
assertThat(readData).isEqualTo(copyOfRange(sampleData, offset, offset + length));
}
/** Asserts {@link SampleQueue#read} returns the given result. */
private void assertResult(int expectedResult, boolean allowOnlyClearBuffers) {
int obtainedResult =
sampleQueue.read(
formatHolder,
inputBuffer,
allowOnlyClearBuffers,
/* loadingFinished= */ false,
/* decodeOnlyUntilUs= */ 0);
assertThat(obtainedResult).isEqualTo(expectedResult);
}
/**
* Asserts the number of allocations currently in use by {@code sampleQueue}.
*

View File

@ -361,13 +361,12 @@ import org.checkerframework.checker.nullness.qual.RequiresNonNull;
// If there's still a chance of avoiding a seek, try and seek within the sample queue.
if (!seekRequired) {
SampleQueue sampleQueue = sampleQueues[trackGroupToSampleQueueIndex[trackGroupIndex]];
sampleQueue.rewind();
// A seek can be avoided if we're able to advance to the current playback position in
// A seek can be avoided if we're able to seek to the current playback position in
// the sample queue, or if we haven't read anything from the queue since the previous
// seek (this case is common for sparse tracks such as metadata tracks). In all other
// cases a seek is required.
seekRequired =
sampleQueue.advanceTo(positionUs, true, true) == SampleQueue.ADVANCE_FAILED
!sampleQueue.seekTo(positionUs, /* allowTimeBeyondBuffer= */ true)
&& sampleQueue.getReadIndex() != 0;
}
}
@ -584,8 +583,7 @@ import org.checkerframework.checker.nullness.qual.RequiresNonNull;
if (loadingFinished && positionUs > sampleQueue.getLargestQueuedTimestampUs()) {
return sampleQueue.advanceToEnd();
} else {
int skipCount = sampleQueue.advanceTo(positionUs, true, true);
return skipCount == SampleQueue.ADVANCE_FAILED ? 0 : skipCount;
return sampleQueue.advanceTo(positionUs);
}
}
@ -1170,9 +1168,7 @@ import org.checkerframework.checker.nullness.qual.RequiresNonNull;
int sampleQueueCount = sampleQueues.length;
for (int i = 0; i < sampleQueueCount; i++) {
SampleQueue sampleQueue = sampleQueues[i];
sampleQueue.rewind();
boolean seekInsideQueue = sampleQueue.advanceTo(positionUs, true, false)
!= SampleQueue.ADVANCE_FAILED;
boolean seekInsideQueue = sampleQueue.seekTo(positionUs, /* allowTimeBeyondBuffer= */ false);
// If we have AV tracks then an in-queue seek is successful if the seek into every AV queue
// is successful. We ignore whether seeks within non-AV queues are successful in this case, as
// they may be sparse or poorly interleaved. If we only have non-AV tracks then a seek is