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

[Reimar Döffinger]

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