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

[Xiang, Haihao]

On Di, 2024-03-19 at 22:52 +0000, Mark Thompson wrote:
> 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.

Hi Mark,

I agree with you. Will you push this patch ? We may switch to dynamic frame pool
for vaapi cases later after pushing your patch. 

Thanks
Haihao