[FFmpeg-devel] [PATCH v3 11/12] avcodec/libx264: add support for writing out CLL and MDCV

[Andreas Rheinhardt]

Jan Ekström:
> Both of these two structures were first available with X264_BUILD
> 163, so make relevant functionality conditional on the version
> being at least such.
> 
> Keep handle_side_data available in all cases as this way X264_init
> does not require additional version based conditions within it.
> 
> Finally, add a FATE test which verifies that pass-through of the
> MDCV/CLL side data is working during encoding.
> ---
>  libavcodec/libx264.c         | 79 ++++++++++++++++++++++++++++++++++++
>  tests/fate/enc_external.mak  |  5 +++
>  tests/ref/fate/libx264-hdr10 | 15 +++++++
>  3 files changed, 99 insertions(+)
>  create mode 100644 tests/ref/fate/libx264-hdr10
> 
> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> index 1a7dc7bdd5..30ea3dae4c 100644
> --- a/libavcodec/libx264.c
> +++ b/libavcodec/libx264.c
> @@ -25,6 +25,7 @@
>  #include "libavutil/eval.h"
>  #include "libavutil/internal.h"
>  #include "libavutil/opt.h"
> +#include "libavutil/mastering_display_metadata.h"
>  #include "libavutil/mem.h"
>  #include "libavutil/pixdesc.h"
>  #include "libavutil/stereo3d.h"
> @@ -842,6 +843,82 @@ static int convert_pix_fmt(enum AVPixelFormat pix_fmt)
>          return AVERROR(EINVAL);\
>      }
>  
> +#if X264_BUILD >= 163
> +static void handle_mdcv(x264_param_t *params,
> +                        const AVMasteringDisplayMetadata *mdcv)
> +{
> +    int *points[][2] = {
> +        {
> +            &params->mastering_display.i_red_x,
> +            &params->mastering_display.i_red_y
> +        },
> +        {
> +            &params->mastering_display.i_green_x,
> +            &params->mastering_display.i_green_y
> +        },
> +        {
> +            &params->mastering_display.i_blue_x,
> +            &params->mastering_display.i_blue_y
> +        },
> +    };
> +
> +    if (!mdcv->has_primaries && !mdcv->has_luminance)
> +        return;
> +
> +    params->mastering_display.b_mastering_display = 1;
> +
> +    if (!mdcv->has_primaries)
> +        goto skip_primaries;

Normally we try to avoid gotos for non-error stuff. You are basically
replacing an ordinary "if" here.

> +
> +    for (int i = 0; i < 3; i++) {
> +        const AVRational *src = mdcv->display_primaries[i];
> +        int *dst[2] = { points[i][0], points[i][1] };
> +
> +        *dst[0] = av_rescale_q(1, src[0], (AVRational){ 1, 50000 });
> +        *dst[1] = av_rescale_q(1, src[1], (AVRational){ 1, 50000 });
> +    }
> +
> +    params->mastering_display.i_white_x =
> +        av_rescale_q(1, mdcv->white_point[0], (AVRational){ 1, 50000 });
> +    params->mastering_display.i_white_y =
> +        av_rescale_q(1, mdcv->white_point[1], (AVRational){ 1, 50000 });
> +
> +skip_primaries:
> +    if (!mdcv->has_luminance)
> +        return;
> +
> +    params->mastering_display.i_display_max =
> +        av_rescale_q(1, mdcv->max_luminance, (AVRational){ 1, 10000 });
> +    params->mastering_display.i_display_min =
> +        av_rescale_q(1, mdcv->min_luminance, (AVRational){ 1, 10000 });
> +}
> +#endif // X264_BUILD >= 163
> +
> +static void handle_side_data(AVCodecContext *avctx, x264_param_t *params)
> +{
> +#if X264_BUILD >= 163
> +    const AVFrameSideDataSet set = avctx->side_data_set;
> +    const AVFrameSideData *cll_sd =
> +        av_side_data_set_get_item(set, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL);
> +    const AVFrameSideData *mdcv_sd =
> +        av_side_data_set_get_item(set, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA);

You can improve code locality by not using two variables for the side
data, but only one:
side_data = av_side_data_set_get_item(set,
AV_FRAME_DATA_CONTENT_LIGHT_LEVEL);
if (side_data) { ... }
side_data = av_side_data_set_get_item(set,
AV_FRAME_DATA_MASTERING_DISPLAY_METADATA);
if (side_data) { ... }

> +
> +    if (cll_sd) {
> +        const AVContentLightMetadata *cll =
> +            (AVContentLightMetadata *)cll_sd->data;
> +
> +        params->content_light_level.i_max_cll  = cll->MaxCLL;
> +        params->content_light_level.i_max_fall = cll->MaxFALL;
> +
> +        params->content_light_level.b_cll = 1;
> +    }
> +
> +    if (mdcv_sd) {
> +        handle_mdcv(params, (AVMasteringDisplayMetadata *)mdcv_sd->data);
> +    }
> +#endif // X264_BUILD >= 163
> +}
> +
>  static av_cold int X264_init(AVCodecContext *avctx)
>  {
>      X264Context *x4 = avctx->priv_data;
> @@ -1142,6 +1219,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
>      if (avctx->chroma_sample_location != AVCHROMA_LOC_UNSPECIFIED)
>          x4->params.vui.i_chroma_loc = avctx->chroma_sample_location - 1;
>  
> +    handle_side_data(avctx, &x4->params);
> +
>      if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER)
>          x4->params.b_repeat_headers = 0;
>  
> diff --git a/tests/fate/enc_external.mak b/tests/fate/enc_external.mak
> index d787941c16..90d8894d04 100644
> --- a/tests/fate/enc_external.mak
> +++ b/tests/fate/enc_external.mak
> @@ -7,5 +7,10 @@ FATE_ENC_EXTERNAL-$(call ENCDEC, LIBSVTAV1 HEVC, MOV, HEVC_DEMUXER LIBDAV1D_DECO
>  fate-libsvtav1-hdr10: CMD = enc_external $(TARGET_SAMPLES)/hevc/hdr10_plus_h265_sample.hevc \
>      mp4 "-c:v libsvtav1" "-show_frames -show_entries frame=side_data_list -of flat"
>  
> +# test for x264 MDCV and CLL passthrough during encoding
> +FATE_ENC_EXTERNAL-$(call ENCDEC, LIBX264 HEVC, MOV, HEVC_DEMUXER H264_DECODER) += fate-libx264-hdr10
> +fate-libx264-hdr10: CMD = enc_external $(TARGET_SAMPLES)/hevc/hdr10_plus_h265_sample.hevc \
> +    mp4 "-c:v libx264" "-show_frames -show_entries frame=side_data_list -of flat"
> +
>  FATE_SAMPLES_FFMPEG_FFPROBE += $(FATE_ENC_EXTERNAL-yes)
>  fate-enc-external: $(FATE_ENC_EXTERNAL-yes)
> diff --git a/tests/ref/fate/libx264-hdr10 b/tests/ref/fate/libx264-hdr10
> new file mode 100644
> index 0000000000..99c11677f0
> --- /dev/null
> +++ b/tests/ref/fate/libx264-hdr10
> @@ -0,0 +1,15 @@
> +frames.frame.0.side_data_list.side_data.0.side_data_type="H.26[45] User Data Unregistered SEI message"
> +frames.frame.0.side_data_list.side_data.1.side_data_type="Mastering display metadata"
> +frames.frame.0.side_data_list.side_data.1.red_x="13250/50000"
> +frames.frame.0.side_data_list.side_data.1.red_y="34500/50000"
> +frames.frame.0.side_data_list.side_data.1.green_x="7500/50000"
> +frames.frame.0.side_data_list.side_data.1.green_y="3000/50000"
> +frames.frame.0.side_data_list.side_data.1.blue_x="34000/50000"
> +frames.frame.0.side_data_list.side_data.1.blue_y="16000/50000"
> +frames.frame.0.side_data_list.side_data.1.white_point_x="15635/50000"
> +frames.frame.0.side_data_list.side_data.1.white_point_y="16450/50000"
> +frames.frame.0.side_data_list.side_data.1.min_luminance="50/10000"
> +frames.frame.0.side_data_list.side_data.1.max_luminance="10000000/10000"
> +frames.frame.0.side_data_list.side_data.2.side_data_type="Content light level metadata"
> +frames.frame.0.side_data_list.side_data.2.max_content=1000
> +frames.frame.0.side_data_list.side_data.2.max_average=200