[FFmpeg-devel] [PATCH] lavfi/vaapi: Don't use fixed-size frame pools

[Xiang, Haihao]

On Ma, 2024-02-26 at 06:11 +0000, Xiang, Haihao wrote:
> On So, 2024-02-25 at 19:51 +0000, Mark Thompson wrote:
> > Since e0da916b8f5b079a4865eef7f64863f50785463d the ffmpeg utility has
> > held multiple frames output by the filter graph in internal queues
> > without telling the filter which created the frames that it is going to
> > do so.  This broke many VAAPI filter->encode cases because a fixed-size
> > frame pool is used in each filter; this avoids the problem by changing
> > VAAPI filtering to always use a dynamically-sized pool.  (Note that
> > other cases with fixed-size pools my still be broken, since the ffmpeg
> > utility is still lying to libavfilter by not setting extra_hw_frames.)
> > Fixed-size frame pool support remains in the decoder, where it may still
> > be needed for compatibility.
> > ---
> > On 08/02/2024 04:15, Xiang, Haihao wrote:
> >  > Is there any comment or concern about adding a quirk for workable drivers
> > ?
> > We
> >  > may use a dynamic frame pool in vaapi decoders and filters for workable
> > drivers
> >  > only.
> >  >
> >  > Note since commit e0da916b, a command below doesn't work with the current
> > fixed
> >  > frame pool used in vaapi filters.
> >  >
> >  > $ ffmpeg -hwaccel_output_format vaapi -hwaccel vaapi -i input.mp4 -vf
> >  > 'scale_vaapi=w=720:h=480' -c:v hevc_vaapi -f null -
> >  > [...]
> >  > [vf#0:0 @ 0x562847b01050] Error while filtering: Cannot allocate memory
> >  > [vf#0:0 @ 0x562847b01050] Task finished with error code: -12 (Cannot
> > allocate
> >  > memory)
> >  > [vf#0:0 @ 0x562847b01050] Terminating thread with return code -12 (Cannot
> >  > allocate memory)
> >  > [...]
> > 
> > Having thought about this more carefully:
> > 
> > There is plenty of decoder hardware out there which effectively has one
> > address register for the reference frames and therefore requires them in
> > array
> > form.  VAAPI originally enforced this, but more recent drivers avoid
> > requiring
> > it by various methods (updated hardware or dual output).  It is still
> > required
> > by DXVA/D3D11, where the restriction is enforced directly and can't be
> > avoided.
> > 
> > Filters do not have the same problem with reference frames since none of the
> > current forms read back in their own outputs, and therefore the restriction
> > on
> > array textures for output frames doesn't really make sense for them.  The
> > one
> > possible case I can see which could plausibly be relevant are filters with
> > temporal inputs (the deinterlacer), but the inputs could always be allocated
> > freely so this doesn't really change anything.
> > 
> > Hence these two changes:
> > 
> > *
> > <https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2024-February/322125.html>,
> > which fixes the ffmpeg utility to set extra_hw_frames properly to account
> > for
> > frames stored in queues.
> > 
> > * This patch, which removes the fixed-size pools for VAAPI filtering.
> > 
> > I believe this fixes all of the VAAPI problem cases (and also DXVA/D3D11 at
> > the same time).  Any thoughts?
> 
> * There are still some issue after applying the above two changes manually. 
> 
> For example (you may find more cases from https://trac.ffmpeg.org/ticket/10856
> ):
> 
> $ ffmpeg -hwaccel vaapi -hwaccel_output_format vaapi -i 4k.mp4 -c:v h264_vaapi
> -
> f null -
> 
> This is because lots of vaapi frames are being used in the downstream elements
> (h264_vaapi encoder in the above case). 

Note the above reason is wrong. libavcodec spawns lots of threads based on the
number of cpu, each thread needs a vaapi frame for decoding, so it is easy to
make the fixed pool exhausted. 

Thanks
Haihao

> We may reduce the error with the change
> in  https://github.com/intel-media-ci/ffmpeg/pull/709/files  
> 
> * Some command lines never work, for example:
> 
> $ ffmpeg -hwaccel vaapi -hwaccel_output_format vaapi -i 1000frames.mp4 -vf
> reverse -an -f null -
> 
> The above two issues disappear if using a dynamically-sized pool. So could we
> use a dynamically-sized pool in vaapi decoders for all known backend drivers
> (e.g. iHD, i965 ) ? For other backend drivers, we still use fixed-size frame
> pools in vaapi decoders, and apply your two changes and
> https://github.com/intel-media-ci/ffmpeg/pull/709/files 
> 
> BTW seems there are format errors in your patches, I failed to apply your
> patches with 'git am'.
> 
> Thanks
> Haihao
> 
> 
> > 
> > Thanks,
> > 
> > - Mark
> > 
> > 
> >   libavfilter/vaapi_vpp.c | 11 +++--------
> >   1 file changed, 3 insertions(+), 8 deletions(-)
> > 
> > diff --git a/libavfilter/vaapi_vpp.c b/libavfilter/vaapi_vpp.c
> > index 59961bfa4a..19e7fdae97 100644
> > --- a/libavfilter/vaapi_vpp.c
> > +++ b/libavfilter/vaapi_vpp.c
> > @@ -104,7 +104,6 @@ int ff_vaapi_vpp_config_output(AVFilterLink *outlink)
> >       AVVAAPIHWConfig *hwconfig = NULL;
> >       AVHWFramesConstraints *constraints = NULL;
> >       AVHWFramesContext *output_frames;
> > -    AVVAAPIFramesContext *va_frames;
> >       VAStatus vas;
> >       int err, i;
> > 
> > @@ -203,9 +202,9 @@ int ff_vaapi_vpp_config_output(AVFilterLink *outlink)
> >       output_frames->width     = ctx->output_width;
> >       output_frames->height    = ctx->output_height;
> > 
> > -    output_frames->initial_pool_size = 4;
> > +    output_frames->initial_pool_size = 0;
> > 
> > -    err = ff_filter_init_hw_frames(avctx, outlink, 10);
> > +    err = ff_filter_init_hw_frames(avctx, outlink, 0);
> >       if (err < 0)
> >           goto fail;
> > 
> > @@ -216,14 +215,10 @@ int ff_vaapi_vpp_config_output(AVFilterLink *outlink)
> >           goto fail;
> >       }
> > 
> > -    va_frames = output_frames->hwctx;
> > -
> >       av_assert0(ctx->va_context == VA_INVALID_ID);
> >       vas = vaCreateContext(ctx->hwctx->display, ctx->va_config,
> >                             ctx->output_width, ctx->output_height,
> > -                          VA_PROGRESSIVE,
> > -                          va_frames->surface_ids, va_frames->nb_surfaces,
> > -                          &ctx->va_context);
> > +                          VA_PROGRESSIVE, NULL, 0, &ctx->va_context);
> >       if (vas != VA_STATUS_SUCCESS) {
> >           av_log(avctx, AV_LOG_ERROR, "Failed to create processing pipeline
> > "
> >                  "context: %d (%s).\n", vas, vaErrorStr(vas));
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".