[FFmpeg-devel] [PATCH] Add IFF metadata handling

[Michael Niedermayer]

On Mon, May 03, 2010 at 09:20:32PM +0200, Sebastian Vater wrote:
> Ronald S. Bultje a ?crit :
> > Hi,
> >
> > On Mon, May 3, 2010 at 2:48 PM, Sebastian Vater
> > <cdgs.basty at googlemail.com> wrote:
> >   
> >> Reason is that I figured out that the original code didn't check even if
> >> get_buffer succeeds...
> >> I have changed the metadata handling in a way it returns an appreciate
> >> I/O error instead.
> >>     
> > [..]
> >   
> >> +            if ((res = get_metadata(s, "comment", data_size)) < 0) {
> >> +                av_log(s, AV_LOG_ERROR, "iff: cannot allocate metadata for comments!");
> >> +                return res;
> >> +            }
> >> +            break;
> >> +
> >> +        case ID_AUTH:
> >> +            if ((res = get_metadata(s, "artist", data_size)) < 0) {
> >> +                av_log(s, AV_LOG_ERROR, "iff: cannot allocate metadata for author!");
> >> +                return res;
> >> +            }
> >>     
> >
> > I don't think the user cares which metadata failed, and that means
> > you're duplicating lots of code here and below.
> >   
> 
> I'm not sure, I thought the same way before, but then I realized that
> somebody might be running this in a huge batch task (converting images,
> etc.) and doesn't want such errors silently be skipped.
> 
> > case ID_AUTH: m = "artist"; break;
> > case ID_COMMENT: m = "comment"; break;
> > if (get_metadata()) {
> >   log();
> >   return;
> > }
> >   
> 
> This, is a pretty good idea, indeed and I supply a new patch for this.
> 
> -- 
> 
> Best regards,
>                    :-) Basty/CDGS (-:
> 

>  iff.c |   48 +++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 41 insertions(+), 7 deletions(-)
> 47b4dc99704e664139fa93ec6c6e5fabf94716ef  iff-metadata.patch
> diff --git a/libavformat/iff.c b/libavformat/iff.c
> index db74b8d..896c25b 100644
> --- a/libavformat/iff.c
> +++ b/libavformat/iff.c
> @@ -92,6 +92,25 @@ static void interleave_stereo(const uint8_t *src, uint8_t *dest, int size)
>      }
>  }
>  
> +/* Metadata string read */
> +static int get_metadata(AVFormatContext *s,
> +                        const char *const tag,
> +                        const unsigned data_size)
> +{
> +    uint8_t *buf = av_malloc(data_size + 1);

integer overflow


> +
> +    if (!buf)
> +        return AVERROR(ENOMEM);
> +
> +    if (get_buffer(s->pb, buf, data_size) < 0) {

segfault

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

In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA
-------------- 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/20100505/c2203dfb/attachment.pgp>