sjs/media
1
0
Fork 0
mirror of https://github.com/samsonjs/media.git synced 2026-04-02 10:45:51 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

Fix inconsistency in SampleQueue seek for sync-sample-only formats

Browse source 
For sync-sample-only formats, we have an optimization to drop all buffers
with less than the start time when writing them to the queue.

For the same formats, if we set a new start time (=seek), we only seek
to the buffer at or before the start time. This means the first sample
in the queue is different depending on whether we seek to a start time
or set a start time and then write samples. This is inconsistent and
effectively means the first sample depends on a race condition between
the Loader thread (writing samples) and the playback thread (attempting
an initial seek in the already loaded samples).

The effect of this inconsistency is that we have to decode one sample
we don't need (and could have skipped) and that some tests become flaky
if the test setup runs into the mentioned race condition.

The fix is to change the SampleQueue seek method to also seek to
a sample at or after the specified time, to align the behavior to the
case where we write the same samples to an empty queue.

The change also clarifies the Javadoc of
MimeTypes.allSamplesAreSyncSamples to note that this should really only
return true if the samples have no "duration" that matters. Otherwise,
we could reasonably return true for most subtitle formats although it
would break subtitle display because we'd remove samples that start
before the seek time.

PiperOrigin-RevId: 547189941
This commit is contained in:
tonihei 2023-07-11 15:57:35 +01:00 committed by Rohit Singh
parent 9039621588
commit 5c8b142174
3 changed files with 247 additions and 40 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
libraries/common/src/main/java/androidx/media3/common/MimeTypes.java
View file

@ -234,7 +234,8 @@ public final class MimeTypes {
/**
* Returns true if it is known that all samples in a stream of the given MIME type and codec are
* guaranteed to be sync samples (i.e., {@link C#BUFFER_FLAG_KEY_FRAME} is guaranteed to be set on
* every sample).
* every sample) and the inherent duration of each sample is negligible (i.e., we never expect to
* require a sample because playback partially falls into its duration).
*
* @param mimeType The MIME type of the stream.
* @param codec The RFC 6381 codec string of the stream, or {@code null} if unknown.

48
libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/SampleQueue.java
View file

@ -103,7 +103,7 @@ public class SampleQueue implements TrackOutput {
@Nullable private Format unadjustedUpstreamFormat;
@Nullable private Format upstreamFormat;
private long upstreamSourceId;
private boolean upstreamAllSamplesAreSyncSamples;
private boolean allSamplesAreSyncSamples;
private boolean loggedUnexpectedNonSyncSample;
private long sampleOffsetUs;
@ -181,6 +181,7 @@ public class SampleQueue implements TrackOutput {
largestQueuedTimestampUs = Long.MIN_VALUE;
upstreamFormatRequired = true;
upstreamKeyframeRequired = true;
allSamplesAreSyncSamples = true;
}
// Called by the consuming thread when there is no loading thread.
@ -222,6 +223,7 @@ public class SampleQueue implements TrackOutput {
unadjustedUpstreamFormat = null;
upstreamFormat = null;
upstreamFormatRequired = true;
allSamplesAreSyncSamples = true;
}
}
@ -463,6 +465,9 @@ public class SampleQueue implements TrackOutput {
/**
* Attempts to seek the read position to the keyframe before or at the specified time.
*
* <p>For formats where {@linkplain MimeTypes#allSamplesAreSyncSamples all samples are sync
* samples}, it seeks the read position to the first sample at or after the specified 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 seeking to the last sample (or keyframe).
@ -477,7 +482,11 @@ public class SampleQueue implements TrackOutput {
return false;
}
int offset =
findSampleBefore(relativeReadIndex, length - readPosition, timeUs, /* keyframe= */ true);
allSamplesAreSyncSamples
? findSampleAfter(
relativeReadIndex, length - readPosition, timeUs, allowTimeBeyondBuffer)
: findSampleBefore(
relativeReadIndex, length - readPosition, timeUs, /* keyframe= */ true);
if (offset == -1) {
return false;
}
@ -618,7 +627,7 @@ public class SampleQueue implements TrackOutput {
}
timeUs += sampleOffsetUs;
if (upstreamAllSamplesAreSyncSamples) {
if (allSamplesAreSyncSamples) {
if (timeUs < startTimeUs) {
// If we know that all samples are sync samples, we can discard those that come before the
// start time on the write side of the queue.
@ -749,7 +758,7 @@ public class SampleQueue implements TrackOutput {
} else {
upstreamFormat = format;
}
upstreamAllSamplesAreSyncSamples =
allSamplesAreSyncSamples &=
MimeTypes.allSamplesAreSyncSamples(upstreamFormat.sampleMimeType, upstreamFormat.codecs);
loggedUnexpectedNonSyncSample = false;
return true;
@ -951,14 +960,15 @@ public class SampleQueue implements TrackOutput {
}
/**
* Finds the sample in the specified range that's before or at the specified time. If {@code
* keyframe} is {@code true} then the sample is additionally required to be a keyframe.
* Finds the offset of the last sample in the specified range that's before or at the specified
* time. If {@code keyframe} is {@code true} then the sample is additionally required to be a
* keyframe.
*
* @param relativeStartIndex The relative index from which to start searching.
* @param length The length of the range being searched.
* @param timeUs The specified time.
* @param timeUs The specified time, in microseconds.
* @param keyframe Whether only keyframes should be considered.
* @return The offset from {@code relativeFirstIndex} to the found sample, or -1 if no matching
* @return The offset from {@code relativeStartIndex} to the found sample, or -1 if no matching
* sample was found.
*/
private int findSampleBefore(int relativeStartIndex, int length, long timeUs, boolean keyframe) {
@ -985,6 +995,28 @@ public class SampleQueue implements TrackOutput {
return sampleCountToTarget;
}
/**
* Finds the offset of the first sample in the specified range that's at or after the specified
* time.
*
* @param relativeStartIndex The relative index from which to start searching.
* @param length The length of the range being searched.
* @param timeUs The specified time, in microseconds.
* @param allowTimeBeyondBuffer Whether {@code length} is returned if the {@code timeUs} is beyond
* the last buffer in the specified range.
* @return The offset from {@code relativeStartIndex} to the found sample, -1 if no sample is at
* or after the specified time.
*/
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;
}
}
return allowTimeBeyondBuffer ? length : -1;
}
/**
* Counts the number of samples that haven't been read that have a timestamp smaller than {@code
* timeUs}.

236
libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/SampleQueueTest.java
View file

@ -74,6 +74,10 @@ public final class SampleQueueTest {
new Format.Builder().setId(/* id= */ "encrypted").setDrmInitData(new DrmInitData()).build();
private static final Format FORMAT_ENCRYPTED_WITH_EXO_MEDIA_CRYPTO_TYPE =
FORMAT_ENCRYPTED.copyWithCryptoType(FakeCryptoConfig.TYPE);
private static final Format FORMAT_SYNC_SAMPLE_ONLY_1 =
new Format.Builder().setId("sync1").setSampleMimeType(MimeTypes.AUDIO_RAW).build();
private static final Format FORMAT_SYNC_SAMPLE_ONLY_2 =
new Format.Builder().setId("sync2").setSampleMimeType(MimeTypes.AUDIO_RAW).build();
private static final byte[] DATA = TestUtil.buildTestData(ALLOCATION_SIZE * 10);
/*
@ -112,9 +116,31 @@ public final class SampleQueueTest {
private static final long LAST_SAMPLE_TIMESTAMP = SAMPLE_TIMESTAMPS[SAMPLE_TIMESTAMPS.length - 1];
private static final int[] SAMPLE_FLAGS =
new int[] {C.BUFFER_FLAG_KEY_FRAME, 0, 0, 0, C.BUFFER_FLAG_KEY_FRAME, 0, 0, 0};
private static final int[] SAMPLE_FLAGS_SYNC_SAMPLES_ONLY =
new int[] {
C.BUFFER_FLAG_KEY_FRAME,
C.BUFFER_FLAG_KEY_FRAME,
C.BUFFER_FLAG_KEY_FRAME,
C.BUFFER_FLAG_KEY_FRAME,
C.BUFFER_FLAG_KEY_FRAME,
C.BUFFER_FLAG_KEY_FRAME,
C.BUFFER_FLAG_KEY_FRAME,
C.BUFFER_FLAG_KEY_FRAME
};
private static final Format[] SAMPLE_FORMATS =
new Format[] {FORMAT_1, FORMAT_1, FORMAT_1, FORMAT_1, FORMAT_2, FORMAT_2, FORMAT_2, FORMAT_2};
private static final int DATA_SECOND_KEYFRAME_INDEX = 4;
private static final Format[] SAMPLE_FORMATS_SYNC_SAMPLES_ONLY =
new Format[] {
FORMAT_SYNC_SAMPLE_ONLY_1,
FORMAT_SYNC_SAMPLE_ONLY_1,
FORMAT_SYNC_SAMPLE_ONLY_1,
FORMAT_SYNC_SAMPLE_ONLY_1,
FORMAT_SYNC_SAMPLE_ONLY_2,
FORMAT_SYNC_SAMPLE_ONLY_2,
FORMAT_SYNC_SAMPLE_ONLY_2,
FORMAT_SYNC_SAMPLE_ONLY_2
};
private static final int[] ENCRYPTED_SAMPLES_FLAGS =
new int[] {
@ -710,9 +736,12 @@ public final class SampleQueueTest {
}
@Test
public void seekToBeforeBuffer() {
public void seekToBeforeBuffer_notAllSamplesAreSyncSamples() {
writeTestData();
boolean success = sampleQueue.seekTo(SAMPLE_TIMESTAMPS[0] - 1, false);
boolean success =
sampleQueue.seekTo(SAMPLE_TIMESTAMPS[0] - 1, /* allowTimeBeyondBuffer= */ false);
assertThat(success).isFalse();
assertThat(sampleQueue.getReadIndex()).isEqualTo(0);
assertReadTestData();
@ -720,9 +749,11 @@ public final class SampleQueueTest {
}
@Test
public void seekToStartOfBuffer() {
public void seekToStartOfBuffer_notAllSamplesAreSyncSamples() {
writeTestData();
boolean success = sampleQueue.seekTo(SAMPLE_TIMESTAMPS[0], false);
boolean success = sampleQueue.seekTo(SAMPLE_TIMESTAMPS[0], /* allowTimeBeyondBuffer= */ false);
assertThat(success).isTrue();
assertThat(sampleQueue.getReadIndex()).isEqualTo(0);
assertReadTestData();
@ -730,9 +761,11 @@ public final class SampleQueueTest {
}
@Test
public void seekToEndOfBuffer() {
public void seekToEndOfBuffer_notAllSamplesAreSyncSamples() {
writeTestData();
boolean success = sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP, false);
boolean success = sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP, /* allowTimeBeyondBuffer= */ false);
assertThat(success).isTrue();
assertThat(sampleQueue.getReadIndex()).isEqualTo(4);
assertReadTestData(
@ -745,9 +778,12 @@ public final class SampleQueueTest {
}
@Test
public void seekToAfterBuffer() {
public void seekToAfterBuffer_notAllSamplesAreSyncSamples() {
writeTestData();
boolean success = sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP + 1, false);
boolean success =
sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP + 1, /* allowTimeBeyondBuffer= */ false);
assertThat(success).isFalse();
assertThat(sampleQueue.getReadIndex()).isEqualTo(0);
assertReadTestData();
@ -755,9 +791,12 @@ public final class SampleQueueTest {
}
@Test
public void seekToAfterBufferAllowed() {
public void seekToAfterBufferAllowed_notAllSamplesAreSyncSamples() {
writeTestData();
boolean success = sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP + 1, true);
boolean success =
sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP + 1, /* allowTimeBeyondBuffer= */ true);
assertThat(success).isTrue();
assertThat(sampleQueue.getReadIndex()).isEqualTo(4);
assertReadTestData(
@ -770,9 +809,11 @@ public final class SampleQueueTest {
}
@Test
public void seekToEndAndBackToStart() {
public void seekToEndAndBackToStart_notAllSamplesAreSyncSamples() {
writeTestData();
boolean success = sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP, false);
boolean success = sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP, /* allowTimeBeyondBuffer= */ false);
assertThat(success).isTrue();
assertThat(sampleQueue.getReadIndex()).isEqualTo(4);
assertReadTestData(
@ -781,10 +822,11 @@ public final class SampleQueueTest {
/* sampleCount= */ SAMPLE_TIMESTAMPS.length - DATA_SECOND_KEYFRAME_INDEX,
/* sampleOffsetUs= */ 0,
/* decodeOnlyUntilUs= */ LAST_SAMPLE_TIMESTAMP);
assertNoSamplesToRead(FORMAT_2);
// Seek back to the start.
success = sampleQueue.seekTo(SAMPLE_TIMESTAMPS[0], false);
success = sampleQueue.seekTo(SAMPLE_TIMESTAMPS[0], /* allowTimeBeyondBuffer= */ false);
assertThat(success).isTrue();
assertThat(sampleQueue.getReadIndex()).isEqualTo(0);
assertReadTestData();
@ -792,21 +834,98 @@ public final class SampleQueueTest {
}
@Test
public void setStartTimeUs_allSamplesAreSyncSamples_discardsOnWriteSide() {
// The format uses a MIME type for which MimeTypes.allSamplesAreSyncSamples() is true.
Format format = new Format.Builder().setSampleMimeType(MimeTypes.AUDIO_RAW).build();
Format[] sampleFormats = new Format[SAMPLE_SIZES.length];
Arrays.fill(sampleFormats, format);
int[] sampleFlags = new int[SAMPLE_SIZES.length];
Arrays.fill(sampleFlags, BUFFER_FLAG_KEY_FRAME);
public void seekToBeforeBuffer_allSamplesAreSyncSamples() {
writeSyncSamplesOnlyTestData();
boolean success =
sampleQueue.seekTo(SAMPLE_TIMESTAMPS[0] - 1, /* allowTimeBeyondBuffer= */ false);
assertThat(success).isFalse();
assertThat(sampleQueue.getReadIndex()).isEqualTo(0);
assertReadSyncSampleOnlyTestData();
assertNoSamplesToRead(FORMAT_SYNC_SAMPLE_ONLY_2);
}
@Test
public void seekToStartOfBuffer_allSamplesAreSyncSamples() {
writeSyncSamplesOnlyTestData();
boolean success = sampleQueue.seekTo(SAMPLE_TIMESTAMPS[0], /* allowTimeBeyondBuffer= */ false);
assertThat(success).isTrue();
assertThat(sampleQueue.getReadIndex()).isEqualTo(0);
assertReadSyncSampleOnlyTestData();
assertNoSamplesToRead(FORMAT_SYNC_SAMPLE_ONLY_2);
}
@Test
public void seekToEndOfBuffer_allSamplesAreSyncSamples() {
writeSyncSamplesOnlyTestData();
boolean success = sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP, /* allowTimeBeyondBuffer= */ false);
assertThat(success).isTrue();
assertThat(sampleQueue.getReadIndex()).isEqualTo(SAMPLE_TIMESTAMPS.length - 1);
assertReadSyncSampleOnlyTestData(
/* firstSampleIndex= */ SAMPLE_TIMESTAMPS.length - 1, /* sampleCount= */ 1);
assertNoSamplesToRead(FORMAT_SYNC_SAMPLE_ONLY_2);
}
@Test
public void seekToAfterBuffer_allSamplesAreSyncSamples() {
writeSyncSamplesOnlyTestData();
boolean success =
sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP + 1, /* allowTimeBeyondBuffer= */ false);
assertThat(success).isFalse();
assertThat(sampleQueue.getReadIndex()).isEqualTo(0);
assertReadSyncSampleOnlyTestData();
assertNoSamplesToRead(FORMAT_SYNC_SAMPLE_ONLY_2);
}
@Test
public void seekToAfterBufferAllowed_allSamplesAreSyncSamples() {
writeSyncSamplesOnlyTestData();
boolean success =
sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP + 1, /* allowTimeBeyondBuffer= */ true);
assertThat(success).isTrue();
assertThat(sampleQueue.getReadIndex()).isEqualTo(SAMPLE_TIMESTAMPS.length);
assertReadFormat(/* formatRequired= */ false, FORMAT_SYNC_SAMPLE_ONLY_2);
assertNoSamplesToRead(FORMAT_SYNC_SAMPLE_ONLY_2);
}
@Test
public void seekToEndAndBackToStart_allSamplesAreSyncSamples() {
writeSyncSamplesOnlyTestData();
boolean success = sampleQueue.seekTo(LAST_SAMPLE_TIMESTAMP, /* allowTimeBeyondBuffer= */ false);
assertThat(success).isTrue();
assertThat(sampleQueue.getReadIndex()).isEqualTo(SAMPLE_TIMESTAMPS.length - 1);
assertReadSyncSampleOnlyTestData(
/* firstSampleIndex= */ SAMPLE_TIMESTAMPS.length - 1, /* sampleCount= */ 1);
assertNoSamplesToRead(FORMAT_SYNC_SAMPLE_ONLY_2);
// Seek back to the start.
success = sampleQueue.seekTo(SAMPLE_TIMESTAMPS[0], /* allowTimeBeyondBuffer= */ false);
assertThat(success).isTrue();
assertThat(sampleQueue.getReadIndex()).isEqualTo(0);
assertReadSyncSampleOnlyTestData();
assertNoSamplesToRead(FORMAT_SYNC_SAMPLE_ONLY_2);
}
@Test
public void setStartTimeUs_allSamplesAreSyncSamples_discardsOnWriteSide() {
sampleQueue.setStartTimeUs(LAST_SAMPLE_TIMESTAMP);
writeTestData(
DATA, SAMPLE_SIZES, SAMPLE_OFFSETS, SAMPLE_TIMESTAMPS, sampleFormats, sampleFlags);
writeSyncSamplesOnlyTestData();
assertThat(sampleQueue.getReadIndex()).isEqualTo(0);
assertReadFormat(/* formatRequired= */ false, format);
assertReadFormat(/* formatRequired= */ false, FORMAT_SYNC_SAMPLE_ONLY_2);
assertReadSample(
SAMPLE_TIMESTAMPS[7],
/* isKeyFrame= */ true,
@ -819,11 +938,6 @@ public final class SampleQueueTest {
@Test
public void setStartTimeUs_notAllSamplesAreSyncSamples_discardsOnReadSide() {
// The format uses a MIME type for which MimeTypes.allSamplesAreSyncSamples() is false.
Format format = new Format.Builder().setSampleMimeType(MimeTypes.VIDEO_H264).build();
Format[] sampleFormats = new Format[SAMPLE_SIZES.length];
Arrays.fill(sampleFormats, format);
sampleQueue.setStartTimeUs(LAST_SAMPLE_TIMESTAMP);
writeTestData();
@ -1398,6 +1512,17 @@ public final class SampleQueueTest {
DATA, SAMPLE_SIZES, SAMPLE_OFFSETS, SAMPLE_TIMESTAMPS, SAMPLE_FORMATS, SAMPLE_FLAGS);
}
/** Writes test data to {@code sampleQueue} with sync-sample-only formats. */
private void writeSyncSamplesOnlyTestData() {
writeTestData(
DATA,
SAMPLE_SIZES,
SAMPLE_OFFSETS,
SAMPLE_TIMESTAMPS,
SAMPLE_FORMATS_SYNC_SAMPLES_ONLY,
SAMPLE_FLAGS_SYNC_SAMPLES_ONLY);
}
/** Writes the specified test data to {@code sampleQueue}. */
@SuppressWarnings("ReferenceEquality")
private void writeTestData(
@ -1449,6 +1574,29 @@ public final class SampleQueueTest {
(sampleFlags & C.BUFFER_FLAG_ENCRYPTED) != 0 ? CRYPTO_DATA : null);
}
/** Asserts correct reading of the sync-sample-only test data from {@code sampleQueue}. */
private void assertReadSyncSampleOnlyTestData() {
assertReadSyncSampleOnlyTestData(
/* firstSampleIndex= */ 0, /* sampleCount= */ SAMPLE_TIMESTAMPS.length);
}
/**
* Asserts correct reading of the sync-sample-only test data from {@code sampleQueue}.
*
* @param firstSampleIndex The index of the first sample that's expected to be read.
* @param sampleCount The number of samples to read.
*/
private void assertReadSyncSampleOnlyTestData(int firstSampleIndex, int sampleCount) {
assertReadTestData(
/* startFormat= */ null,
firstSampleIndex,
sampleCount,
/* sampleOffsetUs= */ 0,
/* decodeOnlyUntilUs= */ 0,
SAMPLE_FORMATS_SYNC_SAMPLES_ONLY,
SAMPLE_FLAGS_SYNC_SAMPLES_ONLY);
}
/** Asserts correct reading of standard test data from {@code sampleQueue}. */
private void assertReadTestData() {
assertReadTestData(/* startFormat= */ null, 0);
@ -1503,11 +1651,37 @@ public final class SampleQueueTest {
int sampleCount,
long sampleOffsetUs,
long decodeOnlyUntilUs) {
assertReadTestData(
startFormat,
firstSampleIndex,
sampleCount,
sampleOffsetUs,
decodeOnlyUntilUs,
SAMPLE_FORMATS,
SAMPLE_FLAGS);
}
/**
* Asserts correct reading of standard test data from {@code sampleQueue}.
*
* @param startFormat The format of the last sample previously read from {@code sampleQueue}.
* @param firstSampleIndex The index of the first sample that's expected to be read.
* @param sampleCount The number of samples to read.
* @param sampleOffsetUs The expected sample offset.
*/
private void assertReadTestData(
@Nullable Format startFormat,
int firstSampleIndex,
int sampleCount,
long sampleOffsetUs,
long decodeOnlyUntilUs,
Format[] sampleFormats,
int[] sampleFlags) {
Format format = adjustFormat(startFormat, sampleOffsetUs);
for (int i = firstSampleIndex; i < firstSampleIndex + sampleCount; i++) {
// Use equals() on the read side despite using referential equality on the write side, since
// sampleQueue de-duplicates written formats using equals().
Format testSampleFormat = adjustFormat(SAMPLE_FORMATS[i], sampleOffsetUs);
Format testSampleFormat = adjustFormat(sampleFormats[i], sampleOffsetUs);
if (!testSampleFormat.equals(format)) {
// If the format has changed, we should read it.
assertReadFormat(false, testSampleFormat);
@ -1519,7 +1693,7 @@ public final class SampleQueueTest {
long expectedTimeUs = SAMPLE_TIMESTAMPS[i] + sampleOffsetUs;
assertReadSample(
expectedTimeUs,
(SAMPLE_FLAGS[i] & C.BUFFER_FLAG_KEY_FRAME) != 0,
(sampleFlags[i] & C.BUFFER_FLAG_KEY_FRAME) != 0,
/* isDecodeOnly= */ expectedTimeUs < decodeOnlyUntilUs,
/* isEncrypted= */ false,
DATA,