[FFmpeg-devel] skip multiple id3v2 headers

[Michael Niedermayer]

On Wed, Sep 01, 2010 at 04:46:58PM -0700, David Byron wrote:
> I've run across some mp3 files with multiple id3v2 tags at the beginning.
> In some particularly annoying cases, an additional tag contains data that
> looks like the beginning of an mpeg frame.  Here's a patch to teach ffmpeg
> to ignore additional id3v2 tags at the beginning of mp3 files.
> 
> This is my first time submitting a patch to ffmpeg.  I tried to follow the
> rules at http://ffmpeg.org/developer.html but I'm not sure I did everything
> correctly.  I do get some test failures, but the same ones before and after
> the patch.  Same for checkheaders.  I can send these failures separately if
> people are interested.  I could use a hand resolving them.
> 
> The mp3-conformance tests (make fate-mp3) pass with this patch.
> 
> I tested on:
> 
> $ uname -a
> Linux UbuntuVM 2.6.28-19-generic #62-Ubuntu SMP Wed Jul 28 01:57:10 UTC 2010
> x86_64 GNU/Linux
> 
> and
> 
> $ ./ffmpeg -version 1>/dev/null
> FFmpeg version SVN-r25022, Copyright (c) 2000-2010 the FFmpeg developers
>   built on Sep  1 2010 16:42:11 with gcc 4.3.3
>   configuration: --prefix=/home/dbyron/src/ffmpeg_parent/install
> --incdir=/home/dbyron/src/ffmpeg_parent/install/include/ffmpeg
> --libdir=/home/dbyron/src/ffmpeg_parent/install/lib/ffmpeg
> --bindir=/home/dbyron/src/ffmpeg_parent/install/bin/ffmpeg
> --shlibdir=/home/dbyron/src/ffmpeg_parent/install/lib/ffmpeg
> --disable-shared --enable-static --samples=/home/dbyron/ffmpeg_samples
>   libavutil     50.24. 0 / 50.24. 0
>   libavcore      0. 6. 0 /  0. 6. 0
>   libavcodec    52.87. 0 / 52.87. 0
>   libavformat   52.78. 3 / 52.78. 3
>   libavdevice   52. 2. 1 / 52. 2. 1
>   libavfilter    1.38. 1 /  1.38. 1
>   libswscale     0.11. 0 /  0.11. 0
> 
> I can send an example file (~14k), or create an issue at roundup.ffmpeg.org
> but I figured I'd try this way first.
> 
> Please consider applying this to the public repo or let me know what changes
> would make that possible.
> 
> Thanks much.
> 
> -DB

>  Changelog           |    1 +
>  libavformat/id3v2.c |   26 ++++++++++++++++++++++++++
>  libavformat/id3v2.h |   10 ++++++++++
>  libavformat/mp3.c   |    7 +++++++
>  4 files changed, 44 insertions(+)
> 0ba10d560bfdf0f6974363661e7f85477b3dbefc  skip_extra_id3v2.patch
> Index: Changelog
> ===================================================================
> --- Changelog	(revision 25022)
> +++ Changelog	(working copy)
> @@ -4,6 +4,7 @@
>  
>  version <next>:
>  
> +- skip additional id3v2 tags at the beginning of mp3 files
>  - WebM support in Matroska de/muxer
>  - low overhead Ogg muxing
>  - MMS-TCP support
> Index: libavformat/mp3.c
> ===================================================================
> --- libavformat/mp3.c	(revision 25022)
> +++ libavformat/mp3.c	(working copy)
> @@ -157,6 +157,13 @@
>      av_set_pts_info(st, 64, 1, 14112000);
>  
>      ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC);
> +
> +    /* skip any extra id3v2 frames that may be present since
> +       they may contain data that looks like an mpeg frame */
> +    while (ff_id3v2_skip(s, ID3v2_DEFAULT_MAGIC))
> +        av_log(s,AV_LOG_VERBOSE,"skipping additional id3v2 tag in \"%s\"\n",
> +               s->filename);
> +

why doesnt ff_id3v2_read() does this ?

also we need a mp3 file that requires this change (not sure if you provided
one and i missed it)

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

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100907/ad8a064a/attachment.pgp>