[FFmpeg-devel] [PATCH V3 2/3] libavcodec/vaapi_encode: Change the way to call async to increase performance

[Xiang, Haihao]

On Tue, 2022-02-08 at 11:05 +0800, Wenbin Chen wrote:
> Fix: #7706. After commit 5fdcf85bbffe7451c2, vaapi encoder's performance
> decrease. The reason is that vaRenderPicture() and vaSyncBuffer() are
> called at the same time (vaRenderPicture() always followed by a
> vaSyncBuffer()). When we encode stream with B frames, we need buffer to
> reorder frames, so we can send serveral frames to HW at once to increase
> performance. Now I changed them to be called in a asynchronous way, which
> will make better use of hardware. 1080p transcoding increases about 17%
> fps on my environment.
> 
> This change fits vaSyncBuffer(), so if driver does not support
> vaSyncBuffer, it will keep previous operation.
> 
> Signed-off-by: Wenbin Chen <wenbin.chen at intel.com>
> ---
>  libavcodec/vaapi_encode.c | 64 ++++++++++++++++++++++++++++++++-------
>  libavcodec/vaapi_encode.h |  5 +++
>  2 files changed, 58 insertions(+), 11 deletions(-)
> 
> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> index b87b58a42b..15ddbbaa4a 100644
> --- a/libavcodec/vaapi_encode.c
> +++ b/libavcodec/vaapi_encode.c
> @@ -984,8 +984,10 @@ static int vaapi_encode_pick_next(AVCodecContext *avctx,
>      if (!pic && ctx->end_of_stream) {
>          --b_counter;
>          pic = ctx->pic_end;
> -        if (pic->encode_issued)
> +        if (pic->encode_complete)
>              return AVERROR_EOF;
> +        else if (pic->encode_issued)
> +            return AVERROR(EAGAIN);
>      }
>  
>      if (!pic) {
> @@ -1210,18 +1212,44 @@ int ff_vaapi_encode_receive_packet(AVCodecContext
> *avctx, AVPacket *pkt)
>              return AVERROR(EAGAIN);
>      }
>  
> -    pic = NULL;
> -    err = vaapi_encode_pick_next(avctx, &pic);
> -    if (err < 0)
> -        return err;
> -    av_assert0(pic);
> +#if VA_CHECK_VERSION(1, 9, 0)

Needn't check the version at compile time because vaSyncBuffer is not called
directly in the code below.

> +    if (ctx->has_sync_buffer_func) {
> +        while (av_fifo_can_read(ctx->encode_fifo) <= MAX_PICTURE_REFERENCES)
> {
> +            pic = NULL;
> +            err = vaapi_encode_pick_next(avctx, &pic);
> +            if (err < 0)
> +                break;
> +
> +            av_assert0(pic);
> +            pic->encode_order = ctx->encode_order +
> +                                av_fifo_can_read(ctx->encode_fifo);
> +            err = vaapi_encode_issue(avctx, pic);
> +            if (err < 0) {
> +                av_log(avctx, AV_LOG_ERROR, "Encode failed: %d.\n", err);
> +                return err;
> +            }
> +            av_fifo_write(ctx->encode_fifo, &pic, 1);
> +        }
> +        if (!av_fifo_can_read(ctx->encode_fifo))
> +            return err;
> +        av_fifo_read(ctx->encode_fifo, &pic, 1);
> +        ctx->encode_order = pic->encode_order + 1;
> +    } else
> +#endif
> +    {
> +        pic = NULL;
> +        err = vaapi_encode_pick_next(avctx, &pic);
> +        if (err < 0)
> +            return err;
> +        av_assert0(pic);
>  
> -    pic->encode_order = ctx->encode_order++;
> +        pic->encode_order = ctx->encode_order++;
>  
> -    err = vaapi_encode_issue(avctx, pic);
> -    if (err < 0) {
> -        av_log(avctx, AV_LOG_ERROR, "Encode failed: %d.\n", err);
> -        return err;
> +        err = vaapi_encode_issue(avctx, pic);
> +        if (err < 0) {
> +            av_log(avctx, AV_LOG_ERROR, "Encode failed: %d.\n", err);
> +            return err;
> +        }
>      }
>  
>      err = vaapi_encode_output(avctx, pic, pkt);
> @@ -2555,6 +2583,19 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx)
>          }
>      }
>  
> +#if VA_CHECK_VERSION(1, 9, 0)
> +    //check vaSyncBuffer function
> +    vas = vaSyncBuffer(ctx->hwctx->display, 0, 0);

Buf id 0 (the 2nd parameter) might be valid, however we needn't synchronize with
a real buf here, we may use VA_INVALID_ID instead

> +    if (vas != VA_STATUS_ERROR_UNIMPLEMENTED) {
> +        ctx->has_sync_buffer_func = 1;
> +        ctx->encode_fifo = av_fifo_alloc2(MAX_PICTURE_REFERENCES + 1,
> +                                          sizeof(VAAPIEncodePicture *),
> +                                          0);
> +        if (!ctx->encode_fifo)
> +            return AVERROR(ENOMEM);
> +    }
> +#endif
> +
>      return 0;
>  
>  fail:
> @@ -2592,6 +2633,7 @@ av_cold int ff_vaapi_encode_close(AVCodecContext *avctx)
>  
>      av_freep(&ctx->codec_sequence_params);
>      av_freep(&ctx->codec_picture_params);
> +    av_fifo_freep2(&ctx->encode_fifo);
>  
>      av_buffer_unref(&ctx->recon_frames_ref);
>      av_buffer_unref(&ctx->input_frames_ref);
> diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
> index b41604a883..d33a486cb8 100644
> --- a/libavcodec/vaapi_encode.h
> +++ b/libavcodec/vaapi_encode.h
> @@ -29,6 +29,7 @@
>  
>  #include "libavutil/hwcontext.h"
>  #include "libavutil/hwcontext_vaapi.h"
> +#include "libavutil/fifo.h"
>  
>  #include "avcodec.h"
>  #include "hwconfig.h"
> @@ -345,6 +346,10 @@ typedef struct VAAPIEncodeContext {
>      int             roi_warned;
>  
>      AVFrame         *frame;
> +    //Store buffered pic

Better to keep the coding style unchanged, always put one space between // and
your comment. 

> +    AVFifo *encode_fifo;
> +    //Whether the driver support vaSyncBuffer
> +    int has_sync_buffer_func;

How about to check whether the driver support vaSyncBuffer in the first patch ?

Thanks
Haihao

>  } VAAPIEncodeContext;
>  
>  enum {