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
Without this, 32-bit files fail to play with `Playback stuck buffering
and not loading`. With this change, playback works on devices where the
`MediaCodec` FLAC decoder supports 32-bit, and crashes on devices with a
`MediaCodec` FLAC decoder that does not support 32-bit.
A follow-up change will aim to transform the 'unsupported' case from a
crash into a report that the track format is not supported.
32-bit support was only fully incorporated into the spec when RFC 9639
was [published in December
2024](https://xiph.org/flac/2024/12/19/rfc-9639-published.html), and
it was been supported by `libflac` (for encode and decode) [since
September 2022](https://xiph.org/flac/2022/09/09/flac-1-4-0-released.html).
The original version of this `FlacExtractor` was written before either
of these, so only supported up to 24-bit.
Issue: androidx/media#2197
PiperOrigin-RevId: 737559285
This change uses the duration and byte count of the Xing frame, if
they're present, to estimate the bitrate of the stream. The seeking
will still be inaccurate (because we're doing CBR seeking in a VBR
stream), but it will be more accurate than basing the bitrate on the
first MP3 frame we read. It will also mean the duration reported is
accurate.
This change also ensures that if the Xing frame contains the number of
audio bytes but no ToC, the audio data length is still propagated into
the `XingSeeker` (and therefore into the `ConstantBitrateSeeker` too).
Issue: androidx/media#2194
PiperOrigin-RevId: 736100104
These tests show the current behaviour:
1. Without customization: not seekable
2. With 'enable CBR' flag:
* File length known: seekable but duration very incorrect
* File length unknown: not seekable (even though the Xing frame
contains the number of bytes in the mp3 stream).
Follow-up test changes will:
1. Add a test for the cbr-always case for this file (branched from the
files submitted in this change).
2. Add a new similar test asset with no ToC but with trailing garbage
bytes, to verify that the length from the Xing frame is used.
Then follow-up implementation changes will update Mp3Extractor to use
duration and byte length from a Xing frame when falling back to CBR
seeking.
Issue: androidx/media#2194
PiperOrigin-RevId: 734164879
This is analagous to `String.codePointAt` and similar methods. By
dealing only in unicode code points, and returning an `int`, we can
avoid having to reason about how to handle UTF-8 code units that
require two UTF-16 code units (each one a Java `char`) to represent
them.
The zero-arg `peekChar` method remains undeprecated, because it behaves
as "you'd expect" when compared to `peekUnsignedByte` (always reads two
big-endian bytes).
PiperOrigin-RevId: 733752645
This was introduced in 841bdc6efe.
There's no need to cast the `char` (2 bytes) down to a `byte` in order
to pack it into an `int` (4 bytes) alongside a short (2 bytes).
We also don't need to use `short` to track the character count (max 4
with UTF-8) - a single byte is enough. This change also uses
`Ints.fromBytes` to avoid having to reason about how casting &
bit-shifting interact.
This change introduces a test which reproduces the failure reported in
Issue: androidx/media#2167.
#cherrypick
PiperOrigin-RevId: 730809219
In all these cases I found at least one public method that takes or
returns a type from the dependency, or a type that inherits from a type
defined in the dependency.
PiperOrigin-RevId: 726130595
The exception occurred when an edit list started at a non-sync frame with no preceding sync frame. The fix searches forward for the next sync frame in such cases, preventing the out-of-bounds access.
Issue: androidx/media#2062
#cherrypick
PiperOrigin-RevId: 720642687
The file in Issue: androidx/media#2052 contains a cue with the following timecode:
```
0:00:00:00,0:00:00:00
```
The content of this cue seems to be some 'converted by' metadata, i.e.
it's basically a comment and clearly not intended to be shown on
screen (since it has zero duration).
There is some fiddly logic later in `SsaParser` to support overlapping
cues with the old `Subtitle` structure [1], and this logic gets tripped
up by the start and end time being equal, which results in a
**single**, empty `List<Cue>` being added - which trips up another
assumption that every SSA cue line results in at least two `List<Cue>`
entries (one containing the cue text, and another containing an empty
list to signal the end of the cues).
This fiddly logic is no longer required, because overlapping
`CuesWithTiming` objects can now be merged in `TextRenderer`, so there
is a possible future simplification to `SsaParser` which removes a lot
of this complexity.
[1] Added in <unknown commit>
PiperOrigin-RevId: 718380386
This is aligned with the documentation of `MediaCodec` which says the
timestamp of a buffer with `BUFFER_FLAG_END_OF_STREAM` should be
ignored:
https://developer.android.com/reference/android/media/MediaCodec#end-of-stream-handling
Add a test that exercises this by clipping off the end of a sample with
CEA-608 captions, because this creates an EOS-flagged buffer with a
non-EOS timestamp.
Also add a straightforward playback test for the
`fragmented_captions.mp4` sample.
PiperOrigin-RevId: 715716036
Add a new flag to FragmentedMp4Extractor
FLAG_READ_WITHIN_GOP_SAMPLE_DEPENDENCIES_H265
Read two bytes from H.265 videos to determine NAL unit type and
temporal layer id.
PiperOrigin-RevId: 714046987
ExoPlayer assumed 4-bytes for length in two places (by assuming the
length is the same as the 4-byte NAL start code):
1. In `AvcConfig` we transform length-delimited to start-delimited
before writing into `initializationData`, and then skip
'nal unit length field' bytes when parsing from `initializationData`
(when we should skip 'start code length' bytes instead).
2. In `Mp4Extractor.readSample` we modify the local variable
`sampleSize` to fix the difference between length field length and
start code length, but **only on the first attempt to read a
sample**. If we are resuming in the middle of reading a sample (after
a recoverable I/O error), this fix for `sampleSize` is not done,
which means we end up missing the last 2-3 bytes of the sample when
the NAL length is 1-2 bytes.
* This is fixed by moving the `sampleSize` 'fixing' code to outside
the `if (sampleCurrentNalBytesRemaining == 0)` block.
* `FragmentedMp4Extractor` has very similar code, but uses a
field for `sampleSize`, rather than a local, so doesn't look
vulnerable to the same problem (though I haven't totally
tested this).
This change adds a test file with 2-byte NAL lengths, generated by
hacking the media3 muxer to emit 2-byte NAL lengths and transforming
`sample.mp4` using the transformer demo app.
PiperOrigin-RevId: 713709203
The previous FragmentedMp4 captions test asset doesn't have captions.
Fix a bug where captions before extractor seek were output after.
PiperOrigin-RevId: 713665817
