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

Michael Niedermayer michaelni
Sun Sep 12 14:45:28 CEST 2010


On Thu, Sep 02, 2010 at 10:11:47PM +0200, Reimar D?ffinger wrote:
> On Thu, Sep 02, 2010 at 09:44:43PM +0200, Michael Niedermayer wrote:
> > On Thu, Sep 02, 2010 at 09:14:04PM +0200, Reimar D?ffinger wrote:
> > > On Thu, Sep 02, 2010 at 11:04:22AM +0200, Michael Niedermayer wrote:
> > > > On Tue, Aug 31, 2010 at 09:49:33PM +0200, Reimar D?ffinger wrote:
> > > > > most video codecs will figure out a width/height themselves or fail
> > > > > if they can't.
> > > > > So IMO it is better not to fail for invalid values in avcodec_open but
> > > > > instead just ignore the values by using the "default" of 0.
> > > > > Otherwise applications would have to manually check the values with
> > > > > av_check_image_size if they want the video to remain playable even
> > > > > if the container values were corrupted.
> > > > > Any objections?
> > > > 
> > > > yes, this change will leave invalid values in width/height and has a
> > > > good chance that this may be exploitable with some decoder
> > > 
> > > Yes, that was quite silly.
> > > Any other comment?
> > 
> > diff with -p next time please :)               
> 
> Any idea how to add this permanently to the SVN config?

no


> 
> > and avcodec_set_dimensions(0,0) could be used and maybe that can be simplified
> > with the surrounding code, would have to see it first to be sure if so ...
> 
> I forgot that avcodec_set_dimensions can be used.
> I think I'm too tired, but I think there might already be an issue as-is,
> if an application set
> codec_width = coded_height = width = 0,
> height = 0x7fffffff
> that would be passed on to the codec as-is.
> I made another try, but as said too tired.
> 
> 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 besides i would appreciate if you dont submit patches when
you are too tired. submitted patches should be reviewed by the submitter
while he is awake before submission


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

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100912/e6458efc/attachment.pgp>



More information about the ffmpeg-devel mailing list