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

[James Almer]

On 9/5/2017 7:12 PM, Hendrik Leppkes wrote:
> 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.

Again, what if there's no new ADTS frame after the supposed end of the
one we're reading? How do we interpret that? That the one we read was
wrong or that it was right and there simply is no new ADTS frame right
after it? How does that help us decide if we propagate it or not?

If anything, i could maybe use avpriv_aac_parse_header(). Barely more
resilient than just looking for a sync code and the size field being >
7, but it's something i guess.

> 
> - Hendrik
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>