Update bypass decode-only logic to timestamp-based pattern

This aligns the bypass code with the MediaCodec logic to not
use buffer.isDecodeOnly and instead rely on a timestamp
comparison with the last reset time. Overall, the change should
not introduce a functional difference and is covered by existing
end-to-end tests with bypass playbacks.

Splitting bypass batch buffers by decoder-only flag is
technically wrong for formats where not every buffer is a
keyframe, and this change also leaves a TODO to explain this
known shortcoming.

PiperOrigin-RevId: 556800038
This commit is contained in:
tonihei 2023-08-14 16:31:55 +01:00 committed by oceanjules
parent a48b99214e
commit 3ccb920930
3 changed files with 17 additions and 51 deletions

View file

@ -105,9 +105,6 @@ import java.nio.ByteBuffer;
setFlags(C.BUFFER_FLAG_KEY_FRAME); setFlags(C.BUFFER_FLAG_KEY_FRAME);
} }
} }
if (buffer.isDecodeOnly()) {
setFlags(C.BUFFER_FLAG_DECODE_ONLY);
}
@Nullable ByteBuffer bufferData = buffer.data; @Nullable ByteBuffer bufferData = buffer.data;
if (bufferData != null) { if (bufferData != null) {
ensureSpaceForWrite(bufferData.remaining()); ensureSpaceForWrite(bufferData.remaining());
@ -125,9 +122,6 @@ import java.nio.ByteBuffer;
if (sampleCount >= maxSampleCount) { if (sampleCount >= maxSampleCount) {
return false; return false;
} }
if (buffer.isDecodeOnly() != isDecodeOnly()) {
return false;
}
@Nullable ByteBuffer bufferData = buffer.data; @Nullable ByteBuffer bufferData = buffer.data;
if (bufferData != null if (bufferData != null
&& data != null && data != null

View file

@ -2252,6 +2252,7 @@ public abstract class MediaCodecRenderer extends BaseRenderer {
// Process any batched data. // Process any batched data.
checkState(!outputStreamEnded); checkState(!outputStreamEnded);
if (bypassBatchBuffer.hasSamples()) { if (bypassBatchBuffer.hasSamples()) {
boolean isDecodeOnly = bypassBatchBuffer.getLastSampleTimeUs() < getLastResetPositionUs();
if (processOutputBuffer( if (processOutputBuffer(
positionUs, positionUs,
elapsedRealtimeUs, elapsedRealtimeUs,
@ -2261,7 +2262,7 @@ public abstract class MediaCodecRenderer extends BaseRenderer {
/* bufferFlags= */ 0, /* bufferFlags= */ 0,
bypassBatchBuffer.getSampleCount(), bypassBatchBuffer.getSampleCount(),
bypassBatchBuffer.getFirstSampleTimeUs(), bypassBatchBuffer.getFirstSampleTimeUs(),
bypassBatchBuffer.isDecodeOnly(), isDecodeOnly,
bypassBatchBuffer.isEndOfStream(), bypassBatchBuffer.isEndOfStream(),
outputFormat)) { outputFormat)) {
// The batch buffer has been fully processed. // The batch buffer has been fully processed.
@ -2344,7 +2345,8 @@ public abstract class MediaCodecRenderer extends BaseRenderer {
&& inputFormat.sampleMimeType.equals(MimeTypes.AUDIO_OPUS)) { && inputFormat.sampleMimeType.equals(MimeTypes.AUDIO_OPUS)) {
oggOpusAudioPacketizer.packetize(bypassSampleBuffer, inputFormat.initializationData); oggOpusAudioPacketizer.packetize(bypassSampleBuffer, inputFormat.initializationData);
} }
if (!bypassBatchBuffer.append(bypassSampleBuffer)) { if (!haveBypassBatchBufferAndNewSampleSameDecodeOnlyState()
|| !bypassBatchBuffer.append(bypassSampleBuffer)) {
bypassSampleBufferPending = true; bypassSampleBufferPending = true;
return; return;
} }
@ -2355,6 +2357,19 @@ public abstract class MediaCodecRenderer extends BaseRenderer {
} }
} }
private boolean haveBypassBatchBufferAndNewSampleSameDecodeOnlyState() {
// TODO: b/295800114 - Splitting the batch buffer by decode-only state isn't safe for formats
// where not every sample is a keyframe because the downstream component may receive encoded
// data starting from a non-keyframe sample.
if (!bypassBatchBuffer.hasSamples()) {
return true;
}
long lastResetPositionUs = getLastResetPositionUs();
boolean batchBufferIsDecodeOnly = bypassBatchBuffer.getLastSampleTimeUs() < lastResetPositionUs;
boolean sampleBufferIsDecodeOnly = bypassSampleBuffer.timeUs < lastResetPositionUs;
return batchBufferIsDecodeOnly == sampleBufferIsDecodeOnly;
}
private static boolean isMediaCodecException(IllegalStateException error) { private static boolean isMediaCodecException(IllegalStateException error) {
if (Util.SDK_INT >= 21 && isMediaCodecExceptionV21(error)) { if (Util.SDK_INT >= 21 && isMediaCodecExceptionV21(error)) {
return true; return true;

View file

@ -104,49 +104,6 @@ public final class BatchBufferTest {
assertThat(batchBuffer.getLastSampleTimeUs()).isEqualTo(customMaxSampleCount - 1); assertThat(batchBuffer.getLastSampleTimeUs()).isEqualTo(customMaxSampleCount - 1);
} }
@Test
public void appendFirstSample_withDecodeOnlyFlag_setsDecodeOnlyFlag() {
initSampleBuffer();
sampleBuffer.setFlags(C.BUFFER_FLAG_DECODE_ONLY);
batchBuffer.append(sampleBuffer);
assertThat(batchBuffer.isDecodeOnly()).isTrue();
}
@Test
public void appendSecondSample_toDecodeOnlyBuffer_withDecodeOnlyFlag_succeeds() {
initSampleBuffer();
sampleBuffer.setFlags(C.BUFFER_FLAG_DECODE_ONLY);
batchBuffer.append(sampleBuffer);
initSampleBuffer();
sampleBuffer.setFlags(C.BUFFER_FLAG_DECODE_ONLY);
assertThat(batchBuffer.append(sampleBuffer)).isTrue();
}
@Test
public void appendSecondSample_toDecodeOnlyBuffer_withoutDecodeOnlyFlag_fails() {
initSampleBuffer();
sampleBuffer.setFlags(C.BUFFER_FLAG_DECODE_ONLY);
batchBuffer.append(sampleBuffer);
initSampleBuffer();
assertThat(batchBuffer.append(sampleBuffer)).isFalse();
}
@Test
public void appendSecondSample_toNonDecodeOnlyBuffer_withDecodeOnlyFlag_fails() {
initSampleBuffer();
batchBuffer.append(sampleBuffer);
initSampleBuffer();
sampleBuffer.setFlags(C.BUFFER_FLAG_DECODE_ONLY);
assertThat(batchBuffer.append(sampleBuffer)).isFalse();
}
@Test @Test
public void appendSecondSample_withKeyframeFlag_setsKeyframeFlag() { public void appendSecondSample_withKeyframeFlag_setsKeyframeFlag() {
initSampleBuffer(); initSampleBuffer();