From 75e28d82e3d03e5af22f3a41209b9a8b894244de Mon Sep 17 00:00:00 2001 From: ibaker Date: Wed, 19 Mar 2025 04:24:49 -0700 Subject: [PATCH] Fix FLAC interactions with PBA 'remaining capacity' The comment sounds like it is worried the next header won't fit after `limit` and before the end of `data`, but the code was previously only checking the space between `position` and `limit`. This led to some unnecessary copies. Running the Robolectric `FlacExtractorTest.sample()` test in only the 'partial reads' and I/O errors case (worst case for this bug) results in 57271 copies without this fix, and 19 copies with it. Sample of the first 10 copies before this fix, showing a copy is made for every byte read from the input: ``` W/ibaker: Making a copy. input.position=8881, data.length=32768, pos=1, lim=1 W/ibaker: Making a copy. input.position=8882, data.length=32768, pos=1, lim=1 W/ibaker: Making a copy. input.position=8883, data.length=32768, pos=1, lim=1 W/ibaker: Making a copy. input.position=8884, data.length=32768, pos=1, lim=1 W/ibaker: Making a copy. input.position=8885, data.length=32768, pos=1, lim=1 W/ibaker: Making a copy. input.position=8886, data.length=32768, pos=1, lim=1 W/ibaker: Making a copy. input.position=8887, data.length=32768, pos=1, lim=1 W/ibaker: Making a copy. input.position=8888, data.length=32768, pos=1, lim=1 W/ibaker: Making a copy. input.position=8889, data.length=32768, pos=1, lim=1 W/ibaker: Making a copy. input.position=8890, data.length=32768, pos=1, lim=1 ``` And the first 10 copies after the fix: ``` W/ibaker: Making a copy. input.position=41648, data.length=32768, pos=32753, lim=32768 W/ibaker: Making a copy. input.position=74401, data.length=32768, pos=32753, lim=32768 W/ibaker: Making a copy. input.position=107154, data.length=32768, pos=32753, lim=32768 W/ibaker: Making a copy. input.position=139907, data.length=32768, pos=32753, lim=32768 W/ibaker: Making a copy. input.position=172660, data.length=32768, pos=32753, lim=32768 W/ibaker: Making a copy. input.position=41648, data.length=32768, pos=32753, lim=32768 W/ibaker: Making a copy. input.position=74401, data.length=32768, pos=32753, lim=32768 W/ibaker: Making a copy. input.position=107154, data.length=32768, pos=32753, lim=32768 W/ibaker: Making a copy. input.position=139907, data.length=32768, pos=32753, lim=32768 W/ibaker: Making a copy. input.position=172660, data.length=32768, pos=32753, lim=32768 ``` PiperOrigin-RevId: 738341007 (cherry picked from commit 71ff9c661cccaf2d0c0f9c67008911b4ec5a4397) --- .../androidx/media3/extractor/flac/FlacExtractor.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/flac/FlacExtractor.java b/libraries/extractor/src/main/java/androidx/media3/extractor/flac/FlacExtractor.java index c2aca7bd8b..24c3299b83 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/flac/FlacExtractor.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/flac/FlacExtractor.java @@ -310,9 +310,13 @@ public final class FlacExtractor implements Extractor { currentFrameFirstSampleNumber = nextFrameFirstSampleNumber; } - if (buffer.bytesLeft() < FlacConstants.MAX_FRAME_HEADER_SIZE) { - // The next frame header may not fit in the rest of the buffer, so put the trailing bytes at - // the start of the buffer, and reset the position and limit. + int remainingBufferCapacity = buffer.getData().length - buffer.limit(); + if (buffer.bytesLeft() < FlacConstants.MAX_FRAME_HEADER_SIZE + && remainingBufferCapacity < FlacConstants.MAX_FRAME_HEADER_SIZE) { + // We're running out of bytes to read before buffer.limit, and the next frame header may not + // fit in the rest of buffer.data beyond buffer.limit, so we move the bytes between + // buffer.position and buffer.limit to the start of buffer.data, and reset the position and + // limit. int bytesLeft = buffer.bytesLeft(); System.arraycopy( buffer.getData(), buffer.getPosition(), buffer.getData(), /* destPos= */ 0, bytesLeft);