[FFmpeg-devel] [PATCH] lavf/aiffdec: handle files with ID3 data larger than ID3 chunk size

[Matthieu Bouron]

On Wed, Jan 16, 2013 at 01:45:05PM +0100, Matthieu Bouron wrote:
> On Wed, Jan 16, 2013 at 12:01:38PM +0100, Matthieu Bouron wrote:
> > On Wed, Jan 16, 2013 at 10:37:03AM +0000, Paul B Mahol wrote:
> > > On 1/16/13, Matthieu Bouron <matthieu.bouron at gmail.com> wrote:
> > > > On Wed, Jan 16, 2013 at 02:37:19AM +0100, Michael Niedermayer wrote:
> > > >> On Wed, Jan 16, 2013 at 01:18:09AM +0100, Matthieu Bouron wrote:
> > > >> > On Wed, Jan 16, 2013 at 12:34:31AM +0100, Michael Niedermayer wrote:
> > > >> > > On Tue, Jan 15, 2013 at 10:49:01PM +0100, Matthieu Bouron wrote:
> > > >> > > > ---
> > > >> > > >  libavformat/aiffdec.c | 7 +++++++
> > > >> > > >  1 file changed, 7 insertions(+)
> > > >> > > >
> > > >> > > > diff --git a/libavformat/aiffdec.c b/libavformat/aiffdec.c
> > > >> > > > index 8d466fa..b6a7946 100644
> > > >> > > > --- a/libavformat/aiffdec.c
> > > >> > > > +++ b/libavformat/aiffdec.c
> > > >> > > > @@ -193,6 +193,7 @@ static int aiff_read_header(AVFormatContext *s)
> > > >> > > >  {
> > > >> > > >      int ret, size, filesize;
> > > >> > > >      int64_t offset = 0;
> > > >> > > > +    int64_t tag_size, tag_offset;
> > > >> > > >      uint32_t tag;
> > > >> > > >      unsigned version = AIFF_C_VERSION1;
> > > >> > > >      AVIOContext *pb = s->pb;
> > > >> > > > @@ -236,6 +237,7 @@ static int aiff_read_header(AVFormatContext *s)
> > > >> > > >                  goto got_sound;
> > > >> > > >              break;
> > > >> > > >          case MKTAG('I', 'D', '3', ' '):
> > > >> > > > +            tag_offset = avio_tell(pb);
> > > >> > > >              ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC,
> > > >> > > > &id3v2_extra_meta);
> > > >> > > >              if (id3v2_extra_meta)
> > > >> > > >                  if ((ret = ff_id3v2_parse_apic(s,
> > > >> > > > &id3v2_extra_meta)) < 0) {
> > > >> > > > @@ -243,6 +245,11 @@ static int aiff_read_header(AVFormatContext
> > > >> > > > *s)
> > > >> > > >                      return ret;
> > > >> > > >                  }
> > > >> > > >              ff_id3v2_free_extra_meta(&id3v2_extra_meta);
> > > >> > > > +            tag_size = avio_tell(pb) - tag_offset;
> > > >> > > > +            if (tag_size > size) {
> > > >> > > > +                av_log(s, AV_LOG_WARNING, "ID3 data is larger than
> > > >> > > > ID3 chunk size\n");
> > > >> > > > +                avio_seek(pb, tag_size - size, SEEK_SET);
> > > >> > > > +            }
> > > >> > >
> > > >> > > to which point do you try to seek here ?
> > > >> > > this looks like you try to do a absolute seek with a relative value
> > > >> > >
> > > >> >
> > > >> > Sorry for the messy patch :(, I didn't intend to seek anywhere but
> > > >> > just to decrease the filesize variable according to the difference
> > > >> > between tag_size and size.
> > > >> > New patch attached.
> > > >>
> > > >> so when the sizes mismatch a correct tag is at position after the
> > > >> larger size ?
> > > >
> > > > No, the samples i have have the ID3 chunk at the end of the file and the
> > > > ID3
> > > > data is larger than the declared ID3 chunk size by one byte.
> > > 
> > > aligned to 2 byte boundary?
> > 
> > It seems to be. For example:
> > ID3 data offset = 80988992
> > ID3 data size = 151960
> > Declared ID3 chunk size = 151959
> > File offset after reading the ID3 data: 81140952 (which corresponds to the
> > filesize)
> 
> The data itself is aligned however the chunk size is incorrect (or maybe i am
> missing something).

The problem was caused by ff_id3v2_read that consummed one extra byte
trying to parse another id3v2 header at the end of the file.

This patch is not relevant anymore and the problem should be fixed with the
lavf/id3v2 patch.

Regards,
Matthieu