[FFmpeg-devel] [PATCH] avoid crashes if input stream size changes without using -r

[Reimar Döffinger]

On Mon, Jul 06, 2009 at 01:23:15PM +0200, Michael Niedermayer wrote:
> On Mon, Jul 06, 2009 at 10:57:26AM +0100, M?ns Rullg?rd wrote:
> > Reimar D?ffinger <Reimar.Doeffinger at gmx.de> writes:
> > 
> > > On Mon, Jul 06, 2009 at 10:28:38AM +0100, M?ns Rullg?rd wrote:
> > >> Reimar D?ffinger <Reimar.Doeffinger at gmx.de> writes:
> > >> > On Mon, Jul 06, 2009 at 05:30:56AM +0200, Michael Niedermayer wrote:
> > >> >> On Sun, Jul 05, 2009 at 01:45:45PM +0200, Reimar D?ffinger wrote:
> > >> >> > I don't know why, but currently FFmpeg only checks for an input stream
> > >> >> > size change when it is using resizing.
> > >> >> 
> > >> >> id guess, the idea might have been to allow size changes on the encoder
> > >> >> side too when supported (trivial for intra only codecs)
> > >> >
> > >> > The code for that isn't there though (rawenc with the current code could
> > >> > already handle that fine), FFmpeg never gives the encoder the slightest
> > >> > hint that the resolution changed (avctx width/height stay at the
> > >> > original values)...
> > >> 
> > >> Then we should fix that.
> > >
> > > That would mean at the same time fixing all encoders to handle it or
> > > there will still be crashes.
> > 
> > CODEC_CAP_*
> 
> yes, a CODEC_CAP_SIZE_CHANGE or some better name could be added and the
> common code in lavc could maybe try to detect unexcpected size changes
> and thus prevent crashes&sec holes for codecs not supporting it

I guarantee you I will not validate any encoder if it manages to support
this correctly.
I think it is a pain and makes for brittle code when you could get
_exactly_ the same thing (if really wanted) by just closing and reopening
the encoder, at least for all video formats that FFmpeg currently supports.
Note that the same issue also exists for pix_fmt changes.
And as an example in FFmpeg, checking in common lavc code would change
the assumption that an error during decoding should always be fatal,
e.g. a single frame with the wrong size due to a few bits being
corrupted is not really a reason to give up completely.
Overall it seems to me that doing the check in lavc will not save
applications using lavc much code (if any)...

> also it could be used in decoders too catching changed w/h from demuxers

That seems like rather low priority to me, for any correct decoder there
shouldn't be any crashes since get_buffer gets them a buffer of the
right size (hopefully).
With encoding on the other had, the encoder has no way to find out the
size of the AVFrame it gets, thus it can't even check that everything is
right.
If we want to do checks for this kind of thing in lavc, extending
AVFrame by width/height/pix_fmt and checking them against avctx probably
would make sense (they can change without the application expecting it,
avctx width/height of the encoding context only change if the
application does it on purpose, so it would be protecting against API
misuse instead of just a forgotten check by the user).