codecs: h264decoder: Store reference picture type using enum value
authorSeungha Yang <seungha@centricular.com>
Thu, 5 Nov 2020 16:45:36 +0000 (01:45 +0900)
committerGStreamer Merge Bot <gitlab-merge-bot@gstreamer-foundation.org>
Fri, 13 Nov 2020 15:25:42 +0000 (15:25 +0000)
Managing reference picture type by using two variables
(ref and long_term) seems to be redundant and that can be
represented by using a single enum value.

This is to sync this implementation with gstreamer-vaapi so that
make comparison between this and gstreamer-vaapi easier and also
in order to minimize the change required for subclass to be able
to support interlaced.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/-/merge_requests/1534>

gst-libs/gst/codecs/gsth264decoder.c
gst-libs/gst/codecs/gsth264picture.c
gst-libs/gst/codecs/gsth264picture.h
sys/d3d11/gstd3d11h264dec.c
sys/nvcodec/gstnvh264dec.c
sys/v4l2codecs/gstv4l2codech264dec.c
sys/va/gstvah264dec.c

index 3a9588d..4ba8ace 100644 (file)
@@ -581,10 +581,10 @@ gst_h264_decoder_update_pic_nums (GstH264Decoder * self, gint frame_num)
       continue;
     }
 
-    if (!picture->ref)
+    if (!GST_H264_PICTURE_IS_REF (picture))
       continue;
 
-    if (picture->long_term) {
+    if (GST_H264_PICTURE_IS_LONG_TERM_REF (picture)) {
       picture->long_term_pic_num = picture->long_term_frame_idx;
     } else {
       if (picture->frame_num > frame_num)
@@ -1034,7 +1034,8 @@ gst_h264_decoder_fill_picture_from_slice (GstH264Decoder * self,
   }
 
   picture->nal_ref_idc = slice->nalu.ref_idc;
-  picture->ref = slice->nalu.ref_idc != 0;
+  if (slice->nalu.ref_idc != 0)
+    gst_h264_picture_set_reference (picture, GST_H264_PICTURE_REF_SHORT_TERM);
 
   /* This assumes non-interlaced stream */
   picture->frame_num = picture->pic_num = slice_hdr->frame_num;
@@ -1457,7 +1458,7 @@ gst_h264_decoder_sliding_window_picture_marking (GstH264Decoder * self)
         "Unmark reference flag of picture %p (frame_num %d, poc %d)",
         to_unmark, to_unmark->frame_num, to_unmark->pic_order_cnt);
 
-    to_unmark->ref = FALSE;
+    gst_h264_picture_set_reference (to_unmark, GST_H264_PICTURE_REF_NONE);
     gst_h264_picture_unref (to_unmark);
 
     num_ref_pics--;
@@ -1482,11 +1483,11 @@ gst_h264_decoder_reference_picture_marking (GstH264Decoder * self,
     gst_h264_dpb_mark_all_non_ref (priv->dpb);
 
     if (picture->dec_ref_pic_marking.long_term_reference_flag) {
-      picture->long_term = TRUE;
+      gst_h264_picture_set_reference (picture, GST_H264_PICTURE_REF_LONG_TERM);
       picture->long_term_frame_idx = 0;
       priv->max_long_term_frame_idx = 0;
     } else {
-      picture->long_term = FALSE;
+      gst_h264_picture_set_reference (picture, GST_H264_PICTURE_REF_SHORT_TERM);
       priv->max_long_term_frame_idx = -1;
     }
 
@@ -1817,7 +1818,7 @@ gst_h264_decoder_init_gap_picture (GstH264Decoder * self,
   picture->nal_ref_idc = 1;
   picture->frame_num = picture->pic_num = frame_num;
   picture->dec_ref_pic_marking.adaptive_ref_pic_marking_mode_flag = FALSE;
-  picture->ref = TRUE;
+  picture->ref = GST_H264_PICTURE_REF_SHORT_TERM;
   picture->dec_ref_pic_marking.long_term_reference_flag = FALSE;
   picture->field = GST_H264_PICTURE_FIELD_FRAME;
 
@@ -1910,7 +1911,7 @@ construct_ref_pic_lists_p (GstH264Decoder * self)
     for (pos = 0; pos < priv->ref_pic_list_p0->len; pos++) {
       GstH264Picture *ref =
           g_array_index (priv->ref_pic_list_p0, GstH264Picture *, pos);
-      if (!ref->long_term)
+      if (!GST_H264_PICTURE_IS_LONG_TERM_REF (ref))
         g_string_append_printf (str, "|%i", ref->pic_num);
       else
         g_string_append_printf (str, "|%is", ref->pic_num);
@@ -1966,7 +1967,7 @@ print_ref_pic_list_b (GstH264Decoder * self, GArray * ref_list_b, gint index)
   for (i = 0; i < ref_list_b->len; i++) {
     GstH264Picture *ref = g_array_index (ref_list_b, GstH264Picture *, i);
 
-    if (!ref->long_term)
+    if (!GST_H264_PICTURE_IS_LONG_TERM_REF (ref))
       g_string_append_printf (str, "|%i", ref->pic_order_cnt);
     else
       g_string_append_printf (str, "|%il", ref->long_term_pic_num);
@@ -2078,7 +2079,7 @@ gst_h264_decoder_clear_ref_pic_lists (GstH264Decoder * self)
 static gint
 long_term_pic_num_f (GstH264Decoder * self, const GstH264Picture * picture)
 {
-  if (picture->ref && picture->long_term)
+  if (GST_H264_PICTURE_IS_LONG_TERM_REF (picture))
     return picture->long_term_pic_num;
   return 2 * (self->priv->max_long_term_frame_idx + 1);
 }
@@ -2086,7 +2087,7 @@ long_term_pic_num_f (GstH264Decoder * self, const GstH264Picture * picture)
 static gint
 pic_num_f (GstH264Decoder * self, const GstH264Picture * picture)
 {
-  if (!picture->long_term)
+  if (!GST_H264_PICTURE_IS_LONG_TERM_REF (picture))
     return picture->pic_num;
   return self->priv->max_pic_num;
 }
index 1112b1e..346582c 100644 (file)
@@ -248,7 +248,7 @@ gst_h264_dpb_delete_unused (GstH264Dpb * dpb)
 
     /* NOTE: don't use g_array_remove_index_fast here since the last picture
      * need to be referenced for bumping decision */
-    if (!picture->needed_for_output && !picture->ref) {
+    if (!picture->needed_for_output && !GST_H264_PICTURE_IS_REF (picture)) {
       GST_TRACE ("remove picture %p (frame num %d) from dpb",
           picture, picture->frame_num);
       g_array_remove_index (dpb->pic_list, i);
@@ -275,7 +275,7 @@ gst_h264_dpb_num_ref_pictures (GstH264Dpb * dpb)
     GstH264Picture *picture =
         g_array_index (dpb->pic_list, GstH264Picture *, i);
 
-    if (picture->ref)
+    if (GST_H264_PICTURE_IS_REF (picture))
       ret++;
   }
 
@@ -299,7 +299,7 @@ gst_h264_dpb_mark_all_non_ref (GstH264Dpb * dpb)
     GstH264Picture *picture =
         g_array_index (dpb->pic_list, GstH264Picture *, i);
 
-    picture->ref = FALSE;
+    gst_h264_picture_set_reference (picture, GST_H264_PICTURE_REF_NONE);
   }
 }
 
@@ -323,7 +323,8 @@ gst_h264_dpb_get_short_ref_by_pic_num (GstH264Dpb * dpb, gint pic_num)
     GstH264Picture *picture =
         g_array_index (dpb->pic_list, GstH264Picture *, i);
 
-    if (picture->ref && !picture->long_term && picture->pic_num == pic_num)
+    if (GST_H264_PICTURE_IS_SHORT_TERM_REF (picture)
+        && picture->pic_num == pic_num)
       return picture;
   }
 
@@ -355,7 +356,7 @@ gst_h264_dpb_get_long_ref_by_long_term_pic_num (GstH264Dpb * dpb,
     GstH264Picture *picture =
         g_array_index (dpb->pic_list, GstH264Picture *, i);
 
-    if (picture->ref && picture->long_term &&
+    if (GST_H264_PICTURE_IS_LONG_TERM_REF (picture) &&
         picture->long_term_pic_num == long_term_pic_num)
       return picture;
   }
@@ -385,7 +386,7 @@ gst_h264_dpb_get_lowest_frame_num_short_ref (GstH264Dpb * dpb)
     GstH264Picture *picture =
         g_array_index (dpb->pic_list, GstH264Picture *, i);
 
-    if (picture->ref && !picture->long_term &&
+    if (GST_H264_PICTURE_IS_SHORT_TERM_REF (picture) &&
         (!ret || picture->frame_num_wrap < ret->frame_num_wrap))
       ret = picture;
   }
@@ -417,7 +418,7 @@ gst_h264_dpb_get_pictures_short_term_ref (GstH264Dpb * dpb, GArray * out)
     GstH264Picture *picture =
         g_array_index (dpb->pic_list, GstH264Picture *, i);
 
-    if (picture->ref && !picture->long_term) {
+    if (GST_H264_PICTURE_IS_SHORT_TERM_REF (picture)) {
       gst_h264_picture_ref (picture);
       g_array_append_val (out, picture);
     }
@@ -445,7 +446,7 @@ gst_h264_dpb_get_pictures_long_term_ref (GstH264Dpb * dpb, GArray * out)
     GstH264Picture *picture =
         g_array_index (dpb->pic_list, GstH264Picture *, i);
 
-    if (picture->ref && picture->long_term) {
+    if (GST_H264_PICTURE_IS_LONG_TERM_REF (picture)) {
       gst_h264_picture_ref (picture);
       g_array_append_val (out, picture);
     }
@@ -675,7 +676,7 @@ gst_h264_dpb_bump (GstH264Dpb * dpb, gboolean drain)
 
   /* NOTE: don't use g_array_remove_index_fast here since the last picture
    * need to be referenced for bumping decision */
-  if (!picture->ref || drain)
+  if (!GST_H264_PICTURE_IS_REF (picture) || drain)
     g_array_remove_index (dpb->pic_list, index);
 
   dpb->last_output_poc = picture->pic_order_cnt;
@@ -729,7 +730,7 @@ gst_h264_dpb_perform_memory_management_control_operation (GstH264Dpb * dpb,
       pic_num_x = get_picNumX (picture, ref_pic_marking);
       other = gst_h264_dpb_get_short_ref_by_pic_num (dpb, pic_num_x);
       if (other) {
-        other->ref = FALSE;
+        gst_h264_picture_set_reference (other, GST_H264_PICTURE_REF_NONE);
         GST_TRACE ("MMCO-1: unmark short-term ref picture %p, (poc %d)",
             other, other->pic_order_cnt);
       } else {
@@ -743,7 +744,7 @@ gst_h264_dpb_perform_memory_management_control_operation (GstH264Dpb * dpb,
       other = gst_h264_dpb_get_long_ref_by_long_term_pic_num (dpb,
           ref_pic_marking->long_term_pic_num);
       if (other) {
-        other->ref = FALSE;
+        gst_h264_picture_set_reference (other, GST_H264_PICTURE_REF_NONE);
         GST_TRACE ("MMCO-2: unmark long-term ref picture %p, (poc %d)",
             other, other->pic_order_cnt);
       } else {
@@ -760,10 +761,10 @@ gst_h264_dpb_perform_memory_management_control_operation (GstH264Dpb * dpb,
       for (i = 0; i < dpb->pic_list->len; i++) {
         other = g_array_index (dpb->pic_list, GstH264Picture *, i);
 
-        if (other->ref && other->long_term && other->long_term_frame_idx ==
+        if (GST_H264_PICTURE_IS_LONG_TERM_REF (other)
+            && other->long_term_frame_idx ==
             ref_pic_marking->long_term_frame_idx) {
-          other->ref = FALSE;
-          other->long_term = FALSE;
+          gst_h264_picture_set_reference (other, GST_H264_PICTURE_REF_NONE);
           GST_TRACE ("MMCO-3: unmark old long-term ref pic %p (poc %d)",
               other, other->pic_order_cnt);
           break;
@@ -773,7 +774,7 @@ gst_h264_dpb_perform_memory_management_control_operation (GstH264Dpb * dpb,
       pic_num_x = get_picNumX (picture, ref_pic_marking);
       other = gst_h264_dpb_get_short_ref_by_pic_num (dpb, pic_num_x);
       if (other) {
-        other->long_term = TRUE;
+        gst_h264_picture_set_reference (other, GST_H264_PICTURE_REF_LONG_TERM);
         other->long_term_frame_idx = ref_pic_marking->long_term_frame_idx;
         GST_TRACE ("MMCO-3: mark long-term ref pic %p, index %d, (poc %d)",
             other, other->long_term_frame_idx, other->pic_order_cnt);
@@ -794,10 +795,9 @@ gst_h264_dpb_perform_memory_management_control_operation (GstH264Dpb * dpb,
       for (i = 0; i < dpb->pic_list->len; i++) {
         other = g_array_index (dpb->pic_list, GstH264Picture *, i);
 
-        if (other->ref && other->long_term &&
+        if (GST_H264_PICTURE_IS_LONG_TERM_REF (other) &&
             other->long_term_frame_idx > max_long_term_frame_idx) {
-          other->ref = FALSE;
-          other->long_term = FALSE;
+          gst_h264_picture_set_reference (other, GST_H264_PICTURE_REF_NONE);
           GST_TRACE ("MMCO-4: unmark long-term ref pic %p, index %d, (poc %d)",
               other, other->long_term_frame_idx, other->pic_order_cnt);
         }
@@ -807,8 +807,7 @@ gst_h264_dpb_perform_memory_management_control_operation (GstH264Dpb * dpb,
       /* 8.2.5.4.5 Unmark all reference pictures */
       for (i = 0; i < dpb->pic_list->len; i++) {
         other = g_array_index (dpb->pic_list, GstH264Picture *, i);
-        other->ref = FALSE;
-        other->long_term = FALSE;
+        gst_h264_picture_set_reference (other, GST_H264_PICTURE_REF_NONE);
       }
       picture->mem_mgmt_5 = TRUE;
       picture->frame_num = 0;
@@ -822,18 +821,17 @@ gst_h264_dpb_perform_memory_management_control_operation (GstH264Dpb * dpb,
       for (i = 0; i < dpb->pic_list->len; i++) {
         other = g_array_index (dpb->pic_list, GstH264Picture *, i);
 
-        if (other->ref && other->long_term && other->long_term_frame_idx ==
+        if (GST_H264_PICTURE_IS_LONG_TERM_REF (other) &&
+            other->long_term_frame_idx ==
             ref_pic_marking->long_term_frame_idx) {
           GST_TRACE ("MMCO-6: unmark old long-term ref pic %p (poc %d)",
               other, other->pic_order_cnt);
-          other->ref = FALSE;
-          other->long_term = FALSE;
+          gst_h264_picture_set_reference (other, GST_H264_PICTURE_REF_NONE);
           break;
         }
       }
 
-      picture->ref = TRUE;
-      picture->long_term = TRUE;
+      gst_h264_picture_set_reference (picture, GST_H264_PICTURE_REF_LONG_TERM);
       picture->long_term_frame_idx = ref_pic_marking->long_term_frame_idx;
       break;
     default:
@@ -843,3 +841,21 @@ gst_h264_dpb_perform_memory_management_control_operation (GstH264Dpb * dpb,
 
   return TRUE;
 }
+
+/**
+ * gst_h264_picture_set_reference:
+ * @picture: a #GstH264Picture
+ * @reference: a GstH264PictureReference
+ *
+ * Update reference picture type of @picture with @reference
+ *
+ * Since: 1.20
+ */
+void
+gst_h264_picture_set_reference (GstH264Picture * picture,
+    GstH264PictureReference reference)
+{
+  g_return_if_fail (picture != NULL);
+
+  picture->ref = reference;
+}
index 727c46d..f439e41 100644 (file)
@@ -37,6 +37,39 @@ typedef struct _GstH264Picture GstH264Picture;
 /* As specified in A.3.1 h) and A.3.2 f) */
 #define GST_H264_DPB_MAX_SIZE 16
 
+/**
+ * GST_H264_PICTURE_IS_REF:
+ * @picture: a #GstH264Picture
+ *
+ * Check whether @picture is used for short-term or long-term reference
+ *
+ * Since: 1.20
+ */
+#define GST_H264_PICTURE_IS_REF(picture) \
+    ((picture)->ref != GST_H264_PICTURE_REF_NONE)
+
+/**
+ * GST_H264_PICTURE_IS_SHORT_TERM_REF:
+ * @picture: a #GstH264Picture
+ *
+ * Check whether @picture is used for short-term reference
+ *
+ * Since: 1.20
+ */
+#define GST_H264_PICTURE_IS_SHORT_TERM_REF(picture) \
+    ((picture)->ref == GST_H264_PICTURE_REF_SHORT_TERM)
+
+/**
+ * GST_H264_PICTURE_IS_LONG_TERM_REF:
+ * @picture: a #GstH264Picture
+ *
+ * Check whether @picture is used for long-term reference
+ *
+ * Since: 1.20
+ */
+#define GST_H264_PICTURE_IS_LONG_TERM_REF(picture) \
+    ((picture)->ref == GST_H264_PICTURE_REF_LONG_TERM)
+
 struct _GstH264Slice
 {
   GstH264SliceHdr header;
@@ -52,6 +85,21 @@ typedef enum
   GST_H264_PICTURE_FIELD_BOTTOM_FIELD,
 } GstH264PictureField;
 
+/**
+ * GstH264PictureReference:
+ * @GST_H264_PICTURE_REF_NONE: Not used for reference picture
+ * @GST_H264_PICTURE_REF_SHORT_TERM: Used for short-term reference picture
+ * @GST_H264_PICTURE_REF_LONG_TERM: Used for long-term reference picture
+ *
+ * Since: 1.20
+ */
+typedef enum
+{
+  GST_H264_PICTURE_REF_NONE = 0,
+  GST_H264_PICTURE_REF_SHORT_TERM,
+  GST_H264_PICTURE_REF_LONG_TERM,
+} GstH264PictureReference;
+
 struct _GstH264Picture
 {
   /*< private >*/
@@ -83,8 +131,7 @@ struct _GstH264Picture
   gint nal_ref_idc;
   gboolean idr;
   gint idr_pic_id;
-  gboolean ref;
-  gboolean long_term;
+  GstH264PictureReference ref;
   gboolean needed_for_output;
   gboolean mem_mgmt_5;
 
@@ -218,6 +265,10 @@ gboolean         gst_h264_dpb_perform_memory_management_control_operation (GstH2
                                                                            GstH264RefPicMarking *ref_pic_marking,
                                                                            GstH264Picture * picture);
 
+/* Internal methods */
+void  gst_h264_picture_set_reference (GstH264Picture * picture,
+                                      GstH264PictureReference reference);
+
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(GstH264Picture, gst_h264_picture_unref)
 
 G_END_DECLS
index 4cd0a87..ded00ef 100644 (file)
@@ -565,7 +565,7 @@ gst_d3d11_h264_dec_start_picture (GstH264Decoder * decoder,
     ID3D11VideoDecoderOutputView *other_view;
     gint id = 0xff;
 
-    if (!other->ref)
+    if (!GST_H264_PICTURE_IS_REF (other))
       continue;
 
     other_view = gst_d3d11_h264_dec_get_output_view_from_picture (self, other);
@@ -574,7 +574,8 @@ gst_d3d11_h264_dec_start_picture (GstH264Decoder * decoder,
       id = gst_d3d11_decoder_get_output_view_index (other_view);
 
     self->ref_frame_list[i].Index7Bits = id;
-    self->ref_frame_list[i].AssociatedFlag = other->long_term;
+    self->ref_frame_list[i].AssociatedFlag =
+        GST_H264_PICTURE_IS_LONG_TERM_REF (other);
     self->field_order_cnt_list[i][0] = other->top_field_order_cnt;
     self->field_order_cnt_list[i][1] = other->bottom_field_order_cnt;
     self->frame_num_list[i] = self->ref_frame_list[i].AssociatedFlag
@@ -587,7 +588,7 @@ gst_d3d11_h264_dec_start_picture (GstH264Decoder * decoder,
 
   pic_params.CurrPic.Index7Bits =
       gst_d3d11_decoder_get_output_view_index (view);
-  pic_params.RefPicFlag = picture->ref;
+  pic_params.RefPicFlag = GST_H264_PICTURE_IS_REF (picture);
   pic_params.frame_num = picture->frame_num;
 
   if (pic_params.field_pic_flag && pic_params.CurrPic.AssociatedFlag) {
index c0264a4..95501f8 100644 (file)
@@ -643,13 +643,13 @@ gst_nv_h264_dec_start_picture (GstH264Decoder * decoder,
   /* nBitstreamDataLen, pBitstreamData, nNumSlices and pSliceDataOffsets
    * will be set later */
 
-  params->ref_pic_flag = picture->ref;
+  params->ref_pic_flag = GST_H264_PICTURE_IS_REF (picture);
   /* will be updated later, if any slices belong to this frame is not
    * intra slice */
   params->intra_pic_flag = 1;
 
   h264_params->frame_num = picture->frame_num;
-  h264_params->ref_pic_flag = picture->ref;
+  h264_params->ref_pic_flag = GST_H264_PICTURE_IS_REF (picture);
   /* FIXME: should be updated depending on field type? */
   h264_params->CurrFieldOrderCnt[0] = picture->top_field_order_cnt;
   h264_params->CurrFieldOrderCnt[1] = picture->bottom_field_order_cnt;
@@ -689,7 +689,7 @@ gst_nv_h264_dec_start_picture (GstH264Decoder * decoder,
       picture_index = other_frame->index;
 
     dpb->PicIdx = picture_index;
-    if (other->long_term) {
+    if (GST_H264_PICTURE_IS_LONG_TERM_REF (other)) {
       dpb->FrameIdx = other->long_term_frame_idx;
       dpb->is_long_term = 1;
     } else {
index 7f67917..a24739e 100644 (file)
@@ -476,8 +476,10 @@ gst_v4l2_codec_h264_dec_fill_decoder_params (GstV4l2CodecH264Dec * self,
       .top_field_order_cnt = ref_pic->pic_order_cnt,
       .bottom_field_order_cnt = ref_pic->bottom_field_order_cnt,
       .flags = V4L2_H264_DPB_ENTRY_FLAG_VALID
-          | (ref_pic->ref ? V4L2_H264_DPB_ENTRY_FLAG_ACTIVE : 0)
-          | (ref_pic->long_term ? V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM : 0),
+          | (GST_H264_PICTURE_IS_REF (ref_pic) ?
+              V4L2_H264_DPB_ENTRY_FLAG_ACTIVE : 0)
+          | (GST_H264_PICTURE_IS_LONG_TERM_REF (ref_pic) ?
+              V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM : 0),
     };
   }
   /* *INDENT-ON* */
index 3c01ebf..e90f84b 100644 (file)
@@ -158,11 +158,11 @@ _fill_vaapi_pic (VAPictureH264 * va_picture, GstH264Picture * picture)
   va_picture->picture_id = gst_va_decode_picture_get_surface (va_pic);
   va_picture->flags = 0;
 
-  if (picture->ref && picture->long_term) {
+  if (GST_H264_PICTURE_IS_LONG_TERM_REF (picture)) {
     va_picture->flags |= VA_PICTURE_H264_LONG_TERM_REFERENCE;
     va_picture->frame_idx = picture->long_term_frame_idx;
   } else {
-    if (picture->ref)
+    if (GST_H264_PICTURE_IS_SHORT_TERM_REF (picture))
       va_picture->flags |= VA_PICTURE_H264_SHORT_TERM_REFERENCE;
     va_picture->frame_idx = picture->frame_num;
   }