[FFmpeg-devel] [PATCH] skip padding at the end of id3 tags

[Jai Menon]

On Wed, Oct 14, 2009 at 5:13 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Wed, Oct 14, 2009 at 02:01:15AM +0000, Jai Menon wrote:
>> On Tue, Oct 13, 2009 at 10:03:28PM +0200, Michael Niedermayer wrote:
>> > On Mon, Oct 12, 2009 at 09:27:47PM +0000, Jai Menon wrote:
>> > > Hi,
>> > >
>> > > I think the recent id3 tag parsing changes led to a regression as in
>> > > subject. Attached patch fixes this.
>>
>> [...]
>>
>> > > diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
>> > > index 88472e7..b3364f5 100644
>> > > --- a/libavformat/id3v2.c
>> > > +++ b/libavformat/id3v2.c
>> > > @@ -213,7 +213,10 @@ void ff_id3v2_parse(AVFormatContext *s, int len, uint8_t version, uint8_t flags)
>> > >
>> > > ? ? ? ? ?if (tag[0] == 'T')
>> > > ? ? ? ? ? ? ?read_ttag(s, tlen, tag);
>> > > -
>> > > + ? ? ? ?else if (!tag[0]) {
>> >
>> > the old code checked all of tag, this checks just the first byte
>> > why?
>>
>> Well, after a look at the spec [1], it seems that if a valid frame is
>> present, then the frame id consists of [A-Z0-9]. So I assumed that a 0
>
> there are several versions of id3v2 have you checked the others as well?

yes

>> would mean that we have run out of frames to process. I can change it
>> to the earlier variant if you wish.
>
> i think either that or add some check that checks for the first being
> 0 and the following being not and then print a warning
> i might be paranoid here but if that case happens the warning could
> safe someones time debuging a bug

okay. is the attached patch acceptable then?

-- 
Jai Menon
-------------- next part --------------
A non-text attachment was scrubbed...
Name: id3_skip.patch
Type: text/x-patch
Size: 557 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091014/d113d3e6/attachment.bin>