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

[Xiang, Haihao]

On Di, 2024-05-21 at 08:35 +0200, Andreas Rheinhardt wrote:
> Xiang, Haihao:
> > From: Haihao Xiang <haihao.xiang at intel.com>
> > 
> > The sdk av1 encoder can accept HDR metadata via mfxEncodeCtrl::ExtParam.
> > 
> > Signed-off-by: Haihao Xiang <haihao.xiang at 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

Thanks, I will fix it in the new version.

> 
> >  
> >  #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

Thanks, I will fix it in the new version.

> 
> > +            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?)

av1_qsv encoder depends on libvpl in configure (therefore the required api
version at compile time is greater than 2.0), so this patch needn't use the
above #if. 

Thanks
Haihao


> 
> > +        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 at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".