[FFmpeg-devel] [PATCH] libavcodec/h261dec: Fix keyframe markup and frame skipping.

[Andrey Semashev]

On 2019-10-26 14:05, Andrey Semashev wrote:
> The decoder never marks pictures as I-frames, which results in no
> keyframe indication and incorrect frame skipping, in cases when
> keyframes should be decoded.
> 
> This commit works around this decoder limitation and marks I-frames
> and keyframes based on "freeze picture release" bit in h261 picture
> header. This reflects h261enc behavior.

So, is this patch acceptable? If yes, could someone merge it please?

> ---
>   libavcodec/h261.h    |  1 +
>   libavcodec/h261dec.c | 27 ++++++++++++++++++---------
>   2 files changed, 19 insertions(+), 9 deletions(-)
> 
> diff --git a/libavcodec/h261.h b/libavcodec/h261.h
> index 399a404b2b..6662d38d6d 100644
> --- a/libavcodec/h261.h
> +++ b/libavcodec/h261.h
> @@ -37,6 +37,7 @@
>   typedef struct H261Context {
>       MpegEncContext s;
>   
> +    int freeze_picture_release; // 1 if freeze picture release bit is set in the picture header
>       int current_mba;
>       int mba_diff;
>       int mtype;
> diff --git a/libavcodec/h261dec.c b/libavcodec/h261dec.c
> index 14a874c45d..3b1711a21d 100644
> --- a/libavcodec/h261dec.c
> +++ b/libavcodec/h261dec.c
> @@ -502,9 +502,9 @@ static int h261_decode_picture_header(H261Context *h)
>       s->avctx->framerate = (AVRational) { 30000, 1001 };
>   
>       /* PTYPE starts here */
> -    skip_bits1(&s->gb); /* split screen off */
> -    skip_bits1(&s->gb); /* camera  off */
> -    skip_bits1(&s->gb); /* freeze picture release off */
> +    skip_bits1(&s->gb); /* split screen indicator */
> +    skip_bits1(&s->gb); /* document camera indicator */
> +    h->freeze_picture_release = get_bits1(&s->gb); /* freeze picture release */
>   
>       format = get_bits1(&s->gb);
>   
> @@ -532,7 +532,8 @@ static int h261_decode_picture_header(H261Context *h)
>   
>       /* H.261 has no I-frames, but if we pass AV_PICTURE_TYPE_I for the first
>        * frame, the codec crashes if it does not contain all I-blocks
> -     * (e.g. when a packet is lost). */
> +     * (e.g. when a packet is lost). We will fix the picture type in the
> +     * output frame based on h->freeze_picture_release later. */
>       s->pict_type = AV_PICTURE_TYPE_P;
>   
>       h->gob_number = 0;
> @@ -590,6 +591,7 @@ static int h261_decode_frame(AVCodecContext *avctx, void *data,
>       H261Context *h     = avctx->priv_data;
>       MpegEncContext *s  = &h->s;
>       int ret;
> +    enum AVPictureType pict_type;
>       AVFrame *pict = data;
>   
>       ff_dlog(avctx, "*****frame %d size=%d\n", avctx->frame_number, buf_size);
> @@ -630,15 +632,17 @@ retry:
>           goto retry;
>       }
>   
> -    // for skipping the frame
> -    s->current_picture.f->pict_type = s->pict_type;
> -    s->current_picture.f->key_frame = s->pict_type == AV_PICTURE_TYPE_I;
> +    // for skipping the frame and keyframe markup
> +    pict_type = h->freeze_picture_release ? AV_PICTURE_TYPE_I : s->pict_type;
>   
> -    if ((avctx->skip_frame >= AVDISCARD_NONREF && s->pict_type == AV_PICTURE_TYPE_B) ||
> -        (avctx->skip_frame >= AVDISCARD_NONKEY && s->pict_type != AV_PICTURE_TYPE_I) ||
> +    if ((avctx->skip_frame >= AVDISCARD_NONREF && pict_type == AV_PICTURE_TYPE_B) ||
> +        (avctx->skip_frame >= AVDISCARD_NONKEY && pict_type != AV_PICTURE_TYPE_I) ||
>            avctx->skip_frame >= AVDISCARD_ALL)
>           return get_consumed_bytes(s, buf_size);
>   
> +    s->current_picture.f->pict_type = s->pict_type;
> +    s->current_picture.f->key_frame = s->pict_type == AV_PICTURE_TYPE_I;
> +
>       if (ff_mpv_frame_start(s, avctx) < 0)
>           return -1;
>   
> @@ -660,6 +664,11 @@ retry:
>   
>       if ((ret = av_frame_ref(pict, s->current_picture_ptr->f)) < 0)
>           return ret;
> +
> +    // fix picture type and correctly mark keyframes
> +    pict->pict_type = pict_type;
> +    pict->key_frame = pict_type == AV_PICTURE_TYPE_I;
> +
>       ff_print_debug_info(s, s->current_picture_ptr, pict);
>   
>       *got_frame = 1;
>