Skip to content

Commit 75e28d8

Browse files
icbakertonihei
authored andcommitted
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 71ff9c6)
1 parent 673da97 commit 75e28d8

File tree

1 file changed

+7
-3
lines changed

1 file changed

+7
-3
lines changed

libraries/extractor/src/main/java/androidx/media3/extractor/flac/FlacExtractor.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -310,9 +310,13 @@ private void getFrameStartMarker(ExtractorInput input) throws IOException {
310310
currentFrameFirstSampleNumber = nextFrameFirstSampleNumber;
311311
}
312312

313-
if (buffer.bytesLeft() < FlacConstants.MAX_FRAME_HEADER_SIZE) {
314-
// The next frame header may not fit in the rest of the buffer, so put the trailing bytes at
315-
// the start of the buffer, and reset the position and limit.
313+
int remainingBufferCapacity = buffer.getData().length - buffer.limit();
314+
if (buffer.bytesLeft() < FlacConstants.MAX_FRAME_HEADER_SIZE
315+
&& remainingBufferCapacity < FlacConstants.MAX_FRAME_HEADER_SIZE) {
316+
// We're running out of bytes to read before buffer.limit, and the next frame header may not
317+
// fit in the rest of buffer.data beyond buffer.limit, so we move the bytes between
318+
// buffer.position and buffer.limit to the start of buffer.data, and reset the position and
319+
// limit.
316320
int bytesLeft = buffer.bytesLeft();
317321
System.arraycopy(
318322
buffer.getData(), buffer.getPosition(), buffer.getData(), /* destPos= */ 0, bytesLeft);

0 commit comments

Comments
 (0)