[FFmpeg-devel] [PATCH] amfenc: Use a blocking call instead of sleeping and polling

[Evgeny Pavlov]

On Wed, Oct 18, 2023 at 10:36 PM Mark Thompson <sw at jkqxz.net> wrote:

> ---
> On 17/10/2023 18:11, Evgeny Pavlov wrote:
> > The reason for using av_usleep() here is that AMF API doesn’t provide an
> > API for explicit wait. There are two modes to get output from encoder:
> >
> > 1. Polling with some sleep to avoid CPU thrashing – currently used in
> FFmpeg
> >
> > 2. Set timeout parameter on AMF encoder and QueryOutput call will block
> > till output is available or the timeout happens.
> >
> > #2 is the preferable way but it is designed more to be used with a
> separate
> > polling thread. With a single-thread approach in FFmpeg, the use of
> timeout
> > can block input submission making things slower.  This is even more
> > pronounced when B-frames are enabled and several inputs are needed to
> produce
> > the first output.
>
> This approach seems like it should work here?  Run non-blocking until the
> queue is full, then switch to blocking when you need to wait for some
> output.
>
> I tried the patch enclosing (H.264 only, different proprties needed for
> other codecs), but it doesn't seem to work - the test assert always hits
> immediately and timing shows that QueryOutput didn't block even though the
> timeout should be set?  I'm probably doing something incorrect, maybe you
> would know how to fix it.
>
> > The condition of this sleep is in special events (primarily when amf
> input
> > queue is full), not the core loop part. During the experiments the cpu
> > increasing is about 2-4% or so, not a burst.
>
> What cases are you experimenting with?
>
> The most problematic case I can think of is multiple encodes running
> simultaneously sharing the same instance so that each one has to wait for
> others to complete and therefore all queues fill up.
>
> The busy wait will end up being the only place where it can block (since
> everything else runs asynchronously), so you will peg one CPU at close to
> 100% per encode running.
>
> Thanks,
>
> - Mark
>
>   libavcodec/amfenc.c | 22 +++++++++++++++++++---
>   libavcodec/amfenc.h |  1 +
>   2 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
> index 061859f85c..db7ddbb083 100644
> --- a/libavcodec/amfenc.c
> +++ b/libavcodec/amfenc.c
> @@ -713,13 +713,22 @@ int ff_amf_receive_packet(AVCodecContext *avctx,
> AVPacket *avpkt)
>           }
>       }
>
> -
> +    block_and_wait = 0;
>       do {
> -        block_and_wait = 0;
>           // poll data
>           if (!avpkt->data && !avpkt->buf) {
> +            int64_t timeout = block_and_wait ? 100 : 0;
> +            if (timeout != ctx->output_query_timeout) {
> +                av_log(avctx, AV_LOG_INFO, "Set output query timeout to
> %"PRId64"\n", timeout);
> +                AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_QUERY_TIMEOUT, timeout);
> +                AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN,
> "Failed to set output query timeout\n");
> +                ctx->output_query_timeout = timeout;
> +            }
> +
>               res_query = ctx->encoder->pVtbl->QueryOutput(ctx->encoder,
> &data);
>               if (data) {
> +                av_log(avctx, AV_LOG_INFO, "QueryOutput returned with
> data\n");
> +
>                   // copy data to packet
>                   AMFBuffer *buffer;
>                   AMFGuid guid = IID_AMFBuffer();
> @@ -740,7 +749,13 @@ int ff_amf_receive_packet(AVCodecContext *avctx,
> AVPacket *avpkt)
>                   data->pVtbl->Release(data);
>
>                   AMF_RETURN_IF_FALSE(ctx, ret >= 0, ret,
> "amf_copy_buffer() failed with error %d\n", ret);
> +            } else {
> +                av_log(avctx, AV_LOG_INFO, "QueryOutput returned with
> nothing (%d)\n", res_query);
> +                // For testing, shouldn't hit this unless machine is
> otherwise very loaded.
> +                av_assert0(!block_and_wait);
>               }
> +
> +            block_and_wait = 0;
>           }
>           res_resubmit = AMF_OK;
>           if (ctx->delayed_surface != NULL) { // try to resubmit frame
> @@ -769,8 +784,9 @@ int ff_amf_receive_packet(AVCodecContext *avctx,
> AVPacket *avpkt)
>
>           if (query_output_data_flag == 0) {
>               if (res_resubmit == AMF_INPUT_FULL || ctx->delayed_drain ||
> (ctx->eof && res_query != AMF_EOF) || (ctx->hwsurfaces_in_queue >=
> ctx->hwsurfaces_in_queue_max)) {
> +                av_log(avctx, AV_LOG_INFO, "Need to wait for output\n");
>                   block_and_wait = 1;
> -                av_usleep(1000);
> +                //av_usleep(1000);
>               }
>           }
>       } while (block_and_wait);
> diff --git a/libavcodec/amfenc.h b/libavcodec/amfenc.h
> index 2dbd378ef8..64c77115b6 100644
> --- a/libavcodec/amfenc.h
> +++ b/libavcodec/amfenc.h
> @@ -72,6 +72,7 @@ typedef struct AmfContext {
>       int                 delayed_drain;
>       AMFSurface         *delayed_surface;
>       AVFrame            *delayed_frame;
> +    int64_t             output_query_timeout;
>
>       // shift dts back by max_b_frames in timing
>       AVFifo             *timestamp_list;
> --
> 2.39.2
> _______________________________________________
> 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".
>

Dynamic switching between non-blocking & blocking approaches isn’t
supported in AMF at this time.

We might request to implement this feature for AMF team, but it might took
some time to implement this.

I would suggest using av_usleep(500) until this feature is implemented.

> What cases are you experimenting with?

This issue is very easy to reproduce when:

1) low resolution transcoding

2) hardware accelerated decoding

The command line sample:  ffmpeg -hwaccel d3d11va -hwaccel_output_format
d3d11 -i input_480x360_h264.mp4 -c:v hevc_amf  output_480x360_hevc.mp4