[FFmpeg-devel] [PATCH v2 11/12] lavc/vaapi_decode: use dynamic frame pool for output frames with libva2

[Mark Thompson]

On 30/01/2024 06:30, Xiang, Haihao wrote:
> On Ma, 2024-01-29 at 21:58 +0000, Mark Thompson wrote:
>> On 26/01/2024 07:25, Xiang, Haihao wrote:
>>> On Wo, 2023-12-20 at 15:10 +0800, Xiang, Haihao wrote:
>>>> From: Haihao Xiang <haihao.xiang at intel.com>
>>>>
>>>> This allows a downstream element stores more frames from VAAPI
>>>> decoders and fixes error in get_buffer()
>>>>
>>>> $ ffmpeg -hwaccel vaapi -hwaccel_output_format vaapi -i
>>>> input_100frames.mp4 \
>>>>    -vf reverse -an -f null -
>>>> ...
>>>> [h264 @ 0x557a075a1400] get_buffer() failed
>>>> [h264 @ 0x557a075a1400] thread_get_buffer() failed
>>>> [h264 @ 0x557a075a1400] decode_slice_header error
>>>> [h264 @ 0x557a075a1400] no frame!
>>>>
>>>> Signed-off-by: Haihao Xiang <haihao.xiang at intel.com>
>>>> ---
>>>>    libavcodec/vaapi_decode.c | 36 ++++++++++++++++++++----------------
>>>>    1 file changed, 20 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c
>>>> index ceac769c52..8cc29e96f9 100644
>>>> --- a/libavcodec/vaapi_decode.c
>>>> +++ b/libavcodec/vaapi_decode.c
>>>> @@ -601,22 +601,26 @@ static int vaapi_decode_make_config(AVCodecContext
>>>> *avctx,
>>>>            if (err < 0)
>>>>                goto fail;
>>>>    
>>>> -        frames->initial_pool_size = 1;
>>>> -        // Add per-codec number of surfaces used for storing reference
>>>> frames.
>>>> -        switch (avctx->codec_id) {
>>>> -        case AV_CODEC_ID_H264:
>>>> -        case AV_CODEC_ID_HEVC:
>>>> -        case AV_CODEC_ID_AV1:
>>>> -            frames->initial_pool_size += 16;
>>>> -            break;
>>>> -        case AV_CODEC_ID_VP9:
>>>> -            frames->initial_pool_size += 8;
>>>> -            break;
>>>> -        case AV_CODEC_ID_VP8:
>>>> -            frames->initial_pool_size += 3;
>>>> -            break;
>>>> -        default:
>>>> -            frames->initial_pool_size += 2;
>>>> +        if (CONFIG_VAAPI_1)
>>>> +            frames->initial_pool_size = 0;
>>>> +        else {
>>>> +            frames->initial_pool_size = 1;
>>>> +            // Add per-codec number of surfaces used for storing
>>>> reference
>>>> frames.
>>>> +            switch (avctx->codec_id) {
>>>> +            case AV_CODEC_ID_H264:
>>>> +            case AV_CODEC_ID_HEVC:
>>>> +            case AV_CODEC_ID_AV1:
>>>> +                frames->initial_pool_size += 16;
>>>> +                break;
>>>> +            case AV_CODEC_ID_VP9:
>>>> +                frames->initial_pool_size += 8;
>>>> +                break;
>>>> +            case AV_CODEC_ID_VP8:
>>>> +                frames->initial_pool_size += 3;
>>>> +                break;
>>>> +            default:
>>>> +                frames->initial_pool_size += 2;
>>>> +            }
>>>>            }
>>>>        }
>>>>    
>>>
>>> Hi Mark,
>>>
>>> Do you have any comment about dynamic frame pool used in vaapi ?
>>
>> Are we completely sure that there are no driver/hardware combinations which
>> rely on this still used?
> 
> I tested this patch with i965, iHD and radeonsi drivers on Linux and vaon12
> driver on Windows. But honestly I am not sure whether there is a driver which
> works with fixed frame pool only.

How does the vaon12 driver work with this given that some D3D12 devices require a fixed array texture?

Note that the interesting test here is not the most recent version of any of these things.  Rather, it is the older versions which exist in a distribution configuration which we still want to support, for example Ubuntu 20.04.

I'm also unclear to what degree this might depend on the hardware being used.  Certainly in D3D12 whether the fixed array texture is required depends on the actual hardware support.

Thanks,

- Mark