[FFmpeg-devel] [PATCH] common ID3v2 support for all formats

Reimar Döffinger Reimar.Doeffinger
Sat Oct 2 12:18:30 CEST 2010


On Fri, Oct 01, 2010 at 11:27:36PM +0200, Michael Niedermayer wrote:
> On Fri, Oct 01, 2010 at 08:21:07PM +0200, Reimar D?ffinger wrote:
> > Hello,
> > considering issue 2258 I think it is a valid conclusion that people
> > will prepend a ID3v2 header to anything that can't run aways fast enough
> > (and then give it a .mp3 extension...).
> > So I propose attached patch that moves that parsing to common code.
> > Note that is assume that due to buffering the ID3 parsing attempt
> > will not cause any issues even on non-seekable input.
> > Note that I have no idea WTF the mpc demuxer code is doing there,
> > my conclusion was that it should be safe to just throw it all away,
> > but I'm not 100% sure I don't miss some subtlety.
> > Mostly unrelated, but one issue with this whole ID3 handling is
> > that AFAICT it will use the file-specific metadata conversion
> > instead of using always the ID3 one.
> 
> >  aacdec.c  |    5 -----
> >  flacdec.c |   14 --------------
> >  mp3.c     |    4 ----
> >  mpc.c     |   34 ++++------------------------------
> >  tta.c     |    8 --------
> >  utils.c   |   16 ++++++++++++++--
> >  6 files changed, 18 insertions(+), 63 deletions(-)
> > 5cc1b70fabc75686a99488dd8c63693412e67935  id3v2.diff
> 
> iam ok with this if its tested and the mpc maintainer confirms the change is
> ok

I had to change the code to use this:
> +    // e.g. AVFMT_NOFILE formats will not have a ByteIOContext
> +    if (ic->pb)
> +        ff_id3v2_read(ic, ID3v2_DEFAULT_MAGIC);

Is this still ok?
There are several other ways, e.g. checking for AVFMT_NOFILE, only
calling ff_id3v2_read if there's a read_header for the format
or making ff_id3v2_read check that pb is not NULL.



More information about the ffmpeg-devel mailing list