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

[Andrey Semashev]

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.
---
 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;
-- 
2.20.1