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

[Reimar Döffinger]

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
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.

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.