Re: [FFmpeg-devel] [PATCH v4 1/4] lavc/vaapi_encode_h265: Add GPB frame support for hevc_vaapi

[Mark Thompson <sw@jkqxz.net>]

On 14/03/2022 02:15, Xiang, Haihao wrote:
> On Sun, 2022-03-13 at 20:45 +0000, Mark Thompson wrote:
>> On 11/03/2022 09:00, Fei Wang wrote:
>>> From: Linjie Fu <linjie.fu@intel.com>
>>>
>>> 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 <linjie.fu@intel.com>
>>> Signed-off-by: Fei Wang <fei.w.wang@intel.com>
>>> ---
>>> 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?
> 
> According to
> https://github.com/intel/media-driver/blob/master/media_driver/agnostic/common/codec/hal/codechal_vdenc_hevc.cpp#L3072-L3087
> , L0 and L1 should be the same for vdenc hevc on some platforms,

Right, so this is actually a different constraint which isn't indicated so far.  It's not just the BI_NOT_EMPTY constraint that P slices cannot be used, it's also that in B slices RefPicList1 has to be identical to RefPicList0.  Perhaps this should be added to libva?

>                                                                  so user can't
> use past and future reference together,

Where is this coming from?  I don't see how past vs. future references are relevant at all (since the driver can't see that anyway), only the matching content of the lists.

Maybe in this case the right behaviour would be to concatenate what would normally be in the two lists and then put that in both of them (assuming it supports the necessary reference counts, but this particular hardware appears to allow 3/3 so it would).

>                                         which is why you experienced the failure
> after applying version 2

Yep - and is also why the P-frame only case works, since that puts the same single entry in both lists.

- 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".