Ensure track indices are only added when samples are committed

Previously, track IDs were added to `trackIndicesPerSampleInQueuedOrder`
even when the sample was not committed. This caused issues where attempts
to read samples from the `SampleQueue` returned `C.RESULT_READ_NOTHING`,
which led to an exception being thrown due to the assumption that samples
were available to read.

This fix updates the logic to track sample commits by comparing the write index before and after calling `SampleQueue.sampleMetadata`. Track indices are only added if the sample was committed, ensuring accurate sample handling and avoiding exceptions.

PiperOrigin-RevId: 675526115
This commit is contained in:
rohks 2024-09-17 05:24:10 -07:00 committed by Copybara-Service
parent 873d485056
commit 9bc89ae989
2 changed files with 51 additions and 15 deletions

View File

@ -17,6 +17,7 @@ package androidx.media3.exoplayer;
import static androidx.annotation.VisibleForTesting.NONE;
import static androidx.media3.common.util.Assertions.checkNotNull;
import static androidx.media3.common.util.Assertions.checkState;
import static androidx.media3.exoplayer.source.SampleStream.FLAG_OMIT_SAMPLE_DATA;
import static androidx.media3.exoplayer.source.SampleStream.FLAG_PEEK;
import static androidx.media3.exoplayer.source.SampleStream.FLAG_REQUIRE_FORMAT;
@ -609,17 +610,8 @@ public final class MediaExtractorCompat {
formatHolder, decoderInputBuffer, readFlags, /* loadingFinished= */ false);
}
formatHolder.clear();
// Additional logging is added to debug b/241321832.
if (result != C.RESULT_BUFFER_READ) {
// This method should only be called when there is a sample available for reading.
throw new IllegalStateException(
Util.formatInvariant(
"Sample read result: %s\n"
+ "Track sample: %s\n"
+ "TrackIndicesPerSampleInQueuedOrder: %s\n"
+ "Tracks added: %s\n",
result, trackOfSample, trackIndicesPerSampleInQueuedOrder, tracks));
}
// This method must only be called when there is a sample available for reading.
checkState(result == C.RESULT_BUFFER_READ);
}
/**
@ -909,12 +901,16 @@ public final class MediaExtractorCompat {
// Disable BUFFER_FLAG_LAST_SAMPLE to prevent the sample queue from ignoring
// FLAG_REQUIRE_FORMAT. See b/191518632.
flags &= ~C.BUFFER_FLAG_LAST_SAMPLE;
if (compatibilityTrackIndex != C.INDEX_UNSET) {
trackIndicesPerSampleInQueuedOrder.addLast(compatibilityTrackIndex);
}
Assertions.checkState(mainTrackIndex != C.INDEX_UNSET);
trackIndicesPerSampleInQueuedOrder.addLast(mainTrackIndex);
int writeIndexBeforeCommitting = this.getWriteIndex();
super.sampleMetadata(timeUs, flags, size, offset, cryptoData);
// Add the track index if the sample was committed
if (this.getWriteIndex() == writeIndexBeforeCommitting + 1) {
if (compatibilityTrackIndex != C.INDEX_UNSET) {
trackIndicesPerSampleInQueuedOrder.addLast(compatibilityTrackIndex);
}
trackIndicesPerSampleInQueuedOrder.addLast(mainTrackIndex);
}
}
@Override

View File

@ -576,6 +576,46 @@ public class MediaExtractorCompatTest {
assertThat(mediaExtractorCompat.getTrackCount()).isEqualTo(2);
}
@Test
public void readNonSyncSample_whenSyncSampleIsExpected_noSampleIsQueued() throws IOException {
TrackOutput[] outputs = new TrackOutput[1];
byte[] sampleData = new byte[] {(byte) 1, (byte) 2, (byte) 3};
fakeExtractor.addReadAction(
(input, seekPosition) -> {
outputs[0] = extractorOutput.track(/* id= */ 0, C.TRACK_TYPE_VIDEO);
extractorOutput.endTracks();
outputs[0].format(
new Format.Builder()
.setSampleMimeType(MimeTypes.VIDEO_H264)
.setCodecs("avc.123")
.build());
return Extractor.RESULT_CONTINUE;
});
// Add a non-sync sample. This sample should be ignored as a sync sample is expected
// at the start of the video.
fakeExtractor.addReadAction(
(input, seekPosition) -> {
outputSampleData(outputs[0], sampleData);
outputs[0].sampleMetadata(
/* timeUs= */ 7,
/* flags= */ 0,
/* size= */ 3,
/* offset= */ 0,
/* cryptoData= */ null);
return Extractor.RESULT_CONTINUE;
});
mediaExtractorCompat.setDataSource(PLACEHOLDER_URI, /* offset= */ 0);
mediaExtractorCompat.selectTrack(0);
// Assert that when a keyframe is expected, no sample is queued if a non-keyframe sample is
// read.
assertThat(mediaExtractorCompat.getSampleTrackIndex()).isEqualTo(-1);
assertThat(mediaExtractorCompat.getSampleTime()).isEqualTo(-1);
assertThat(mediaExtractorCompat.getSampleSize()).isEqualTo(-1);
assertThat(mediaExtractorCompat.readSampleData(ByteBuffer.allocate(0), /* offset= */ 0))
.isEqualTo(-1);
}
// Internal methods.
private void assertReadSample(int trackIndex, long timeUs, int size, byte... sampleData) {