[FFmpeg-devel] [PATCH] lavc/libvpxenc: prevent fifo from filling up

[David Lemler]

> On 07/05/2023 2:16 PM CDT James Zern <jzern-at-google.com at ffmpeg.org> wrote:
> 
>  
> On Wed, Jul 5, 2023 at 12:15 PM James Zern <jzern at google.com> wrote:
> >
> > Hi,
> >
> 
> +ffmpeg-dev. I took the wrong email off the reply.
> 
> > On Mon, Jul 3, 2023 at 10:08 PM David Lemler <david at lemler.family> wrote:
> > >
> > > Prevent the fifo used in encoding VPx videos from filling up and stopping
> > > encode when it reaches 21845 items, which happens when the video has more
> > > than
> > > that number of frames.
> > >
> >
> > What failure do you see, a memory allocation error? 21845 sounds
> > somewhat arbitrary, maybe specific to your machine, so I'd leave it
> > off the comment and commit message.
> >

When running a command like the following to perform the first pass of a
2-pass VP9 encode:

ffmpeg -i input.mp4 -c:v libvpx-vp9 -b:v 0 -crf 31 -pass 1 -an \
-deadline best -threads 8 -row-mt 1 -f null /dev/null

ffmpeg exited with the error:
Error submitting video frame to the encoder
Conversion failed!

and the ffmpeg2pass-0.log file existed but was empty.

I've tested and reproduced the issue on Intel x86-64 on Windows
(10 22H2, gyan.dev build) and Linux (Debian 12, own compiled build), and on
AMD EPYC x86-64 on Linux (Debian 12, own compiled build).  I believe the
number of frames is (1024 * 1024) / sizeof(FrameData); see lavc/libvpxenc.c
the call to av_fifo_alloc2 around line 999 and lavutil/fifo.c the assignment
of f->auto_grow_limit around line 72.  I'm not sure if that calculation
would change on different systems/architectures.

During testing, I added a debug log below the can_grow assignment ternary in
fifo_check_space in lavutil/fifo.c logging out the values of need_grow,
can_grow, f->auto_grow_limit, and f->nb_elems and noticed that as the encode
was happening, the nb_elems value kept increasing and can_grow kept
decreasing until nb_elems got to f->auto_grow_limit and can_grow to 0, at
which point the encode failed with the error mentioned previously.

This is the first time I've contributed to FFmpeg (or a project that uses a
mailing list), so I'm still becoming familiar with standard practices for
commit formatting and development.  If that value is dependent on variables
like compiler or architecture or you otherwise think I should remove it, let
me know and I'll re-submit a modified patch.

> > > This problem occurs when performing the first pass of a 2-pass encode, as
> > > otherwise, the fifo is properly drained, preventing it from overflowing.
> > >
> > > Problem is fixed by manually draining the fifo when performing the first
> > > pass
> > > of a 2-pass encode.
> > >
> > > Fixes the regression originally introduced in
> > > 5bda4ec6c3cb6f286bb40dee4457c3c26e0f78cb
> > >
> >
> > Anton, any comments here?
> >
> > > Signed-off-by: David Lemler <david at lemler.family>
> > > ---
> > >  libavcodec/libvpxenc.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> > > index 8833df2d68..e4ab13e6ab 100644
> > > --- a/libavcodec/libvpxenc.c
> > > +++ b/libavcodec/libvpxenc.c
> > > @@ -1452,6 +1452,12 @@ static int queue_frames(AVCodecContext *avctx, struct
> > > vpx_codec_ctx *encoder,
> > >              memcpy((uint8_t*)stats->buf + stats->sz,
> > >                     pkt->data.twopass_stats.buf,
> > > pkt->data.twopass_stats.sz);
> > >              stats->sz += pkt->data.twopass_stats.sz;
> > > +            /* Drain the fifo if it has any items in it to prevent it from
> > > +               filling up when performing the first pass of a 2-pass encode
> > > if
> > > +               the video has more than 21845 frames. */
> > > +            if (av_fifo_can_read(ctx->fifo) > 0) {
> >
> > Another option would be to avoid calling frame_data_submit() in the first pass.
> >
> > > +                av_fifo_drain2(ctx->fifo, 1);
> > > +            }
> > >              break;
> > >          }
> > >          case VPX_CODEC_PSNR_PKT:
> > > --
> > > 2.41.0.windows.1
> > >