[FFmpeg-devel] [PATCH] id3v2: always skip padding

[Michael Niedermayer]

On Wed, Jul 28, 2010 at 12:48:42AM +1000, Alexander Kojevnikov wrote:
> On 27 July 2010 23:23, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Tue, Jul 27, 2010 at 12:29:29PM +1000, Alexander Kojevnikov wrote:
> >> Hi list,
> >>
> >> If padding at the end of an id3v2 tag is less than `taghdrlen` (10
> >> bytes), it is not skipped. As a result, the buffer points at an
> >> incorrect position which later results in "Header missing" errors.
> >>
> >> Attached patch fixes this issue.
> >>
> >> Cheers,
> >> Alex
> >
> >> ?id3v2.c | ? ?5 ++++-
> >> ?1 file changed, 4 insertions(+), 1 deletion(-)
> >> 0fee5829ca199fc7ff83ad27e5ee8af839a365ab ?0001-id3v2-always-skip-padding.patch
> >> From 19f82c52719b7cac498acdc1392c9c8c80eafb3f Mon Sep 17 00:00:00 2001
> >> From: Alexander Kojevnikov <alexander at kojevnikov.com>
> >> Date: Tue, 27 Jul 2010 12:20:37 +1000
> >> Subject: [PATCH] id3v2: always skip padding
> >>
> >> ---
> >> ?libavformat/id3v2.c | ? ?5 ++++-
> >> ?1 files changed, 4 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
> >> index 38c272a..8d050af 100644
> >> --- a/libavformat/id3v2.c
> >> +++ b/libavformat/id3v2.c
> >> @@ -266,13 +266,16 @@ void ff_id3v2_parse(AVFormatContext *s, int len, uint8_t version, uint8_t flags)
> >> ? ? ? ? ?else if (!tag[0]) {
> >> ? ? ? ? ? ? ?if (tag[1])
> >> ? ? ? ? ? ? ? ? ?av_log(s, AV_LOG_WARNING, "invalid frame id, assuming padding");
> >> - ? ? ? ? ? ?url_fskip(s->pb, len);
> >
> > this maybe should be tlen
> 
> It has to be `len` which initially contains the size of the entire tag
> including the padding and is decremented after each tag frame is read.
> `tlen` on the other hand contains the size of the current tag frame,
> it tells us nothing about padding.

no
look at the code:

        len -= taghdrlen + tlen;
...
        next = url_ftell(s->pb) + tlen;

        if (tag[0] == 'T')
            read_ttag(s, tlen, tag);
        else if (!tag[0]) {
            if (tag[1])
                av_log(s, AV_LOG_WARNING, "invalid frame id, assuming padding");
            url_fskip(s->pb, len);
            break;
        }
        /* Skip to end of tag */
        url_fseek(s->pb, next, SEEK_SET);


tlen is subtracted from len but its not read or skiped in any way in the
break case above



[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 190 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100727/1282e833/attachment.pgp>