[FFmpeg-devel] [PATCH] avcodec/h264_metadata_bsf: Allow zeroing constraint_set4_flag and constraint_set5_flag

[James Almer]

On 6/23/2021 1:12 PM, Derek Buitenhuis wrote:
> These bits are reserved in earlie versions of the H.264 spec, and
> some poor hardware decoders require they are zero. Thus, it is useful
> to be able to zero these on streams that may have them set. The result
> is still a valid H.264 bitstream.
> 
> Signed-off-by: Derek Buitenhuis <derek.buitenhuis at gmail.com>
> ---
> e.g. x264 wrote these bits for several months before reverting that
> functionality, since it broke several hardware decoders.
> ---
>   doc/bitstream_filters.texi     | 5 +++++
>   libavcodec/h264_metadata_bsf.c | 8 ++++++++
>   2 files changed, 13 insertions(+)
> 
> diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi
> index 60e729484d..81220638d4 100644
> --- a/doc/bitstream_filters.texi
> +++ b/doc/bitstream_filters.texi
> @@ -253,6 +253,11 @@ Set whether the stream has fixed framerate - typically this indicates
>   that the framerate is exactly half the tick rate, but the exact
>   meaning is dependent on interlacing and the picture structure (see
>   H.264 section E.2.1 and table E-6).
> + at item zero_new_constraint_set_flags
> +Zero constraint_set4_flag and constraint_set5_flag in the SPS. These
> +bits were reserved in a previous version of the H.264 spec, and thus
> +some hardware encoders require these to be zero. The result of zeroing

Decoders.

> +this is still a valid bitstream.
>   
>   @item crop_left
>   @item crop_right
> diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
> index ef74cba560..452a8ec5dc 100644
> --- a/libavcodec/h264_metadata_bsf.c
> +++ b/libavcodec/h264_metadata_bsf.c
> @@ -61,6 +61,7 @@ typedef struct H264MetadataContext {
>   
>       AVRational tick_rate;
>       int fixed_frame_rate_flag;
> +    int zero_new_constraint_set_flags;
>   
>       int crop_left;
>       int crop_right;
> @@ -228,6 +229,10 @@ static int h264_metadata_update_sps(AVBSFContext *bsf,
>           need_vui = 1;
>       }
>       SET_VUI_FIELD(fixed_frame_rate_flag);
> +    if (ctx->zero_new_constraint_set_flags) {
> +        sps->constraint_set4_flag = 0;
> +        sps->constraint_set5_flag = 0;
> +    }
>   
>       if (sps->separate_colour_plane_flag || sps->chroma_format_idc == 0) {
>           crop_unit_x = 1;
> @@ -618,6 +623,9 @@ static const AVOption h264_metadata_options[] = {
>       { "fixed_frame_rate_flag", "Set VUI fixed frame rate flag",
>           OFFSET(fixed_frame_rate_flag), AV_OPT_TYPE_INT,
>           { .i64 = -1 }, -1, 1, FLAGS },
> +    { "zero_new_constraint_set_flags", "Set constraint_set4_flag / constraint_set5_flag to zero",
> +        OFFSET(zero_new_constraint_set_flags), AV_OPT_TYPE_BOOL,
> +        { .i64 = 0 }, 0, 1, FLAGS },
>   
>       { "crop_left", "Set left border crop offset",
>           OFFSET(crop_left), AV_OPT_TYPE_INT,

I guess it's fine, but Mark may have some comments about it.