Re: [FFmpeg-devel] [PATCH avcodec/amfenc: 10 bit support, v4, 1/3] avcodec/amfenc: Fixes the color information in the output.

[Evgeny Pavlov <lucenticus@gmail.com>]

On Mon, Oct 16, 2023 at 11:41 PM Mark Thompson <sw@jkqxz.net> wrote:

> On 09/10/2023 10:52, Evgeny Pavlov wrote:
> > From: Michael Fabian 'Xaymar' Dirks <michael.dirks@xaymar.com>
> >
> > added 10 bit support for amf hevc.
> >
> > before:
> >
> > command - ffmpeg.exe -hide_banner -y -hwaccel d3d11va
> -hwaccel_output_format d3d11 -i test_10bit_file.mkv -an -c:v h264_amf
> res.dx11_hw_h264.mkv
> > output -  Format of input frames context (p010le) is not supported by
> AMF.
> > command - ffmpeg.exe -hide_banner -y -hwaccel d3d11va
> -hwaccel_output_format d3d11 -i test_10bit_file -an -c:v hevc_amf
> res.dx11_hw_hevc.mkv
> > output -  Format of input frames context (p010le) is not supported by
> AMF.
> >
> > after:
> >
> > command - ffmpeg.exe -hide_banner -y -hwaccel d3d11va
> -hwaccel_output_format d3d11 -i test_10bit_file -an -c:v h264_amf
> res.dx11_hw_h264.mkv
> > output -  10-bit input video is not supported by AMF H264 encoder
> > command - ffmpeg.exe -hide_banner -y -hwaccel d3d11va
> -hwaccel_output_format d3d11 -i test_10bit_file -an -c:v hevc_amf
> res.dx11_hw_hevc.mkv
> > output -  10bit file
> >
> > v2 - lost line returned in ff_amf_pix_fmts
> > v3 - fixes after review
> > v4 - extract duplicated code, fix incorrect processing of 10-bit input
> for h264
> >
> > Co-authored-by: Evgeny Pavlov <lucenticus@gmail.com>
> > ---
> >   libavcodec/amfenc.c      | 37 +++++++++++++++++++++++++++++++++++++
> >   libavcodec/amfenc.h      |  3 +++
> >   libavcodec/amfenc_h264.c | 24 ++++++++++++++++++++----
> >   libavcodec/amfenc_hevc.c | 26 +++++++++++++++++++++++++-
> >   4 files changed, 85 insertions(+), 5 deletions(-)
> >
> > diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
> > index 061859f85c..0bd15dd812 100644
> > --- a/libavcodec/amfenc.c
> > +++ b/libavcodec/amfenc.c
> > @@ -60,6 +60,7 @@ const enum AVPixelFormat ff_amf_pix_fmts[] = {
> >   #if CONFIG_DXVA2
> >       AV_PIX_FMT_DXVA2_VLD,
> >   #endif
> > +    AV_PIX_FMT_P010,
> >       AV_PIX_FMT_NONE
> >   };
> >
> > @@ -72,6 +73,7 @@ static const FormatMap format_map[] =
> >   {
> >       { AV_PIX_FMT_NONE,       AMF_SURFACE_UNKNOWN },
> >       { AV_PIX_FMT_NV12,       AMF_SURFACE_NV12 },
> > +    { AV_PIX_FMT_P010,       AMF_SURFACE_P010 },
> >       { AV_PIX_FMT_BGR0,       AMF_SURFACE_BGRA },
> >       { AV_PIX_FMT_RGB0,       AMF_SURFACE_RGBA },
> >       { AV_PIX_FMT_GRAY8,      AMF_SURFACE_GRAY8 },
> > @@ -785,6 +787,41 @@ int ff_amf_receive_packet(AVCodecContext *avctx,
> AVPacket *avpkt)
> >       return ret;
> >   }
> >
> > +int ff_amf_get_color_profile(AVCodecContext *avctx)
> > +{
> > +    amf_int64 color_profile = AMF_VIDEO_CONVERTER_COLOR_PROFILE_UNKNOWN;
>
> Can you explain what this CONVERTER_COLOR_PROFILE option is actually doing?
>
> You've passed the primaries and transfer function to AMF options later on,
> but this seems to then only consider a subset of possible matrices rather
> than just passing that through as well to write into the VUI metadata.  Why?
>
> (If this isn't supported by AMF then a more correct solution might be to
> insert a metadata BSF to edit the correct values into the output stream
> after it has been encoded.)
>
> When RGB surface is submitted, AMF encoder not only writes VUI header but
also does color conversion. In this case CONVERTER_COLOR_PROFILE defines
color conversion matrix. This conversion may happen with shaders or inside
VCN if it is capable to do so.

These are input parameters for conversion – if conversion is involved:
AMF_VIDEO_ENCODER_INPUT_COLOR_PROFILE – for color conversion: limited
number of color spaces is supported.
AMF_VIDEO_ENCODER_INPUT_TRANSFER_CHARACTERISTIC
AMF_VIDEO_ENCODER_INPUT_COLOR_PRIMARIES
AMF_VIDEO_ENCODER_INPUT_HDR_METADATA

These are output parameters for conversion and data for VUI:
AMF_VIDEO_ENCODER_OUTPUT_COLOR_PROFILE – for VUI only, used if color
conversion is done outside of AMF
AMF_VIDEO_ENCODER_OUTPUT_TRANSFER_CHARACTERISTIC
AMF_VIDEO_ENCODER_OUTPUT_COLOR_PRIMARIES
AMF_VIDEO_ENCODER_OUTPUT_HDR_METADATA

It would be possible to add unsupported color matrices via editing VUI in
the output stream but it is better to do it via a separate patch as
supported covers most common use cases.


> > +    if (avctx->color_range == AVCOL_RANGE_JPEG) {
> > +        /// Color Space for Full (JPEG) Range
> > +        switch (avctx->colorspace) {
> > +        case AVCOL_SPC_SMPTE170M:
> > +            color_profile = AMF_VIDEO_CONVERTER_COLOR_PROFILE_FULL_601;
> > +            break;
> > +        case AVCOL_SPC_BT709:
> > +            color_profile = AMF_VIDEO_CONVERTER_COLOR_PROFILE_FULL_709;
> > +            break;
> > +        case AVCOL_SPC_BT2020_NCL:
> > +        case AVCOL_SPC_BT2020_CL:
> > +            color_profile = AMF_VIDEO_CONVERTER_COLOR_PROFILE_FULL_2020;
> > +            break;
> > +        }
> > +    } else {
> > +        /// Color Space for Limited (MPEG) range
> > +        switch (avctx->colorspace) {
> > +        case AVCOL_SPC_SMPTE170M:
> > +            color_profile = AMF_VIDEO_CONVERTER_COLOR_PROFILE_601;
> > +            break;
> > +        case AVCOL_SPC_BT709:
> > +            color_profile = AMF_VIDEO_CONVERTER_COLOR_PROFILE_709;
> > +            break;
> > +        case AVCOL_SPC_BT2020_NCL:
> > +        case AVCOL_SPC_BT2020_CL:
> > +            color_profile = AMF_VIDEO_CONVERTER_COLOR_PROFILE_2020;
> > +            break;
> > +        }
> > +    }
> > +    return color_profile;
> > +}
> > +
> >   const AVCodecHWConfigInternal *const ff_amfenc_hw_configs[] = {
> >   #if CONFIG_D3D11VA
> >       HW_CONFIG_ENCODER_FRAMES(D3D11, D3D11VA),
> > diff --git a/libavcodec/amfenc.h b/libavcodec/amfenc.h
> > index 2dbd378ef8..62736ef579 100644
> > --- a/libavcodec/amfenc.h
> > +++ b/libavcodec/amfenc.h
> > @@ -21,6 +21,7 @@
> >
> >   #include <AMF/core/Factory.h>
> >
> > +#include <AMF/components/ColorSpace.h>
> >   #include <AMF/components/VideoEncoderVCE.h>
> >   #include <AMF/components/VideoEncoderHEVC.h>
> >   #include <AMF/components/VideoEncoderAV1.h>
> > @@ -170,6 +171,8 @@ int ff_amf_receive_packet(AVCodecContext *avctx,
> AVPacket *avpkt);
> >   */
> >   extern const enum AVPixelFormat ff_amf_pix_fmts[];
> >
> > +int ff_amf_get_color_profile(AVCodecContext *avctx);
> > +
> >   /**
> >   * Error handling helper
> >   */
> > diff --git a/libavcodec/amfenc_h264.c b/libavcodec/amfenc_h264.c
> > index bd544d12df..f785e091c9 100644
> > --- a/libavcodec/amfenc_h264.c
> > +++ b/libavcodec/amfenc_h264.c
> > @@ -199,6 +199,8 @@ static av_cold int
> amf_encode_init_h264(AVCodecContext *avctx)
> >       AMFRate                          framerate;
> >       AMFSize                          framesize =
> AMFConstructSize(avctx->width, avctx->height);
> >       int                              deblocking_filter = (avctx->flags
> & AV_CODEC_FLAG_LOOP_FILTER) ? 1 : 0;
> > +    amf_int64                        color_profile;
> > +    enum                             AVPixelFormat pix_fmt;
> >
> >       if (avctx->framerate.num > 0 && avctx->framerate.den > 0) {
> >           framerate = AMFConstructRate(avctx->framerate.num,
> avctx->framerate.den);
> > @@ -262,10 +264,24 @@ FF_ENABLE_DEPRECATION_WARNINGS
> >           AMF_ASSIGN_PROPERTY_RATIO(res, ctx->encoder,
> AMF_VIDEO_ENCODER_ASPECT_RATIO, ratio);
> >       }
> >
> > -    /// Color Range (Partial/TV/MPEG or Full/PC/JPEG)
> > -    if (avctx->color_range == AVCOL_RANGE_JPEG) {
> > -        AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
> AMF_VIDEO_ENCODER_FULL_RANGE_COLOR, 1);
> > -    }
> > +    color_profile = ff_amf_get_color_profile(avctx);
> > +    AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_OUTPUT_COLOR_PROFILE, color_profile);
> > +
> > +    /// Color Range (Support for older Drivers)
> > +    AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
> AMF_VIDEO_ENCODER_FULL_RANGE_COLOR, !!(avctx->color_range ==
> AVCOL_RANGE_JPEG));
> > +
> > +    /// Color Depth
> > +    pix_fmt = avctx->hw_frames_ctx ?
> ((AVHWFramesContext*)avctx->hw_frames_ctx->data)->sw_format
> > +                                : avctx->pix_fmt;
> > +
> > +    // 10 bit input video is not supported by AMF H264 encoder
> > +    AMF_RETURN_IF_FALSE(ctx, pix_fmt != AV_PIX_FMT_P010,
> AVERROR_INVALIDDATA, "10-bit input video is not supported by AMF H264
> encoder\n");
> > +
> > +    AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_COLOR_BIT_DEPTH, AMF_COLOR_BIT_DEPTH_8);
> > +    /// Color Transfer Characteristics (AMF matches ISO/IEC)
> > +    AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_OUTPUT_TRANSFER_CHARACTERISTIC,
> (amf_int64)avctx->color_trc);
> > +    /// Color Primaries (AMF matches ISO/IEC)
> > +    AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_OUTPUT_COLOR_PRIMARIES,
> (amf_int64)avctx->color_primaries);
> >
> >       // autodetect rate control method
> >       if (ctx->rate_control_mode ==
> AMF_VIDEO_ENCODER_RATE_CONTROL_METHOD_UNKNOWN) {
> > diff --git a/libavcodec/amfenc_hevc.c b/libavcodec/amfenc_hevc.c
> > index 352564a301..7800695648 100644
> > --- a/libavcodec/amfenc_hevc.c
> > +++ b/libavcodec/amfenc_hevc.c
> > @@ -34,8 +34,9 @@ static const AVOption options[] = {
> >       { "high_quality",           "high quality trancoding",
>       0, AV_OPT_TYPE_CONST, {.i64 =
> AMF_VIDEO_ENCODER_HEVC_USAGE_HIGH_QUALITY              }, 0, 0, VE, "usage"
> },
> >       { "lowlatency_high_quality","low latency yet high quality
> trancoding",  0, AV_OPT_TYPE_CONST, {.i64 =
> AMF_VIDEO_ENCODER_HEVC_USAGE_LOW_LATENCY_HIGH_QUALITY  }, 0, 0, VE, "usage"
> },
> >
> > -    { "profile",        "Set the profile (default main)",
>  OFFSET(profile),   AV_OPT_TYPE_INT,{ .i64 =
> AMF_VIDEO_ENCODER_HEVC_PROFILE_MAIN }, AMF_VIDEO_ENCODER_HEVC_PROFILE_MAIN,
> AMF_VIDEO_ENCODER_HEVC_PROFILE_MAIN, VE, "profile" },
> > +    { "profile",        "Set the profile (default main)",
>  OFFSET(profile),   AV_OPT_TYPE_INT,{ .i64 =
> AMF_VIDEO_ENCODER_HEVC_PROFILE_MAIN }, AMF_VIDEO_ENCODER_HEVC_PROFILE_MAIN,
> AMF_VIDEO_ENCODER_HEVC_PROFILE_MAIN_10, VE, "profile" },
> >       { "main",           "", 0,
> AV_OPT_TYPE_CONST,{ .i64 = AMF_VIDEO_ENCODER_HEVC_PROFILE_MAIN }, 0, 0, VE,
> "profile" },
> > +    { "main10",         "", 0,                      AV_OPT_TYPE_CONST,{
> .i64 = AMF_VIDEO_ENCODER_HEVC_PROFILE_MAIN_10 }, 0, 0, VE, "profile" },
> >
> >       { "profile_tier",   "Set the profile tier (default main)",
> OFFSET(tier), AV_OPT_TYPE_INT,{ .i64 = AMF_VIDEO_ENCODER_HEVC_TIER_MAIN },
> AMF_VIDEO_ENCODER_HEVC_TIER_MAIN, AMF_VIDEO_ENCODER_HEVC_TIER_HIGH, VE,
> "tier" },
> >       { "main",           "", 0, AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_TIER_MAIN }, 0, 0, VE, "tier" },
> > @@ -160,6 +161,9 @@ static av_cold int
> amf_encode_init_hevc(AVCodecContext *avctx)
> >       AMFRate             framerate;
> >       AMFSize             framesize = AMFConstructSize(avctx->width,
> avctx->height);
> >       int                 deblocking_filter = (avctx->flags &
> AV_CODEC_FLAG_LOOP_FILTER) ? 1 : 0;
> > +    amf_int64           color_depth;
> > +    amf_int64           color_profile;
> > +    enum                AVPixelFormat pix_fmt;
> >
> >       if (avctx->framerate.num > 0 && avctx->framerate.den > 0) {
> >           framerate = AMFConstructRate(avctx->framerate.num,
> avctx->framerate.den);
> > @@ -183,10 +187,14 @@ FF_ENABLE_DEPRECATION_WARNINGS
> >
> >       AMF_ASSIGN_PROPERTY_RATE(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_FRAMERATE, framerate);
> >
> > +    color_depth = AMF_COLOR_BIT_DEPTH_8;
> >       switch (avctx->profile) {
> >       case AV_PROFILE_HEVC_MAIN:
> >           profile = AMF_VIDEO_ENCODER_HEVC_PROFILE_MAIN;
> >           break;
> > +    case FF_PROFILE_HEVC_MAIN_10:
>
> Mixed FF_/AV_PROFILE enums looks a bit odd.
>
Oops, thanks for noticing this!

>
> > +        profile = AMF_VIDEO_ENCODER_HEVC_PROFILE_MAIN_10;
> > +        break;
> >       default:
> >           break;
> >       }
> > @@ -215,6 +223,22 @@ FF_ENABLE_DEPRECATION_WARNINGS
> >           AMF_ASSIGN_PROPERTY_RATIO(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_ASPECT_RATIO, ratio);
> >       }
> >
> > +    color_profile = ff_amf_get_color_profile(avctx);
> > +    AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_OUTPUT_COLOR_PROFILE, color_profile);
> > +    /// Color Range (Support for older Drivers)
> > +    AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_NOMINAL_RANGE, !!(avctx->color_range ==
> AVCOL_RANGE_JPEG));
> > +    /// Color Depth
>
> Move the first setting of color_depth here?  The setting above in
> unrelated code is easy to miss.
>
I will move the first color_depth setting here, thanks

>
> > +    pix_fmt = avctx->hw_frames_ctx ?
> ((AVHWFramesContext*)avctx->hw_frames_ctx->data)->sw_format
> > +                                    : avctx->pix_fmt;
> > +    if (pix_fmt == AV_PIX_FMT_P010) {
> > +        color_depth = AMF_COLOR_BIT_DEPTH_10;
> > +    }
> > +    AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_COLOR_BIT_DEPTH, color_depth);
>
> Is this the input bit depth or the codec bit depth?  (Can they be
> different?)
>
According to AMF documentation, this property "sets the number of bits in
each pixel’s color component in the encoder’s compressed output bitstream".
We should set up correct bit depth for encoder if we have 10-bit input

>
> > +    /// Color Transfer Characteristics (AMF matches ISO/IEC)
> > +    AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_OUTPUT_TRANSFER_CHARACTERISTIC,
> (amf_int64)avctx->color_trc);
> > +    /// Color Primaries (AMF matches ISO/IEC)
> > +    AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_OUTPUT_COLOR_PRIMARIES,
> (amf_int64)avctx->color_primaries);
> > +
> >       // Picture control properties
> >       AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_NUM_GOPS_PER_IDR, ctx->gops_per_idr);
> >       AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_GOP_SIZE, avctx->gop_size);
>
> Thanks,
>
> - Mark
> _______________________________________________
> 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".
>
_______________________________________________
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".