[FFmpeg-devel] [PATCH avcodec/amfenc: 10 bit support, v4, 1/3] avcodec/amfenc: Fixes the color information in the output.
Evgeny Pavlov
lucenticus at gmail.com
Fri Oct 20 16:41:05 EEST 2023
On Wed, Oct 18, 2023 at 10:48 PM Mark Thompson <sw at jkqxz.net> wrote:
> On 17/10/2023 19:00, Evgeny Pavlov wrote:
> > On Mon, Oct 16, 2023 at 11:41 PM Mark Thompson <sw at jkqxz.net> wrote:
> > ...
> >>> @@ -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.
>
> How does the setup you have here support the most common case, where that
> the user has the input in the right format and just wants the colour
> properties that they have set (primaries/transfer/matrix/range/chroma
> location) to be written directly into the encoded stream?
>
Full set of matrices is not currently supported in AMF, only what is in
enum in AMF headers. Makes sense to request all from AMF development team.
Primaries, transfer, range and chroma are fully supported from <>_OUTPUT_<>
parameters.
>
>
> (In most encoders this is the only case, since ad-hoc conversion of the
> input like this is generally considered out-of-scope and done separately.)
>
> >>> + 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),
> >>> ...
> >>> + 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
>
> Doesn't that mean it should be set based on the profile rather than the
> input bit depth?
>
The explicit bit depth set is needed as according to HEVC spec MAIN10
profile can be 8 or 10 bit.
>
>
> (Or, if only the input bit depth is supported then you probably want to
> validate above that the profile and input bit depth actually match.)
>
Input bit depth is defined by surface format in Init() call.
>
> >>> + /// 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);
> >>> +
>
> Thanks,
>
> - Mark
> _______________________________________________
> 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".
>
More information about the ffmpeg-devel
mailing list