[FFmpeg-devel] [PATCH] avformat/aacdec: don't immediately abort if an ADTS frame is not found

[Hendrik Leppkes]

On Tue, Sep 5, 2017 at 11:20 PM, James Almer <jamrial at gmail.com> wrote:
> On 9/5/2017 5:30 PM, Hendrik Leppkes wrote:
>> On Mon, Sep 4, 2017 at 4:47 AM, James Almer <jamrial at gmail.com> wrote:
>>> Skip the invalid data in an attempt to find one instead, and continue decoding from there.
>>>
>>> Fixes ticket #6634
>>>
>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>> ---
>>>  libavformat/aacdec.c | 41 +++++++++++++++++++++++++++++------------
>>>  1 file changed, 29 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/libavformat/aacdec.c b/libavformat/aacdec.c
>>> index 364b33404f..7aacc88560 100644
>>> --- a/libavformat/aacdec.c
>>> +++ b/libavformat/aacdec.c
>>> @@ -77,10 +77,29 @@ static int adts_aac_probe(AVProbeData *p)
>>>          return 0;
>>>  }
>>>
>>> +static int resync(AVFormatContext *s)
>>> +{
>>> +    uint16_t state;
>>> +
>>> +    state = avio_r8(s->pb);
>>> +    while (!avio_feof(s->pb) && avio_tell(s->pb) < s->probesize) {
>>> +        state = (state << 8) | avio_r8(s->pb);
>>> +        if ((state >> 4) != 0xFFF)
>>> +            continue;
>>> +        avio_seek(s->pb, -2, SEEK_CUR);
>>> +        break;
>>> +    }
>>> +
>>
>> The ADTS sync code isn't that much of a sync code, maybe it might be
>> more resilient if you try to read the frame size and check if after
>> that the next frame also starts with a valid code?
>
> That will only let me know if at the end of the frame is another frame,
> and not if the frame I'm reading is complete or not.
> Maybe it's complete and right after it there's an id3v1 tag. Maybe it's
> complete and right after it there's garbage. Maybe it's incomplete
> because another ADTS frame started suddenly in the middle of the one i
> tried to read because the source is some weird stream (sample in the
> ticket this fixes), and after reading the reported size of the intended
> frame the demuxer will find itself in the middle of the new one but
> unable to know that's the case.
>
> Really, at the demuxer level i can't do much more than read an ADTS
> header, make sure it's at least seven bytes long and that the reported
> size of the entire frame is bigger than the header size, then make a
> packet out of that. The decoder/parser will handle things from there,
> knowing that at very least for the first few bytes what reaches them is
> an ADTS frame.
>

We're not talking about validating the ADTS frame, but just making
sure you really "resync" to the start of a frame, and not some
arbitrary random position that just happens to be 0xFFF, because that
code isn't very long or very special.

- Hendrik