[FFmpeg-devel] [PATCH 2/8] av1_metadata: Add option to update video parameters

[James Almer]

On 12/1/2018 4:28 PM, Andreas Rheinhardt wrote:
> Up until now, this BSF only changed the bitstream, not the
> AVCodecParameters. The result is that e.g. some muxers use outdated
> information to write header information that conflicts with (and precedes)
> the new information at the bitstream level, so that the updates might
> not lead to the desired change.
> This commit changes this. It adds a mechanism by which the new
> information can be used to update the AVCodecParameters, too. This is
> the new default and an option has been added to revert to the old
> behaviour.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at googlemail.com>
> ---
>  doc/bitstream_filters.texi    |  6 +++++
>  libavcodec/av1_metadata_bsf.c | 43 +++++++++++++++++++++++++++++++----
>  2 files changed, 45 insertions(+), 4 deletions(-)
> 
> diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi
> index 15c578aa8a..80df345f26 100644
> --- a/doc/bitstream_filters.texi
> +++ b/doc/bitstream_filters.texi
> @@ -87,6 +87,12 @@ the timing info in the sequence header.
>  Set the number of ticks in each picture, to indicate that the stream
>  has a fixed framerate.  Ignored if @option{tick_rate} is not also set.
>  
> + at item full_update
> +If this is set, the AVCodecParameters are updated in addition to the
> +bitstream. If unset, muxers might add header information based upon the
> +old AVCodecParameters that contradicts and potentially precedes the
> +changes made at the bitstream level. On by default.
> +
>  @end table
>  
>  @section chomp
> diff --git a/libavcodec/av1_metadata_bsf.c b/libavcodec/av1_metadata_bsf.c
> index 52d383661f..0c8d00acea 100644
> --- a/libavcodec/av1_metadata_bsf.c
> +++ b/libavcodec/av1_metadata_bsf.c
> @@ -46,11 +46,15 @@ typedef struct AV1MetadataContext {
>  
>      AVRational tick_rate;
>      int num_ticks_per_picture;
> +
> +    int full_update;
>  } AV1MetadataContext;
>  
>  
>  static int av1_metadata_update_sequence_header(AVBSFContext *bsf,
> -                                               AV1RawSequenceHeader *seq)
> +                                               AV1RawSequenceHeader *seq,
> +                                               int *color_range,
> +                                               int *chroma_sample_position)
>  {
>      AV1MetadataContext *ctx = bsf->priv_data;
>      AV1RawColorConfig  *clc = &seq->color_config;
> @@ -82,6 +86,8 @@ static int av1_metadata_update_sequence_header(AVBSFContext *bsf,
>                     "on RGB streams encoded in BT.709 sRGB.\n");
>          } else {
>              clc->color_range = ctx->color_range;
> +            if (color_range)
> +                *color_range = ctx->color_range;
>          }
>      }
>  
> @@ -91,6 +97,8 @@ static int av1_metadata_update_sequence_header(AVBSFContext *bsf,
>                     "can only be set for 4:2:0 streams.\n");
>          } else {
>              clc->chroma_sample_position = ctx->chroma_sample_position;
> +            if (chroma_sample_position)
> +                *chroma_sample_position = ctx->chroma_sample_position;
>          }
>      }
>  
> @@ -135,7 +143,8 @@ static int av1_metadata_filter(AVBSFContext *bsf, AVPacket *out)
>      for (i = 0; i < frag->nb_units; i++) {
>          if (frag->units[i].type == AV1_OBU_SEQUENCE_HEADER) {
>              obu = frag->units[i].content;
> -            err = av1_metadata_update_sequence_header(bsf, &obu->obu.sequence_header);
> +            err = av1_metadata_update_sequence_header(bsf, &obu->obu.sequence_header,
> +                                                      NULL, NULL);
>              if (err < 0)
>                  goto fail;
>          }
> @@ -184,7 +193,7 @@ static int av1_metadata_init(AVBSFContext *bsf)
>      AV1MetadataContext *ctx = bsf->priv_data;
>      CodedBitstreamFragment *frag = &ctx->access_unit;
>      AV1RawOBU *obu;
> -    int err, i;
> +    int err, i, color_range = -1, chroma_sample_position = -1;
>  
>      err = ff_cbs_init(&ctx->cbc, AV_CODEC_ID_AV1, bsf);
>      if (err < 0)
> @@ -200,7 +209,8 @@ static int av1_metadata_init(AVBSFContext *bsf)
>          for (i = 0; i < frag->nb_units; i++) {
>              if (frag->units[i].type == AV1_OBU_SEQUENCE_HEADER) {
>                  obu = frag->units[i].content;
> -                err = av1_metadata_update_sequence_header(bsf, &obu->obu.sequence_header);
> +                err = av1_metadata_update_sequence_header(bsf, &obu->obu.sequence_header,
> +                                                          &color_range, &chroma_sample_position);
>                  if (err < 0)
>                      goto fail;
>              }
> @@ -213,6 +223,28 @@ static int av1_metadata_init(AVBSFContext *bsf)
>          }
>      }
>  
> +    if (ctx->full_update) {
> +        if (chroma_sample_position >= 0) {
> +            const int conversion_table[4] = { AVCHROMA_LOC_UNSPECIFIED,
> +                                              AVCHROMA_LOC_LEFT,
> +                                              AVCHROMA_LOC_TOPLEFT,
> +                                              AVCHROMA_LOC_UNSPECIFIED };
> +            chroma_sample_position = conversion_table[chroma_sample_position];
> +        }
> +
> +        if (ctx->color_range >= 0)
> +            ctx->color_range++;
> +
> +        ff_cbs_update_video_parameters(ctx->cbc, bsf->par_out, -1, -1, -1, -1,
> +                                       -1, color_range, ctx->color_primaries,
> +                                       ctx->transfer_characteristics,
> +                                       ctx->matrix_coefficients,
> +                                       chroma_sample_position, -1);
> +
> +        if (ctx->color_range != -1)
> +            ctx->color_range--;
> +    }
> +
>      err = 0;
>  fail:
>      ff_cbs_fragment_uninit(ctx->cbc, frag);
> @@ -273,6 +305,9 @@ static const AVOption av1_metadata_options[] = {
>          OFFSET(num_ticks_per_picture), AV_OPT_TYPE_INT,
>          { .i64 = -1 }, -1, INT_MAX, FLAGS },
>  
> +    { "full_update", "Update not only bitstream, but also AVCodecParameters.",
> +        OFFSET(full_update), AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, FLAGS},

I don't think this needs an option. It should be the default behavior.
The BSF framework is written in a way that changing AVCodecParameters
values is expected (Or necessary, like in cases where it adds extradata).

> +
>      { NULL }
>  };
>  
>