[FFmpeg-devel] [Ffmpeg-devel] ID3v2

[Andreas Öman]

Hi

Michael Niedermayer wrote:
> Hi
> 
> cosmetic v1/v2 renaming patch ok

Not resubmitting that again then.

> 
> the other patch should be split in id3v2 reading and writing patches

Done

> int v=0;
> while(len--)
>     v= (v<<7) + (get_byte(s)&0x7F);
> return v;
> 
> is more flexible and wont cause a buffer overflow if someone by
> mistake passes a len>4

Done

> if taglen is 0 before this function then len will be -1 here and the 0 will
> be written outside of the array

Yep

> 
> 
> [...]
> 
>> +    case 1:  /* UTF-16 we cannot handle */
>> +    case 2:  /* UTF-16BE we cannot handle */
>> +    default:
>> +        break;
>> +    }
> 
> this is useless

Apart from an informational perspective yes. Deleted it never the less.

> 
> 
> shouldnt this skip+return also be done in the default: case above?

Yeah, probably.

The id3v2-specification is pretty brain damaged when it comes to
the 'tag size' field in the header. 'tag size' does not include the
trailing footer (which is only present in v2.4 if flags & 0x10
is true).

quote: (from spec)

    The ID3v2 tag size is the sum of the byte length of the extended
    header, the padding and the frames after unsynchronisation. If a
    footer is present this equals to ('total size' - 20) bytes, otherwise
    ('total size' - 10) bytes.

This is IMHO a scholar example of how to NOT design these type of
things. I more correct design would have the initial 'tag size'
in the header include _everything_ so a parser could safely
skip all bytes if it cannot parse the particular version.

Very well, i believe this new version is at least as good
as the non-id3v2 mp3.c.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: id3v2-reader.diff
Type: text/x-patch
Size: 4660 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070521/c2de8ab2/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: id3v2-writer.diff
Type: text/x-patch
Size: 2374 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070521/c2de8ab2/attachment-0001.bin>