avcodec/h26[45]_metadata_bsf: Use separate contexts for reading/writing

Currently, both bsfs used the same CodedBitstreamContext for reading and
writing; as a consequence, the state of the writer's context at the
beginning of writing a fragment is exactly the state of the reader after
having read the fragment; in particular, the writer might not have
encountered one of its active parameter sets yet.

This is not nice and may lead to invalid output even when the input
is completely spec-compliant: Think of an access unit containing
a primary coded picture referencing a PPS with id id (that is known from
an earlier access unit/from extradata), then a new version of the PPS
with id id and then a redundant coded picture that is also referencing
the PPS with id id. This is spec-compliant, as the standard allows to
overwrite a PPS with a different PPS in between coded pictures and not
only at the beginning of an access unit. In this scenario, the reader
would read the primary coded picture with the old PPS and the redundant
coded picture with the new PPS (as it should); yet the writer would
write both with the new PPS as extradata which might lead to errors or
to invalid data being output without any error (e.g. if the two PPS
differed in redundant_pic_cnt_present_flag).

The above scenario does not directly translate to HEVC as long as one
restricts oneself to input with nuh_layer_id == 0 only (as cbs_h265
does: it currently strips away any NAL unit with nuh_layer_id > 0 when
decomposing); if one doesn't the same issue as above can happen.

If one also allowed input packets to contain more than one access unit,
issues like the above can happen even without redundant coded
pictures/multiple layers.

Therefore this commit uses separate contexts for reader and writer.

Reviewed-by: Mark Thompson <sw@jkqxz.net>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>

libavcodec/h264_metadata_bsf.c

@@ -49,7 +49,8 @@ enum {
 typedef struct H264MetadataContext {
     const AVClass *class;
 
-    CodedBitstreamContext *cbc;
+    CodedBitstreamContext *input;
+    CodedBitstreamContext *output;
     CodedBitstreamFragment access_unit;
 
     int done_first_au;
@@ -289,7 +290,7 @@ static int h264_metadata_update_side_data(AVBSFContext *bsf, AVPacket *pkt)
     if (!side_data_size)
         return 0;
 
-    err = ff_cbs_read(ctx->cbc, au, side_data, side_data_size);
+    err = ff_cbs_read(ctx->input, au, side_data, side_data_size);
     if (err < 0) {
         av_log(bsf, AV_LOG_ERROR, "Failed to read extradata from packet side data.\n");
         return err;
@@ -303,7 +304,7 @@ static int h264_metadata_update_side_data(AVBSFContext *bsf, AVPacket *pkt)
         }
     }
 
-    err = ff_cbs_write_fragment_data(ctx->cbc, au);
+    err = ff_cbs_write_fragment_data(ctx->output, au);
     if (err < 0) {
         av_log(bsf, AV_LOG_ERROR, "Failed to write extradata into packet side data.\n");
         return err;
@@ -334,7 +335,7 @@ static int h264_metadata_filter(AVBSFContext *bsf, AVPacket *pkt)
     if (err < 0)
         goto fail;
 
-    err = ff_cbs_read_packet(ctx->cbc, au, pkt);
+    err = ff_cbs_read_packet(ctx->input, au, pkt);
     if (err < 0) {
         av_log(bsf, AV_LOG_ERROR, "Failed to read packet.\n");
         goto fail;
@@ -602,7 +603,7 @@ static int h264_metadata_filter(AVBSFContext *bsf, AVPacket *pkt)
         }
     }
 
-    err = ff_cbs_write_packet(ctx->cbc, pkt, au);
+    err = ff_cbs_write_packet(ctx->output, pkt, au);
     if (err < 0) {
         av_log(bsf, AV_LOG_ERROR, "Failed to write packet.\n");
         goto fail;
@@ -626,12 +627,15 @@ static int h264_metadata_init(AVBSFContext *bsf)
     CodedBitstreamFragment *au = &ctx->access_unit;
     int err, i;
 
-    err = ff_cbs_init(&ctx->cbc, AV_CODEC_ID_H264, bsf);
+    err = ff_cbs_init(&ctx->input,  AV_CODEC_ID_H264, bsf);
+    if (err < 0)
+        return err;
+    err = ff_cbs_init(&ctx->output, AV_CODEC_ID_H264, bsf);
     if (err < 0)
         return err;
 
     if (bsf->par_in->extradata) {
-        err = ff_cbs_read_extradata(ctx->cbc, au, bsf->par_in);
+        err = ff_cbs_read_extradata(ctx->input, au, bsf->par_in);
         if (err < 0) {
             av_log(bsf, AV_LOG_ERROR, "Failed to read extradata.\n");
             goto fail;
@@ -645,7 +649,7 @@ static int h264_metadata_init(AVBSFContext *bsf)
             }
         }
 
-        err = ff_cbs_write_extradata(ctx->cbc, bsf->par_out, au);
+        err = ff_cbs_write_extradata(ctx->output, bsf->par_out, au);
         if (err < 0) {
             av_log(bsf, AV_LOG_ERROR, "Failed to write extradata.\n");
             goto fail;
@@ -663,7 +667,8 @@ static void h264_metadata_close(AVBSFContext *bsf)
     H264MetadataContext *ctx = bsf->priv_data;
 
     ff_cbs_fragment_free(&ctx->access_unit);
-    ff_cbs_close(&ctx->cbc);
+    ff_cbs_close(&ctx->input);
+    ff_cbs_close(&ctx->output);
 }
 
 #define OFFSET(x) offsetof(H264MetadataContext, x)

libavcodec/h265_metadata_bsf.c

@@ -40,7 +40,8 @@ enum {
 typedef struct H265MetadataContext {
     const AVClass *class;
 
-    CodedBitstreamContext *cbc;
+    CodedBitstreamContext *input;
+    CodedBitstreamContext *output;
     CodedBitstreamFragment access_unit;
 
     H265RawAUD aud_nal;
@@ -350,7 +351,7 @@ static int h265_metadata_update_side_data(AVBSFContext *bsf, AVPacket *pkt)
     if (!side_data_size)
         return 0;
 
-    err = ff_cbs_read(ctx->cbc, au, side_data, side_data_size);
+    err = ff_cbs_read(ctx->input, au, side_data, side_data_size);
     if (err < 0) {
         av_log(bsf, AV_LOG_ERROR, "Failed to read extradata from packet side data.\n");
         return err;
@@ -372,7 +373,7 @@ static int h265_metadata_update_side_data(AVBSFContext *bsf, AVPacket *pkt)
         }
     }
 
-    err = ff_cbs_write_fragment_data(ctx->cbc, au);
+    err = ff_cbs_write_fragment_data(ctx->output, au);
     if (err < 0) {
         av_log(bsf, AV_LOG_ERROR, "Failed to write extradata into packet side data.\n");
         return err;
@@ -402,7 +403,7 @@ static int h265_metadata_filter(AVBSFContext *bsf, AVPacket *pkt)
     if (err < 0)
         goto fail;
 
-    err = ff_cbs_read_packet(ctx->cbc, au, pkt);
+    err = ff_cbs_read_packet(ctx->input, au, pkt);
     if (err < 0) {
         av_log(bsf, AV_LOG_ERROR, "Failed to read packet.\n");
         goto fail;
@@ -473,7 +474,7 @@ static int h265_metadata_filter(AVBSFContext *bsf, AVPacket *pkt)
         }
     }
 
-    err = ff_cbs_write_packet(ctx->cbc, pkt, au);
+    err = ff_cbs_write_packet(ctx->output, pkt, au);
     if (err < 0) {
         av_log(bsf, AV_LOG_ERROR, "Failed to write packet.\n");
         goto fail;
@@ -495,12 +496,15 @@ static int h265_metadata_init(AVBSFContext *bsf)
     CodedBitstreamFragment *au = &ctx->access_unit;
     int err, i;
 
-    err = ff_cbs_init(&ctx->cbc, AV_CODEC_ID_HEVC, bsf);
+    err = ff_cbs_init(&ctx->input,  AV_CODEC_ID_HEVC, bsf);
+    if (err < 0)
+        return err;
+    err = ff_cbs_init(&ctx->output, AV_CODEC_ID_HEVC, bsf);
     if (err < 0)
         return err;
 
     if (bsf->par_in->extradata) {
-        err = ff_cbs_read_extradata(ctx->cbc, au, bsf->par_in);
+        err = ff_cbs_read_extradata(ctx->input, au, bsf->par_in);
         if (err < 0) {
             av_log(bsf, AV_LOG_ERROR, "Failed to read extradata.\n");
             goto fail;
@@ -522,7 +526,7 @@ static int h265_metadata_init(AVBSFContext *bsf)
             }
         }
 
-        err = ff_cbs_write_extradata(ctx->cbc, bsf->par_out, au);
+        err = ff_cbs_write_extradata(ctx->output, bsf->par_out, au);
         if (err < 0) {
             av_log(bsf, AV_LOG_ERROR, "Failed to write extradata.\n");
             goto fail;
@@ -540,7 +544,8 @@ static void h265_metadata_close(AVBSFContext *bsf)
     H265MetadataContext *ctx = bsf->priv_data;
 
     ff_cbs_fragment_free(&ctx->access_unit);
-    ff_cbs_close(&ctx->cbc);
+    ff_cbs_close(&ctx->input);
+    ff_cbs_close(&ctx->output);
 }
 
 #define OFFSET(x) offsetof(H265MetadataContext, x)