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 2C95940F31 for ; Sun, 13 Mar 2022 20:45:59 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id DCA8F68B124; Sun, 13 Mar 2022 22:45:56 +0200 (EET) Received: from mail-wr1-f51.google.com (mail-wr1-f51.google.com [209.85.221.51]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 51FA368A5CC for ; Sun, 13 Mar 2022 22:45:50 +0200 (EET) Received: by mail-wr1-f51.google.com with SMTP id u10so20950489wra.9 for ; Sun, 13 Mar 2022 13:45:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jkqxz-net.20210112.gappssmtp.com; s=20210112; h=message-id:date:mime-version:user-agent:subject:content-language:to :references:from:in-reply-to:content-transfer-encoding; bh=OlLpCGAl+VgZUN458ViJ6MnAmm0ABM4XZRXyCzrecW4=; b=wgbJ7idImG1Q8zCPpg9JAGfWobakJ3X12XlICgsAymDK3+cbVrfeslIINYZMO/JPah 8aVASetRyOH+Gz747Nfzmhj+Mmw3Po5ZaBFQVyliyC1YRtVjVcEDgmlKoordpAPVPA2N CKDZuU9pLA1awcOOq90ZP37PGyXWq8KClucPOYwOd7PrHjEuxzTI5qMyN9iVlJuKZX4U IueN7Hn4DCjm6IWGo07nddcNnb1aSsyImmO6+TwTKNx6CERRaRUlX50ToyGUhVmPeoc3 7z7UfOCPTqAaArTu1cKs73RnWqiKevIWIu/Mgrpaz2Fn1tQ16duncAufqDdnFA99RvIO +N2g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:references:from:in-reply-to :content-transfer-encoding; bh=OlLpCGAl+VgZUN458ViJ6MnAmm0ABM4XZRXyCzrecW4=; b=LAAs87TgX2QiAtmDV9H9+gkOfViSZVVlcJDNkzY3BNHtNnunBcN7kC5QD8tN9Z7rzn eOigSDrGxB0fhf+mnPH2zbl3Z8/W2M/9LZT7ejCeaVOwRwVu88gxmfcNMxZrhoTF/yPm KQIaS0CC5z+ZOeInQCgozSD2fdd1w8S5RtkuftdaI5sZ8h1qaZKJLhK0hMqjtvK87Z9J QQvrft3FqKylBeTnyKdsgHgicPAZWBZhPo6H7gDhlp3yaMuAFV4+y9SErn1d6/eKr4EH XJs2QId6edR1CyVQNvIjLqwyCUdVuMY5z5lMm9XbMjjlc8A1BybtS/wu9PkP1O6rnHKP LjDA== X-Gm-Message-State: AOAM531N1p0RUChP71+1mpTOOB/oxkRC30xbORD1XCTQBNHdKChpDwt/ VTEedPukiAI0tDEqL7VAvaODrGhYHvAPtA== X-Google-Smtp-Source: ABdhPJxu40lT1w3MscIYoqIBjnYvJWlwBM7V+X6hn+PNUqUHjyU/898LtvmG7wsNP7rH4bm9ZlUC/Q== X-Received: by 2002:a5d:404b:0:b0:1f1:f880:7aca with SMTP id w11-20020a5d404b000000b001f1f8807acamr14625269wrp.179.1647204349540; Sun, 13 Mar 2022 13:45:49 -0700 (PDT) Received: from [192.168.0.11] (cpc91222-cmbg18-2-0-cust46.5-4.cable.virginm.net. [81.106.30.47]) by smtp.gmail.com with ESMTPSA id b15-20020adfc74f000000b001e888b871a0sm12062895wrh.87.2022.03.13.13.45.48 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 13 Mar 2022 13:45:49 -0700 (PDT) Message-ID: Date: Sun, 13 Mar 2022 20:45:45 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.6.2 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20220311090046.262777-1-fei.w.wang@intel.com> From: Mark Thompson In-Reply-To: <20220311090046.262777-1-fei.w.wang@intel.com> Subject: Re: [FFmpeg-devel] [PATCH v4 1/4] lavc/vaapi_encode_h265: Add GPB frame support for hevc_vaapi 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 11/03/2022 09:00, Fei Wang wrote: > From: Linjie Fu > > Use GPB frames to replace regular P/B frames if backend driver does not > support it. > > - GPB: > Generalized P and B picture. Regular P/B frames replaced by B > frames with previous-predict only, L0 == L1. Normal B frames > still have 2 different ref_lists and allow bi-prediction > > Signed-off-by: Linjie Fu > Signed-off-by: Fei Wang > --- > update: > 1. Add b to gpb. > 2. Optimise debug message. > > libavcodec/vaapi_encode.c | 74 +++++++++++++++++++++++++++++++--- > libavcodec/vaapi_encode.h | 2 + > libavcodec/vaapi_encode_h265.c | 24 ++++++++++- > 3 files changed, 93 insertions(+), 7 deletions(-) > > diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c > index 3bf379b1a0..bdba9726b2 100644 > --- a/libavcodec/vaapi_encode.c > +++ b/libavcodec/vaapi_encode.c > @@ -848,9 +848,13 @@ static void vaapi_encode_set_b_pictures(AVCodecContext *avctx, > pic->b_depth = current_depth; > > vaapi_encode_add_ref(avctx, pic, start, 1, 1, 0); > - vaapi_encode_add_ref(avctx, pic, end, 1, 1, 0); > vaapi_encode_add_ref(avctx, pic, prev, 0, 0, 1); > > + if (!ctx->b_to_gpb) > + vaapi_encode_add_ref(avctx, pic, end, 1, 1, 0); > + else > + vaapi_encode_add_ref(avctx, pic, end, 0, 1, 0); This is doing something extremely dubious. If b-to-gpb is set, then don't use the future reference? That means these pictures will only have the past reference, and the coding efficiency will often be much worse. E.g. if you have the default structure (max_b_frames = 2, max_b_depth = 1) then in a sequence of four pictures you get: 1 referring to something previous 4 referring to 1 2 referring to 1 and 4 3 referring to 1 and 4 and this change means you lose the 2 -> 4 and 3 -> 4 references. Therefore, a change in the picture between 1 and 2 will end up coded three times in 2, 3 and 4 rather than just being coded in 4 and then referred to by the others. > + > for (ref = end->refs[1]; ref; ref = ref->refs[1]) > vaapi_encode_add_ref(avctx, pic, ref, 0, 1, 0); > } > @@ -871,8 +875,11 @@ static void vaapi_encode_set_b_pictures(AVCodecContext *avctx, > > vaapi_encode_add_ref(avctx, pic, pic, 0, 1, 0); > vaapi_encode_add_ref(avctx, pic, start, 1, 1, 0); > - vaapi_encode_add_ref(avctx, pic, end, 1, 1, 0); > vaapi_encode_add_ref(avctx, pic, prev, 0, 0, 1); > + if (!ctx->b_to_gpb) > + vaapi_encode_add_ref(avctx, pic, end, 1, 1, 0); > + else > + vaapi_encode_add_ref(avctx, pic, end, 0, 1, 0); > > for (ref = end->refs[1]; ref; ref = ref->refs[1]) > vaapi_encode_add_ref(avctx, pic, ref, 0, 1, 0); > @@ -1845,6 +1852,51 @@ static av_cold int vaapi_encode_init_gop_structure(AVCodecContext *avctx) > ref_l1 = attr.value >> 16 & 0xffff; > } > > + ctx->p_to_gpb = 0; > + ctx->b_to_gpb = 0; > + > +#if VA_CHECK_VERSION(1, 9, 0) > + if (!(ctx->codec->flags & FLAG_INTRA_ONLY || > + avctx->gop_size <= 1)) { > + attr = (VAConfigAttrib) { VAConfigAttribPredictionDirection }; > + vas = vaGetConfigAttributes(ctx->hwctx->display, > + ctx->va_profile, > + ctx->va_entrypoint, > + &attr, 1); > + if (vas != VA_STATUS_SUCCESS) { > + av_log(avctx, AV_LOG_WARNING, "Failed to query prediction direction " > + "attribute: %d (%s).\n", vas, vaErrorStr(vas)); > + return AVERROR_EXTERNAL; > + } else if (attr.value == VA_ATTRIB_NOT_SUPPORTED) { > + av_log(avctx, AV_LOG_VERBOSE, "Driver does not report any additional " > + "prediction constraints.\n"); > + } else { > + if (((ref_l0 > 0 || ref_l1 > 0) && !(attr.value & VA_PREDICTION_DIRECTION_PREVIOUS)) || > + ((ref_l1 == 0) && (attr.value & (VA_PREDICTION_DIRECTION_FUTURE | VA_PREDICTION_DIRECTION_BI_NOT_EMPTY)))) { > + av_log(avctx, AV_LOG_ERROR, "Driver report incorrect prediction " > + "direction attribute.\n"); > + return AVERROR_EXTERNAL; > + } > + > + if (!(attr.value & VA_PREDICTION_DIRECTION_FUTURE)) { > + if (ref_l0 > 0 && ref_l1 > 0) { > + ctx->b_to_gpb = 1; > + av_log(avctx, AV_LOG_VERBOSE, "Driver support previous prediction " > + "only for B-frames.\n"); > + } > + } Please note that the PREDICTION_DIRECTION_FUTURE/PREVIOUS options don't mean anything for H.265. The driver isn't told which direction the prediction is in, it's only told about some reference frames and how to refer to them. Whether those frames are in the past or future is unspecified and irrelevant - a P frame can refer to a single future frame if it wants. (I tried to argue this when it was added, but given that they are meaningless I didn't argue very hard.) I suspect you are trying to use this as a test of something else. Perhaps you could explain what the test you actually want is? > + > + if (attr.value & VA_PREDICTION_DIRECTION_BI_NOT_EMPTY) { > + if (ref_l0 > 0 && ref_l1 > 0) { > + ctx->p_to_gpb = 1; > + av_log(avctx, AV_LOG_VERBOSE, "Driver does not support P-frames, " > + "replacing them with previous prediction only B-frames.\n"); > + } > + } > + } > + } > +#endif > ... - 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".