[FFmpeg-devel] [PATCH 2/2] utils: check that parameters havnt changed in reget_buffer()

Michael Niedermayer michaelni at gmx.at
Thu Jan 12 01:23:23 CET 2012


On Thu, Jan 12, 2012 at 12:46:17AM +0100, Reimar Döffinger wrote:
> On Thu, Jan 12, 2012 at 12:02:47AM +0100, Michael Niedermayer wrote:
> > Fixes Ticket902
> > 
> > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > ---
> >  libavcodec/utils.c |    7 +++++++
> >  1 files changed, 7 insertions(+), 0 deletions(-)
> > 
> > diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> > index b79e23e..2b091e1 100644
> > --- a/libavcodec/utils.c
> > +++ b/libavcodec/utils.c
> > @@ -564,6 +564,13 @@ int avcodec_default_reget_buffer(AVCodecContext *s, AVFrame *pic){
> >          return s->get_buffer(s, pic);
> >      }
> >  
> > +    if (pic->width != s->width || pic->height != s->height || pic->format != s->pix_fmt) {
> > +        av_log(s, AV_LOG_WARNING, "Width/height/fmt changing with reget buffer");
> > +        s->release_buffer(s, pic);
> > +        pic->buffer_hints |= FF_BUFFER_HINTS_READABLE;
> > +        return s->get_buffer(s,pic);
> > +    }
> 
> Why are you duplicating this code instead of just moving this up a bit?

i missed that it could be simplified that way
ill do that in a moment


> Ok, you'd have to duplicate the pic->data[0] check but that still seems
> less.
> I am also not sure this is a really correct fix, IMO a decoder really
> should free all (reference) frames on a resolution change instead
> of leaving a user-overrideable function to deal with it.

we could replace all calls of the function pointer by calls to
ff_reget_buffer() and do the check in there.
But i dont volunteer to do that change


> This also reminds me I had some resolution change annoyance in MPlayer,
> too, when decoding a sequence like
> PIPBB
> with a resolution change at the "I" at least with multithreaded decoding
> means that the decoder will return a frame with the old resolution
> after having requested a frame of the new resolution (which in itself
> is a pain to support) and while also AVCodecContext already contains
> the new resolution.
> I think the decoder should rather flush its reorder buffer before
> continuing with decoding the next part, though I admit that
> a) I think quite a few applications including FFmpeg wouldn't handle
> that quite right
> b) Particularly with frame multithreading it would make the transition
> less smooth - on the other hand with the current behaviour and direct
> rendering the only reasonable option I saw was just to drop all frames
> in-between which is no good either.

IIRC some specifications allow resolution changes mid stream and allow
using frames of differing resolution as reference frames.
If we plan tu support such stuff then flushing would not be an option

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

What does censorship reveal? It reveals fear. -- Julian Assange
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120112/2a3e75ca/attachment.asc>


More information about the ffmpeg-devel mailing list