From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 8ECFB450D7 for ; Thu, 5 Jan 2023 22:28:27 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 790A468BDB7; Fri, 6 Jan 2023 00:28:24 +0200 (EET) Received: from mail-wm1-f45.google.com (mail-wm1-f45.google.com [209.85.128.45]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 72E0C68BD94 for ; Fri, 6 Jan 2023 00:28:18 +0200 (EET) Received: by mail-wm1-f45.google.com with SMTP id i17-20020a05600c355100b003d99434b1cfso2440106wmq.1 for ; Thu, 05 Jan 2023 14:28:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jkqxz-net.20210112.gappssmtp.com; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=nQ8tEEETJHeUJeU20Jr8cL1T3dNlzyNDWiLXs5kNQp4=; b=PIcMei6MueZXFG9eQDPmHnWVh5Ia/DAiWgLJmkhny8umu/hnqvOqZFsUrl6WGopm+4 4ckfizkNFoSSLWwEcp6gOoYMmg9HtNklPWbrsbFjmBnUv4hFfIwaMt6RcDAn6SerejLR fVtu4NV+du/l6pGmdNqObUhYkfEGQqJuYlPSngKGGz+CUlUQnN04zTapBZgUe+bl/Xwm gFKA1Ptz7Ag1jEByVBceQpMHkWIGjc+LcWiYu8uaGD57dUMUJF17KlnwCXlbxn+VCYg3 3TgJN6r2pC/RBgQNVmm8uLcT9EgMIOKYjUhF/le7zXpJvlQMV10yz8Il37MSmVnogT+1 0qYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=nQ8tEEETJHeUJeU20Jr8cL1T3dNlzyNDWiLXs5kNQp4=; b=IdozdFXMarbgkq1ZefMcNkqhiCw14gigZaKU5W6KHTpY2BSRZTIFF4MUDsjB1x7IwL OCL8k1r3VdBgjF9LUEuVQEUKdxuyJovjTIUtP0YPQWkhn8+F1D/WfIH6y+U2A5+WmLpX yLrSuGQOl9BkOn7AOeO96CluthPxNUl9PlI0bOiVQHHEiF2Jx5Crh8TrrXtJnAsCUwbz cp6Go6Tas14jAcFDwqEm786tvMbJYX1GY9pZH8kxSNDfbJLBWALXOlobIvWiLZ9lgngk DNThs/15d+T4IveRGturwL0WZ2NK1vSd79Bw45Dkm2VbgRSUyL5IyesUmtcilIW+nA8n lvyw== X-Gm-Message-State: AFqh2krDp5WIxtEFm7aapZhjVLCWgp90eklNqElk7kim6hyP9azuMkGQ dIrZKKAEANw3hRTo1r8uMXbrAXJUATiE9Dwj X-Google-Smtp-Source: AMrXdXv73+Ypxx4UsvSCEHzenB99LBDYkEE351JmaosIOsiPIIxLJzAIw7vMTtHjflOlju7YUD3PhQ== X-Received: by 2002:a05:600c:15d4:b0:3d1:d746:d95b with SMTP id v20-20020a05600c15d400b003d1d746d95bmr41856863wmf.41.1672957697658; Thu, 05 Jan 2023 14:28:17 -0800 (PST) Received: from [192.168.0.14] (cpc91224-cmbg18-2-0-cust209.5-4.cable.virginm.net. [81.106.228.210]) by smtp.gmail.com with ESMTPSA id u13-20020a05600c19cd00b003c6f1732f65sm4338031wmq.38.2023.01.05.14.28.17 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 05 Jan 2023 14:28:17 -0800 (PST) Message-ID: <4a9e9778-c94e-f5b7-bce6-0f60a4a5371a@jkqxz.net> Date: Thu, 5 Jan 2023 22:27:53 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.0 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: From: Mark Thompson In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH] vaapi: support VAProfileH264High10 X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 26/12/2022 06:59, Jean Jogh wrote: > see https://github.com/intel/libva/pull/664 > > Signed-off-by: jianfeng.zheng > --- > configure | 13 +++++++++++++ > libavcodec/h264_slice.c | 6 ++++++ > libavcodec/vaapi_decode.c | 10 ++++++++++ > libavcodec/vaapi_encode_h264.c | 24 ++++++++++++++++++++++-- > libavcodec/vaapi_h264.c | 5 +++-- > 5 files changed, 54 insertions(+), 4 deletions(-) > > diff --git a/configure b/configure > index f08cdab3d1..ac199d97cb 100755 > --- a/configure > +++ b/configure > @@ -2410,6 +2410,7 @@ HAVE_LIST=" > texi2html > xmllint > zlib_gzip > + va_profile_h264_high10 > " > > # options emitted with CONFIG_ prefix but not available on the command line > @@ -6958,6 +6959,18 @@ if enabled vaapi; then > check_type "va/va.h va/va_enc_jpeg.h" "VAEncPictureParameterBufferJPEG" > check_type "va/va.h va/va_enc_vp8.h" "VAEncPictureParameterBufferVP8" > check_type "va/va.h va/va_enc_vp9.h" "VAEncPictureParameterBufferVP9" > + > + # > + # Using 'VA_CHECK_VERSION' in source codes make things easy. But > we have to wait > + # until newly added VAProfile being distributed by VAAPI released version. > + # > + # Before or after that, we can use auto-detection to keep version > compatibility. > + # It always works. > + # > + disable va_profile_h264_high10 && enabled h264_vaapi_hwaccel > + test_code cc va/va.h "VAProfile p = VAProfileH264High10" && > + enable va_profile_h264_high10 To echo comments from others, I think it would be better follow the same form as other codecs and use the VA_CHECK_VERSION for the next release. > + > fi > > if enabled_all opencl libdrm ; then > diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c > index 420758ba0a..b9236281b1 100644 > --- a/libavcodec/h264_slice.c > +++ b/libavcodec/h264_slice.c > @@ -806,6 +806,12 @@ static enum AVPixelFormat > get_pixel_format(H264Context *h, int force_callback) > } else if (CHROMA422(h)) > *fmt++ = AV_PIX_FMT_YUV422P10; > else > +#if CONFIG_H264_VAAPI_HWACCEL > + // Just add as candidate. Whether VAProfileH264High10 usable or > + // not is decided by vaapi_decode_make_config() defined in FFmpeg > + // and vaQueryCodingProfile() defined in libva. > + *fmt++ = AV_PIX_FMT_VAAPI; > +#endif > *fmt++ = AV_PIX_FMT_YUV420P10; Braces definitely needed here! > break; > case 12: > diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c > index 134f10eca5..03bbbf2c41 100644 > --- a/libavcodec/vaapi_decode.c > +++ b/libavcodec/vaapi_decode.c > @@ -402,6 +402,9 @@ static const struct { > H264ConstrainedBaseline), > MAP(H264, H264_MAIN, H264Main ), > MAP(H264, H264_HIGH, H264High ), > +#ifdef HAVE_VA_PROFILE_H264_HIGH10 > + MAP(H264, H264_HIGH_10, H264High10 ), > +#endif > #if VA_CHECK_VERSION(0, 37, 0) > MAP(HEVC, HEVC_MAIN, HEVCMain ), > MAP(HEVC, HEVC_MAIN_10, HEVCMain10 ), > @@ -510,6 +513,13 @@ static int vaapi_decode_make_config(AVCodecContext *avctx, > if (exact_match) > break; > } > +#ifdef HAVE_VA_PROFILE_H264_HIGH10 > + //incase 8bit stream being decoded under VAProfileH264High10 > + if (avctx->codec_id == AV_CODEC_ID_H264 && > + (avctx->profile == FF_PROFILE_H264_EXTENDED || > avctx->profile == FF_PROFILE_H264_BASELINE) && > + matched_va_profile == VAProfileH264High) > + break; > +#endif Can you explain what this is trying to do? Why does it need a special case where H.265 doesn't? > } > av_freep(&profile_list); > > diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c > index dd17be2190..d26ac5a98f 100644 > --- a/libavcodec/vaapi_encode_h264.c > +++ b/libavcodec/vaapi_encode_h264.c > @@ -23,6 +23,7 @@ > > #include "libavutil/avassert.h" > #include "libavutil/common.h" > +#include "libavutil/pixdesc.h" > #include "libavutil/internal.h" > #include "libavutil/opt.h" > > @@ -290,10 +291,21 @@ static int > vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx) > H264RawPPS *pps = &priv->raw_pps; > VAEncSequenceParameterBufferH264 *vseq = ctx->codec_sequence_params; > VAEncPictureParameterBufferH264 *vpic = ctx->codec_picture_params; > + const AVPixFmtDescriptor *desc; > + int bit_depth; > > memset(sps, 0, sizeof(*sps)); > memset(pps, 0, sizeof(*pps)); > > + desc = av_pix_fmt_desc_get(priv->common.input_frames->sw_format); > + av_assert0(desc); > + if (desc->nb_components == 1 || desc->log2_chroma_w != 1 || > desc->log2_chroma_h != 1) { > + av_log(avctx, AV_LOG_ERROR, "Chroma format of input pixel format " > + "%s is not supported.\n", desc->name); > + return AVERROR(EINVAL); > + } > + bit_depth = desc->comp[0].depth; > + > sps->nal_unit_header.nal_ref_idc = 3; > sps->nal_unit_header.nal_unit_type = H264_NAL_SPS; > > @@ -303,11 +315,11 @@ static int > vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx) > avctx->profile == FF_PROFILE_H264_MAIN) > sps->constraint_set1_flag = 1; > > - if (avctx->profile == FF_PROFILE_H264_HIGH) > + if (avctx->profile == FF_PROFILE_H264_HIGH || avctx->profile == > FF_PROFILE_H264_HIGH_10) > sps->constraint_set3_flag = ctx->gop_size == 1; > > if (avctx->profile == FF_PROFILE_H264_MAIN || > - avctx->profile == FF_PROFILE_H264_HIGH) { > + avctx->profile == FF_PROFILE_H264_HIGH || avctx->profile == > FF_PROFILE_H264_HIGH_10) { > sps->constraint_set4_flag = 1; > sps->constraint_set5_flag = ctx->b_per_p == 0; > } > @@ -348,6 +360,8 @@ static int > vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx) > > sps->seq_parameter_set_id = 0; > sps->chroma_format_idc = 1; > + sps->bit_depth_luma_minus8 = bit_depth - 8; > + sps->bit_depth_chroma_minus8 = bit_depth - 8; > > sps->log2_max_frame_num_minus4 = 4; > sps->pic_order_cnt_type = 0; > @@ -1111,6 +1125,9 @@ static av_cold int > vaapi_encode_h264_configure(AVCodecContext *avctx) > } > > static const VAAPIEncodeProfile vaapi_encode_h264_profiles[] = { > +#ifdef HAVE_VA_PROFILE_H264_HIGH10 > + { FF_PROFILE_H264_HIGH_10, 10, 3, 1, 1, VAProfileH264High10 }, > +#endif > { FF_PROFILE_H264_HIGH, 8, 3, 1, 1, VAProfileH264High }, > { FF_PROFILE_H264_MAIN, 8, 3, 1, 1, VAProfileH264Main }, > { FF_PROFILE_H264_CONSTRAINED_BASELINE, > @@ -1175,11 +1192,13 @@ static av_cold int > vaapi_encode_h264_init(AVCodecContext *avctx) > av_log(avctx, AV_LOG_ERROR, "H.264 extended profile " > "is not supported.\n"); > return AVERROR_PATCHWELCOME; > +#ifndef HAVE_VA_PROFILE_H264_HIGH10 > case FF_PROFILE_H264_HIGH_10: > case FF_PROFILE_H264_HIGH_10_INTRA: > av_log(avctx, AV_LOG_ERROR, "H.264 10-bit profiles " > "are not supported.\n"); > return AVERROR_PATCHWELCOME; > +#endif This is doing early rejection of explicitly-specified profiles which libavcodec definitely can't support - I think just remove this block entirely, since it will be rejected later if not supported. (Presumably explicitly-specified High 10 Intra does work as expected? It looks like it should, but it would be good to check.) > case FF_PROFILE_H264_HIGH_422: > case FF_PROFILE_H264_HIGH_422_INTRA: > case FF_PROFILE_H264_HIGH_444: > @@ -1267,6 +1286,7 @@ static const AVOption vaapi_encode_h264_options[] = { > { PROFILE("constrained_baseline", FF_PROFILE_H264_CONSTRAINED_BASELINE) }, > { PROFILE("main", FF_PROFILE_H264_MAIN) }, > { PROFILE("high", FF_PROFILE_H264_HIGH) }, > + { PROFILE("high10", FF_PROFILE_H264_HIGH_10) }, > #undef PROFILE > > { "level", "Set level (level_idc)", Hmm. It looks like libva has the same deficiency here as in H.265 where it doesn't support the lowest QPs. Is there any way of fixing that on the libva side?7 > diff --git a/libavcodec/vaapi_h264.c b/libavcodec/vaapi_h264.c > index 9332aa6f31..3e44c8caf6 100644 > --- a/libavcodec/vaapi_h264.c > +++ b/libavcodec/vaapi_h264.c > @@ -234,6 +234,7 @@ static int vaapi_h264_start_frame(AVCodecContext > *avctx, > VAPictureParameterBufferH264 pic_param; > VAIQMatrixBufferH264 iq_matrix; > int err; > + int qp_bd_offset = 6 * (sps->bit_depth_luma - 8); > > pic->output_surface = ff_vaapi_get_surface_id(h->cur_pic_ptr->f); > > @@ -256,8 +257,8 @@ static int vaapi_h264_start_frame(AVCodecContext > *avctx, > .log2_max_pic_order_cnt_lsb_minus4 = > sps->log2_max_poc_lsb - 4, > .delta_pic_order_always_zero_flag = > sps->delta_pic_order_always_zero_flag, > }, > - .pic_init_qp_minus26 = pps->init_qp - 26, > - .pic_init_qs_minus26 = pps->init_qs - 26, > + .pic_init_qp_minus26 = pps->init_qp - > 26 - qp_bd_offset, > + .pic_init_qs_minus26 = pps->init_qs - > 26 - qp_bd_offset, The field is (signed) pic_init_qp_minus26 as found in the PPS in order to allow header parsing in hardware if it wants. You should not be applying QpBdOffset as if making QP'_y here, because that's only done at the MB level. Given that you've sent this and therefore it presumably works, I guess you must have a driver applying the reverse offset to recover the actual QP? Can you fix the driver not to do that? > .chroma_qp_index_offset = > pps->chroma_qp_index_offset[0], > .second_chroma_qp_index_offset = > pps->chroma_qp_index_offset[1], > .pic_fields.bits = { 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".