Re: [FFmpeg-devel] [PATCH v4 4/9] avcodec: add cbs for h266/vvc

[James Almer]

On 1/25/2021 2:12 PM, James Almer wrote:
Did you add this just to set active_vps? I thought the 
idea was to remove all that since it's not a concept defined in h266.

The following should get rid of active_*, but the actual fields can't be 
removed from CodedBitstreamH266Context unless we duplicate the 
cbs_h2645_replace_ps() macro to remove the relevant lines it shares with 
the other two codecs.

diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
index da541407cf..2516481015 100644
--- a/libavcodec/cbs_h2645.c
+++ b/libavcodec/cbs_h2645.c
@@ -1690,10 +1690,6 @@ static void cbs_h266_flush(CodedBitstreamContext *ctx)
     }
     av_buffer_unref(&h266->ph_ref);
     h266->ph = NULL;
-
-    h266->active_vps = NULL;
-    h266->active_sps = NULL;
-    h266->active_pps = NULL;
 }


diff --git a/libavcodec/cbs_h266.h b/libavcodec/cbs_h266.h
index 8ce29be3fa..bf15a14d85 100644
--- a/libavcodec/cbs_h266.h
+++ b/libavcodec/cbs_h266.h
@@ -800,9 +800,7 @@ typedef struct CodedBitstreamH266Context {
     H266RawPPS  *pps[VVC_MAX_PPS_COUNT];
     H266RawPH   *ph;

-    // The currently active parameter sets.  These are updated when any
-    // NAL unit refers to the relevant parameter set.  These pointers
-    // must also be present in the arrays above.
+    // Unused, added to keep compatibility with the cbs_h2645_replace_ps() 
macro
     const H266RawVPS *active_vps;
     const H266RawSPS *active_sps;
     const H266RawPPS *active_pps;
diff --git a/libavcodec/cbs_h266_syntax_template.c 
b/libavcodec/cbs_h266_syntax_template.c
index c030766c34..3e1a8be5d9 100644
--- a/libavcodec/cbs_h266_syntax_template.c
+++ b/libavcodec/cbs_h266_syntax_template.c
@@ -463,11 +463,9 @@ static int FUNC(ref_pic_list_struct)(CodedBitstreamContext 
*ctx, RWContext *rw,
 }

 static int FUNC(ref_pic_lists)(CodedBitstreamContext *ctx, RWContext *rw,
-                               H266RefPicLists *current)
+                               H266RefPicLists *current,
+                               const H266RawSPS *sps, const H266RawPPS *pps)
 {
-    CodedBitstreamH266Context *h266 = ctx->priv_data;
-    const H266RawSPS *sps = h266->active_sps;
-    const H266RawPPS *pps = h266->active_pps;
     const H266RefPicListStruct *ref_list;
     int err, i, j, num_ltrp_entries;
     for (i = 0; i < 2; i++) {
@@ -974,7 +972,6 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, RWContext 
*rw,
         vps->vps_layer_id[0] = current->nal_unit_header.nuh_layer_id;
         h266->vps_ref[0] = ref;
         h266->vps[0] = vps;
-        h266->active_vps = vps;
     }

     u(3, sps_max_sublayers_minus1, 0, VVC_MAX_SUBLAYERS - 1);
@@ -1528,7 +1525,6 @@ static int FUNC(pps)(CodedBitstreamContext *ctx, 
RWContext *rw,
                current->pps_seq_parameter_set_id);
         return AVERROR_INVALIDDATA;
     }
-    h266->active_sps = sps;

     flag(pps_mixed_nalu_types_in_pic_flag);
     ue(pps_pic_width_in_luma_samples,
@@ -2194,9 +2190,6 @@ static int FUNC(picture_header)(CodedBitstreamContext 
*ctx, RWContext *rw,
                sps->sps_video_parameter_set_id);
         return AVERROR_INVALIDDATA;
     }
-    h266->active_sps = sps;
-    h266->active_pps = pps;
-    h266->active_vps = vps;

     ub(sps->sps_log2_max_pic_order_cnt_lsb_minus4 + 4, ph_pic_order_cnt_lsb);
     if (current->ph_gdr_pic_flag)
@@ -2292,7 +2285,7 @@ static int FUNC(picture_header)(CodedBitstreamContext 
*ctx, RWContext *rw,
     else
         infer(ph_pic_output_flag, 1);
     if (pps->pps_rpl_info_in_ph_flag) {
-        CHECK(FUNC(ref_pic_lists)(ctx, rw, &current->ph_ref_pic_lists));
+        CHECK(FUNC(ref_pic_lists)(ctx, rw, &current->ph_ref_pic_lists, sps, 
pps));
     }
     if (sps->sps_partition_constraints_override_enabled_flag)
         flag(ph_partition_constraints_override_flag);
@@ -2591,17 +2584,13 @@ static int FUNC(slice_header)(CodedBitstreamContext 
*ctx, RWContext *rw,
         h266->ph = (H266RawPH*)h266->ph_ref->data;
         memcpy(h266->ph, &current->sh_picture_header, sizeof(H266RawPH));
     }
-    sps = h266->active_sps;
-    pps = h266->active_pps;
     ph  = h266->ph;
-
     if (!ph) {
         av_log(ctx->log_ctx, AV_LOG_ERROR, "Picture header not available.\n");
         return AVERROR_INVALIDDATA;
     }
-    //sps and pps checked in picture_header(), just assert them.
-    av_assert0(sps);
-    av_assert0(pps);
+    pps = h266->pps[ph->ph_pic_parameter_set_id];
+    sps = h266->sps[pps->pps_seq_parameter_set_id];

     if (sps->sps_subpic_info_present_flag) {
         ub(sps->sps_subpic_id_len_minus1 + 1, sh_subpic_id);
@@ -2722,7 +2711,7 @@ static int FUNC(slice_header)(CodedBitstreamContext *ctx, 
RWContext *rw,
         ((nal_unit_type != VVC_IDR_W_RADL &&
         nal_unit_type != VVC_IDR_N_LP) ||
         sps->sps_idr_rpl_present_flag)) {
-        CHECK(FUNC(ref_pic_lists)(ctx, rw, &current->sh_ref_pic_lists));
+        CHECK(FUNC(ref_pic_lists)(ctx, rw, &current->sh_ref_pic_lists, sps, 
pps));
         ref_pic_lists = &current->sh_ref_pic_lists;
     } else {
         ref_pic_lists = &h266->ph->ph_ref_pic_lists;
diff --git a/libavcodec/vvc_parser.c b/libavcodec/vvc_parser.c
index b160ef9426..61db91d6f0 100644
--- a/libavcodec/vvc_parser.c
+++ b/libavcodec/vvc_parser.c
@@ -166,15 +166,13 @@ static int parse_nal_units(AVCodecParserContext *ctx, 
const uint8_t *buf,
         goto end;
     }

-    sps = h266->active_sps;
-    pps = h266->active_pps;
     ph = h266->ph;
     if (!ph) {
         av_log(avctx, AV_LOG_ERROR, "No picture header.\n");
         goto end;
     }
-    av_assert0(sps);
-    av_assert0(pps);
+    pps = h266->pps[ph->ph_pic_parameter_set_id];
+    sps = h266->sps[pps->pps_seq_parameter_set_id];

     ctx->key_frame    = ph->ph_gdr_or_irap_pic_flag;
     ctx->coded_width  = pps->pps_pic_width_in_luma_samples;

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".