[FFmpeg-devel] [PATCH v3 2/2] lavc/get_buffer: Add a warning on failed allocation from a fixed pool

[Mark Thompson]

On 19/03/2024 04:16, Xiang, Haihao wrote:
> On Ma, 2024-03-18 at 21:33 +0000, Mark Thompson wrote:
>> On 18/03/2024 05:53, Xiang, Haihao wrote:
>>> On So, 2024-03-17 at 20:51 +0000, Mark Thompson wrote:
>>>> For hardware cases where we are forced to have a fixed pool of frames
>>>> allocated up-front (such as array textures on decoder output), suggest
>>>> a possible workaround to the user if an allocation fails because the
>>>> pool is exhausted.
>>>> ---
>>>> Perhaps helpful; any thoughts?
>>>>
>>>> The warning comes out before any errors, looking like:
>>>>
>>>> [mpeg2video @ 0x560ff51b4600] Failed to allocate a vaapi/nv12 frame from a
>>>> fixed pool of hardware frames.
>>>> [mpeg2video @ 0x560ff51b4600] Consider setting extra_hw_frames to a larger
>>>> value (currently set to 8, giving a pool size of 14).
>>>> [mpeg2video @ 0x560ff51b4600] get_buffer() failed
>>>> [vist#0:0/mpeg2video @ 0x560ff5199840] [dec:mpeg2video @ 0x560ff51b3b40]
>>>> Error
>>>> submitting packet to decoder: Operation not permitted
>>>
>>> I'm OK to print such warning so user may know how to work around it. But now
>>> many cases are impacted by this error
>>> (e.g. https://trac.ffmpeg.org/ticket/10856
>>> ), I think it is a regression to user. I still prefer to use a dynamic
>>> buffer
>>> pool instead fixed frame pool to avoid such error when the dynamic buffer
>>> pool
>>> can work.
>>
>> How would we implement this on D3D11 or D3D12?
> 
> I understand not all can support dynamic frame pool, your patch is useful for
> decoders using fixed pool. But for driver which doesn't require array textures,
> I think we'd be better to use dynamic frame pool instead so user needn't worry
> about frame allocation. For example, we may use dynamic frame pool for vaapi
> with iHD driver, what do you think about adding a quirk to enable dynamic frame
> pool for special drivers ?

I think we should come to a conclusion on what the generic code for this case should be and then consider whether any special cases are needed.

When compared to the state right now, I agree with you that just switching VAAPI to always be dynamic would probably be better just to avoid the nasty failures, but given that we need to improve the situation for cases (like D3D11) where we don't have an ad-hoc workaround we should be comparing to whatever that concludes rather than the broken state right now.

Thanks,

- Mark