[FFmpeg-cvslog] r19275 - trunk/libavformat/id3v2.c

[Jai Menon]

On Mon, Jun 29, 2009 at 6:50 PM, Michael Niedermayer<michaelni at gmx.at> wrote:
> On Mon, Jun 29, 2009 at 05:03:26PM +0530, Jai Menon wrote:
>> On Mon, Jun 29, 2009 at 2:32 PM, Jai Menon<jmenon86 at gmail.com> wrote:
>> > On Fri, Jun 26, 2009 at 12:56 AM, michael<subversion at mplayerhq.hu> wrote:
>> >> Author: michael
>> >> Date: Thu Jun 25 21:26:39 2009
>> >> New Revision: 19275
>> >>
>> >> Log:
>> >> Fix id3v2.3/4 tag size parsing.
>> >> Fixes issue1106
>> >>
>> >> Modified:
>> >> ? trunk/libavformat/id3v2.c
>> >>
>> >> Modified: trunk/libavformat/id3v2.c
>> >> ==============================================================================
>> >> --- trunk/libavformat/id3v2.c ? Thu Jun 25 21:10:27 2009 ? ? ? ?(r19274)
>> >> +++ trunk/libavformat/id3v2.c ? Thu Jun 25 21:26:39 2009 ? ? ? ?(r19275)
>> >> @@ -155,7 +155,7 @@ void ff_id3v2_parse(AVFormatContext *s,
>> >> ? ? while (len >= taghdrlen) {
>> >> ? ? ? ? if (isv34) {
>> >> ? ? ? ? ? ? tag ?= get_be32(s->pb);
>> >> - ? ? ? ? ? ?tlen = get_size(s->pb, 4);
>> >> + ? ? ? ? ? ?tlen = get_be32(s->pb);
>> >
>> > About this, the spec at [1] seems to suggest that :
>> >
>> > "The ID3v2 tag size is stored as a 32 bit synchsafe integer (section
>> > ? 6.2), making a total of 28 effective bits (representing up to 256MB)"
>>
>> Sorry, wrong citation, here is the correct one :
>>
>> ? ?The frame ID is followed by a size descriptor containing the size of
>> ? ?the data in the final frame, after encryption, compression and
>> ? ?unsynchronisation. The size is excluding the frame header ('total
>> ? ?frame size' - 10 bytes) and stored as a 32 bit synchsafe integer.
>>
>> If we go by this, then apic frame size in those samples (issue 1106
>> and 1202) is invalid and maybe should be dealt as a special case. I'm
>> not quite sure, thought you might have insights on this.
>
> iam not quite sure either but i did understand the i3v2.2/3 specs as not
> doing synchsafing on these fields. I missed that id3v2.4 does as you quote

Actually i had written the patch and was going to post it but looks
like its not needed. thanks for taking care of this :)

> lets see if this now works or if some files turn up that still fail ...

I think it should be spec compliant now so hopefully no problems.

<offtopic>

we should probably export more of those frame ids now that the
metadata api is in place :)
dunno about album art though.

</offtopic>

-- 
Regards,

Jai