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

[Xiang, Haihao]

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 about add a driver_quirk for workable drivers ? Or add a driver quirk in the
future if there is a driver which supports fixed frame pool only ?

Thanks
Haihao

> 
> I note that the D3D12 implementation in ffmpeg is currently incomplete and
> does not work on some hardware because it only supports dynamic pools (non-
> array textures), which makes me wonder whether changing this would cause the
> same problem for VAAPI.