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.rheinha...@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.
>
> +@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 }
> };
>
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
