[FFmpeg-devel] [PATCH] avformat/id3v2: support buggy id3v2.3 tag length in id3v2.4

Benoit Fouet benoit.fouet at free.fr
Thu Oct 16 18:06:39 CEST 2014


Hi,

On 16 October 2014 17:10:38 CEST, Michael Niedermayer <michaelni at gmx.at> wrote:
>On Thu, Oct 16, 2014 at 11:44:47AM +0200, Benoit Fouet wrote:
>> Some encoders do not use syncsafe sizes in v2.4 id3 tags. Check the
>next
>> tag to try to choose between the two.
>> 
>> Fixes ticket #4003
>> ---
>>  libavformat/id3v2.c | 42 +++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 41 insertions(+), 1 deletion(-)
>> 
>> diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
>> index 5469e0a..3bccd76 100644
>> --- a/libavformat/id3v2.c
>> +++ b/libavformat/id3v2.c
>> @@ -170,6 +170,23 @@ static unsigned int get_size(AVIOContext *s, int
>len)
>>      return v;
>>  }
>>  
>> +/* No real verification, only check that the tag consists of
>> + * a combination of capital alpha-numerical characters */
>> +static int is_tag(const char *buf, int len)
>> +{
>> +    if (!len)
>> +        return 0;
>> +
>> +    while (len--)
>> +        if ((buf[len] < 'A' ||
>> +             buf[len] > 'Z') &&
>> +            (buf[len] < '0' ||
>> +             buf[len] > '9'))
>> +            return 0;
>> +
>> +    return 1;
>> +}
>> +
>>  /**
>>   * Free GEOB type extra metadata.
>>   */
>> @@ -734,8 +751,31 @@ static void id3v2_parse(AVIOContext *pb,
>AVDictionary **metadata,
>>              tag[4] = 0;
>>              if (version == 3) {
>>                  tlen = avio_rb32(pb);
>> -            } else
>> +            } else {
>>                  tlen = get_size(pb, 4);
>> +                if (tlen > 0x7f) {
>
>i think that check isnt sufficient to detect that the 2 readings
>differ. For example 0xFF would be read as 0xFF by one and 0x7F by
>the other and would not trigger this
> 

True, although I guess that could be handled by just making this test a >= instead of >.

>also maybe len could be used to distingish a subset of cases
> 

Not sure I understand what you mean here... The tlen check seems to be enough to me.

>and there is the problem with seekable=0 streams where seeking
>back might fail, not sure what to do in that case ...
> 

We could theoretically buffer the data instead of seeking back, as the syncsafe size will always be smaller than the other one. Is this something that we want?
I can work on something like that.

-- 
Ben




More information about the ffmpeg-devel mailing list