[FFmpeg-devel] [PATCH] libavcodec/nvdec: Do not exceed 32 surfaces when initializing hw_frames_ctx

[Ameer Jalil]

Turns out the issue I was facing which prompted this patch request stemmed
from improper usage of the APIs, and this patch was a workaround that was
hiding the proper fix.
Please feel free to reject this patch request, especially if it is likely
to cause unforeseen issues down the line.

Thanks
-Ameer

On Tue, Aug 10, 2021 at 8:14 AM Dennis Mungai <dmngaie at gmail.com> wrote:

> On Tue, 10 Aug 2021 at 15:04, Timo Rothenpieler <timo at rothenpieler.org>
> wrote:
>
> > On 10.08.2021 04:37, Ameer J wrote:
> > > From: ameerj <52414509+ameerj at users.noreply.github.com>
> > >
> > > nvdec is likely to fail when the initial pool size exceeds 32. This
> > change ensures we don't exceed the limit when initializing a new
> > hw_frames_ctx
> > >
> > > Signed-off-by: ameerj <52414509+ameerj at users.noreply.github.com>
> > > ---
> > >   libavcodec/nvdec.c | 6 ++++--
> > >   1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/libavcodec/nvdec.c b/libavcodec/nvdec.c
> > > index 251be039a8..bef33dbae9 100644
> > > --- a/libavcodec/nvdec.c
> > > +++ b/libavcodec/nvdec.c
> > > @@ -303,8 +303,10 @@ static int nvdec_init_hwframes(AVCodecContext
> > *avctx, AVBufferRef **out_frames_r
> > >       frames_ctx = (AVHWFramesContext*)(*out_frames_ref)->data;
> > >
> > >       if (dummy) {
> > > -        // Copied from ff_decode_get_hw_frames_ctx for compatibility
> > > -        frames_ctx->initial_pool_size += 3;
> > > +        // The function above guarantees 1 work surface, We must
> > guarantee 4 work surfaces.
> > > +        // (the absolute minimum), so add the missing count without
> > exceeding the maximum
> > > +        // recommended for nvdec.
> > > +        frames_ctx->initial_pool_size =
> > min(frames_ctx->initial_pool_size + 3, 32);
> >
> > Where does this min() function come from?
> >
> > >           frames_ctx->free = nvdec_free_dummy;
> > >           frames_ctx->pool = av_buffer_pool_init(0, nvdec_alloc_dummy);
> > >
> >
> > So far, the 32 surface limit has only ever been enforced as a soft
> > limit, and a warning printed when exceeded. Since it being a limit is
> > not strictly documented and it might be risen in the future.
> >
> > Also, if the pool is limited in size, something is likely to fail due to
> > that further down the chain.
> > So I'm not sure if this is a better solution.
>
>
>
> Related,
>
> Can the pool surface setting be made configurable?
> The older CUVID based wrappers expose it as an option (via -surfaces) as
> does the NVENC encoder wrapper implementation. This would be useful in
> cases where lower VRAM foot print is required (for multiple transcodes on
> the same GPU) and at the same time, tunable enough to prevent downstream
> failures if surfaces are insufficient.
>
> >
> _______________________________________________
> 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".
>