[FFmpeg-devel] [PATCH] libavformat/aac: Parse all ID3 tags present between ADTS frames

Mattias Amnefelt mattiasa at avm.se
Wed Apr 4 10:22:03 EEST 2018


On 2018-04-04 03:42, James Almer wrote:
> On 4/3/2018 10:40 PM, Carl Eugen Hoyos wrote:
>> 2018-04-04 3:38 GMT+02:00, James Almer <jamrial at gmail.com>:
>>> On 4/3/2018 10:33 PM, Carl Eugen Hoyos wrote:
>>>> 2018-04-03 7:58 GMT+02:00, Mattias Amnefelt <mattiasa at avm.se>:
>>>>> Yes, my feeling was also that it's better to handle this when possible.
>>>>>
>>>>> You are of course correct that the two tags needs to be inbetween
>>>>> frames. Sorry about that, I stripped the sample down too much. I updated
>>>>> with a sample which has two frames. This new sample fails the test
>>>>> without the patch.
>>>>> +fate-adts-id3v2-two-tags-demux: CMD = framecrc -f aac -i
>>>>> $(TARGET_SAMPLES)/aac/id3v2_two_tags.aac -c:a copy
>>>> The "-f aac" looks like a bad idea to me.
>>>> It's also true for the tests above, but that's still not reason to
>>>> add more.
>>>>
>>>> Please avoid top-posting here, Carl Eugen
>>> At least in one of them it was added because the sample had too few
>>> frames and probing was detecting it with a score of 1, which seemed too
>>> fragile.
>> I believe that it is good to have a sample that is detected with
>> a small score as part of fate.
>>
>> Carl Eugen
> When i asked it was suggested to just force the demuxer. I have no
> opinion one way or another, so feel free to change it.
I have to admit I just copy-n-pasted the test above. I just 
double-checked and all the id3 tag tests pass without -f aac now. I'm 
not sure if anything has changed since the test was added or not. Do you 
want a patch which removes it for all of them?

/mattiasa



More information about the ffmpeg-devel mailing list