Re: [FFmpeg-devel] [PATCH 2/2] lavc/qsvenc_av1: accept HDR metadata if have

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

Xiang, Haihao:
> From: Haihao Xiang <haihao.xiang@intel.com>
> 
> The sdk av1 encoder can accept HDR metadata via mfxEncodeCtrl::ExtParam.
> 
> Signed-off-by: Haihao Xiang <haihao.xiang@intel.com>
> ---
>  libavcodec/qsvenc_av1.c | 75 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 75 insertions(+)
> 
> diff --git a/libavcodec/qsvenc_av1.c b/libavcodec/qsvenc_av1.c
> index 33727bb07e..a7d35d7bef 100644
> --- a/libavcodec/qsvenc_av1.c
> +++ b/libavcodec/qsvenc_av1.c
> @@ -26,6 +26,8 @@
>  
>  #include "libavutil/common.h"
>  #include "libavutil/opt.h"
> +#include "libavutil/mem.h"
> +#include "libavutil/mastering_display_metadata.h"

Breaks alphabetical ordering

>  
>  #include "avcodec.h"
>  #include "codec_internal.h"
> @@ -39,6 +41,77 @@ typedef struct QSVAV1EncContext {
>      QSVEncContext qsv;
>  } QSVAV1EncContext;
>  
> +static int qsv_av1_set_encode_ctrl(AVCodecContext *avctx,
> +                                   const AVFrame *frame, mfxEncodeCtrl *enc_ctrl)
> +{
> +    QSVAV1EncContext *q = avctx->priv_data;
> +    AVFrameSideData *sd;
> +
> +    if (!frame || !QSV_RUNTIME_VERSION_ATLEAST(q->qsv.ver, 2, 11))
> +        return 0;
> +
> +    sd = av_frame_get_side_data(frame, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA);
> +    if (sd) {
> +        AVMasteringDisplayMetadata *mdm = (AVMasteringDisplayMetadata *)sd->data;
> +
> +        if (mdm->has_primaries && mdm->has_luminance) {
> +            const int chroma_den   = 1 << 16;
> +            const int max_luma_den = 1 << 8;
> +            const int min_luma_den = 1 << 14;
> +            int i;

Should have loop-scope

> +            mfxExtMasteringDisplayColourVolume *mdcv = av_mallocz(sizeof(mfxExtMasteringDisplayColourVolume));
> +            if (!mdcv)
> +                return AVERROR(ENOMEM);
> +
> +            mdcv->Header.BufferId = MFX_EXTBUFF_MASTERING_DISPLAY_COLOUR_VOLUME;
> +            mdcv->Header.BufferSz = sizeof(*mdcv);
> +
> +            for (i = 0; i < 3; i++) {
> +                mdcv->DisplayPrimariesX[i] =
> +                    av_rescale(mdm->display_primaries[i][0].num, chroma_den,
> +                               mdm->display_primaries[i][0].den);
> +                mdcv->DisplayPrimariesY[i] =
> +                    av_rescale(mdm->display_primaries[i][1].num, chroma_den,
> +                               mdm->display_primaries[i][1].den);
> +            }
> +
> +            mdcv->WhitePointX =
> +                av_rescale(mdm->white_point[0].num, chroma_den,
> +                           mdm->white_point[0].den);
> +            mdcv->WhitePointY =
> +                av_rescale(mdm->white_point[1].num, chroma_den,
> +                           mdm->white_point[1].den);
> +
> +            mdcv->MaxDisplayMasteringLuminance =
> +                av_rescale(mdm->max_luminance.num, max_luma_den,
> +                           mdm->max_luminance.den);
> +            mdcv->MinDisplayMasteringLuminance =
> +                av_rescale(mdm->min_luminance.num, min_luma_den,
> +                           mdm->min_luminance.den);
> +
> +            enc_ctrl->ExtParam[enc_ctrl->NumExtParam++] = (mfxExtBuffer *)mdcv;
> +        }
> +    }
> +
> +    sd = av_frame_get_side_data(frame, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL);
> +    if (sd) {
> +        AVContentLightMetadata *clm = (AVContentLightMetadata *)sd->data;
> +        mfxExtContentLightLevelInfo * clli = av_mallocz(sizeof(mfxExtContentLightLevelInfo));

sizeof(*clli)

But actually, both allocations seem to be unnecessary: This callback is
only called when there is a QSVFrame and it already contains a
mfxExtContentLightLevelInfo and mfxExtMasteringDisplayColourVolume (used
by the decoder), so why not simply reuse that? The type of
set_encode_ctrl_cb would need to be changed of course.

(Btw: The relevant QSVFrame fields are under "#if QSV_VERSION_ATLEAST(1,
35)", yet this patch does not use any such #if. Why?)

> +        if (!clli)
> +            return AVERROR(ENOMEM);
> +
> +        clli->Header.BufferId = MFX_EXTBUFF_CONTENT_LIGHT_LEVEL_INFO;
> +        clli->Header.BufferSz = sizeof(*clli);
> +
> +        clli->MaxContentLightLevel          = clm->MaxCLL;
> +        clli->MaxPicAverageLightLevel       = clm->MaxFALL;
> +
> +        enc_ctrl->ExtParam[enc_ctrl->NumExtParam++] = (mfxExtBuffer *)clli;
> +    }
> +
> +    return 0;
> +}
> +
>  static av_cold int qsv_enc_init(AVCodecContext *avctx)
>  {
>      QSVAV1EncContext *q = avctx->priv_data;
> @@ -61,6 +134,8 @@ static av_cold int qsv_enc_init(AVCodecContext *avctx)
>             return ret;
>      }
>  
> +    q->qsv.set_encode_ctrl_cb = qsv_av1_set_encode_ctrl;
> +
>      return ff_qsv_enc_init(avctx, &q->qsv);
>  }
>  

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".