[FFmpeg-devel] [PATCH] fixes in mpeg audio parsing

[Michael Niedermayer]

On Thu, Nov 13, 2008 at 09:29:36AM +0200, Yoav Steinberg wrote:
>
>
> Michael Niedermayer wrote:
>> On Wed, Nov 12, 2008 at 07:02:55PM +0200, Yoav Steinberg wrote:
>>> Hi,
>>> Attached are patches witch make some improvements for mp3 parsing.
>>> skip_vbrtag_and_id3v1.patch:
>>> - When file contains ID3v1 tag at the end don't attempt to read it as 
>>> part of a packet. In some cases there are mp3 files where the ID3v1 tag 
>>> was stuck forcefully at the end of the file inside the last frame 
>>> boundary. In those cases the patch eliminates attempts to parse/decode 
>>> the ID3 tag.
>>> - If we find a VBR tag at the beginning of the file don't attempt to 
>>> parse it as a valid frame.
>> These are 2 seperate things which must be in seperate patches.
>
> Here is a single patch for avoiding parsing of VBR tag information frames.
>
>>> @@ -439,8 +446,14 @@
>>>      }
>>>       if(frames < 0)
>>> +    {
>>> +        url_fseek(s->pb, base, SEEK_SET);
>>>          return;
>>> +    }
>> the brace placement is inconsistant relative to the existing
>
> oops, fixed.
>
>>>  +    /* Skip the vbr tag frame */
>>> +    url_fseek(s->pb, base + vbrtag_size, SEEK_SET);
>>> +
>>>      spf = c.lsf ? 576 : 1152; /* Samples per frame, layer 3 */
>>>      st->duration = av_rescale_q(frames, (AVRational){spf, 
>>> c.sample_rate},
>>>                                  st->time_base);
>>> @@ -452,7 +465,6 @@
>>>      AVStream *st;
>>>      uint8_t buf[ID3v1_TAG_SIZE];
>>>      int len, ret, filesize;
>>> -    int64_t off;
>>>       st = av_new_stream(s, 0);
>>>      if (!st)
>>> @@ -492,9 +504,7 @@
>>>          url_fseek(s->pb, 0, SEEK_SET);
>>>      }
>>>  -    off = url_ftell(s->pb);
>>> -    mp3_parse_vbr_tags(s, st, off);
>>> -    url_fseek(s->pb, off, SEEK_SET);
>>> +    mp3_parse_vbr_tags(s, st, url_ftell(s->pb));
>> url_ftell() can be called in mp3_parse_vbr_tags()
>
> Removed base param from mp3_parse_vbr_tags, now url_ftell is called from 
> within.
>
>

> Index: mp3.c
> ===================================================================
> --- mp3.c	(revision 15776)
> +++ mp3.c	(working copy)
> @@ -402,18 +402,21 @@
>  /**
>   * Try to find Xing/Info/VBRI tags and compute duration from info therein
>   */
> -static void mp3_parse_vbr_tags(AVFormatContext *s, AVStream *st, int64_t base)
> +static void mp3_parse_vbr_tags(AVFormatContext *s, AVStream *st)
>  {
>      uint32_t v, spf;
>      int frames = -1; /* Total number of frames in file */
>      const int64_t xing_offtbl[2][2] = {{32, 17}, {17,9}};
>      MPADecodeContext c;
> +    int vbrtag_size = 0;
> +    int64_t base = url_ftell(s->pb);
>  
>      v = get_be32(s->pb);
>      if(ff_mpa_check_header(v) < 0)
>        return;

a return here will randomize the file position, and similarly will
for other returns


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

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081113/0141c7d1/attachment.pgp>