[FFmpeg-devel] [RFC] ignore invalid user-supplied width/height

Reimar Döffinger Reimar.Doeffinger
Wed Sep 15 06:49:59 CEST 2010


On Mon, Sep 13, 2010 at 07:18:45PM +0200, Michael Niedermayer wrote:
> On Sun, Sep 12, 2010 at 04:39:20PM +0200, Reimar D?ffinger wrote:
> > On Sun, Sep 12, 2010 at 02:45:28PM +0200, Michael Niedermayer wrote:
> > > > Index: libavcodec/utils.c
> > > > ===================================================================
> > > > --- libavcodec/utils.c  (revision 25017)
> > > > +++ libavcodec/utils.c  (working copy)
> > > > @@ -485,10 +485,15 @@ int attribute_align_arg avcodec_open(AVCodecContex
> > > >      else if(avctx->width && avctx->height)
> > > >          avcodec_set_dimensions(avctx, avctx->width, avctx->height);
> > > >  
> > > > +    if ((avctx->coded_width || avctx->coded_height || avctx->width || avctx->height)
> > > > +        && (  av_check_image_size(avctx->coded_width, avctx->coded_height, 0, avctx) < 0
> > > > +           || av_check_image_size(avctx->width,       avctx->height,       0, avctx) < 0) {
> > > > +        av_log(avctx, AV_LOG_WARNING, "ignoring invalid width/height values\n");
> > > > +        avcodec_set_dimensions(avctx, 0, 0);
> > > > +    }
> > > 
> > > this function is also used for encoders i think and i dont think this is
> > > correct for them
> > 
> > Well,
> > 1) 0,0 has always been allowed through, so at least it does not make anything worse
> 
> encoding video with 0,0 is surely not ok

As said, there is a check in the encode function, just init is not "protected".

> > 2) an AVCodec can have both encode and decode, so we can't really do a 100% correct
> >    check here. We do already have a check in avcodec_encode_video anyway (which admittedly
> >    is too late for any init code).
> > 
> > I am fairly confident that this proposed patch fixes two issues (failures due to corrupted
> > values from the container and bogus width/height values being allow through as long
> > as one of them and the coded_width/coded_height values are 0) and does not introduce
> > any new ones.
> 
> ja ja, commit it if you in awake state think its ok

Applied.

> > And sorry for munging this in this thread, but why is lowres only checked against
> > max_lowres if avctx->codec->init is non-NULL? Doesn't make sense to me.
> 
> neither to me

Carl took care of that.



More information about the ffmpeg-devel mailing list