[FFmpeg-devel] [PATCH] ADTS AAC with ID3v2

[David DeHaven]

On Jan 14, 2009, at 10:53 AM, Michael Niedermayer wrote:

> On Wed, Jan 14, 2009 at 01:39:05PM -0500, Alex Converse wrote:
>> On Wed, Jan 14, 2009 at 12:57 PM, Michael Niedermayer <michaelni at gmx.at 
>> >wrote:
>>
>>> On Wed, Jan 14, 2009 at 10:17:33AM -0500, Alex Converse wrote:
>>>> On Wed, Jan 14, 2009 at 10:10 AM, Michael Niedermayer <michaelni at gmx.at
>>>> wrote:
>>>>
>>>>> On Wed, Jan 14, 2009 at 09:47:29AM -0500, Alex Converse wrote:
>>>>>> On Wed, Jan 14, 2009 at 9:17 AM, Michael Niedermayer <
>>> michaelni at gmx.at
>>>>>> wrote:
>>>>>>
>>>>>>> On Wed, Jan 14, 2009 at 12:23:00AM -0500, Alex Converse wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> The attached patch adds support to the ADTS AAC probe to detect
>>> AAC
>>>>> files
>>>>>>>> with ID3v2 tags at a higher level than the MP3 probe and the  
>>>>>>>> MPC
>>>>> probe.
>>>>>>> It
>>>>>>>> refactors some id3 detection from mp3.c into id3v2.c.
>>>>>>>>
>>>>>>> [...]
>>>>>>>
>>>>>>>> #define ID3v1_TAG_SIZE 128
>>>>>>>> -
>>>>>>>> #define ID3v1_GENRE_MAX 125
>>>>>>>>
>>>>>>>> static const char * const id3v1_genre_str[ID3v1_GENRE_MAX +  
>>>>>>>> 1] =
>>> {
>>>>>>>
>>>>>>> cosmetic
>>>>>>>
>>>>>>> [...]
>>>>>>>> @@ -487,7 +472,7 @@ static int mp3_read_header(AVFormatContext
>>> *s,
>>>>>>>>     ret = get_buffer(s->pb, buf, ID3v2_HEADER_SIZE);
>>>>>>>>     if (ret != ID3v2_HEADER_SIZE)
>>>>>>>>         return -1;
>>>>>>>> -    if (id3v2_match(buf)) {
>>>>>>>> +    if (ff_id3v2_match(buf)) {
>>>>>>>>         /* parse ID3v2 header */
>>>>>>>>         len = ((buf[6] & 0x7f) << 21) |
>>>>>>>>             ((buf[7] & 0x7f) << 14) |
>>>>>>>
>>>>>>> the split out of (ff_)id3v2_match is ok but should be a seperate
>>> patch
>>>>>>> and commit
>>>>>>>
>>>>>>> [...]
>>>>>>>>     }
>>>>>>>> -    if   (first_frames>=3) return AVPROBE_SCORE_MAX/2+1;
>>>>>>>> +    if   (first_frames>=3) return AVPROBE_SCORE_MAX/2+2;
>>>>>>>>     else if(max_frames>500)return AVPROBE_SCORE_MAX/2;
>>>>>>>>     else if(max_frames>=3) return AVPROBE_SCORE_MAX/4;
>>>>>>>>     else if(max_frames>=1) return 1;
>>>>>>>
>>>>>>> why?
>>>>>>
>>>>>>
>>>>>> Just having an ID3v2 tag gives a score of AVPROBE_SCORE_MAX/2+1  
>>>>>> for
>>> mp3.
>>>>>
>>>>> That sounds like a bug ...
>>>>>
>>>>
>>>> The comment about mpeg-ps in mp3.c made me think it was a hackish  
>>>> design
>>>> decision, not a bug.
>>>
>>> well one could call it that way, still the existence of hackish  
>>> code is no
>>> argument to add more hackish code like the +1 -> +2
>>>
>>
>> What would be the correct solution for MP3 then? skipping the tag  
>> and trying
>> again?
>
> i think so

I'll second that and add that it affects FLAC files I've seen in the  
wild too... I had a fix at some point that skipped the ID3 if it was  
there, I might still have it somewhere. I could update it for the  
separate id3v2 code and pass it along.

-DrD-