diff mbox series

[FFmpeg-devel] lavc/vaapi_encode: fix propagating durations and opaques

Message ID 20230131131447.24732-1-anton@khirnov.net
State Accepted
Commit 7d49fef8b426041e22fde77771b435d9411241a0
Headers show
Series [FFmpeg-devel] lavc/vaapi_encode: fix propagating durations and opaques | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Anton Khirnov Jan. 31, 2023, 1:14 p.m. UTC 
input_image is freed by the time the output packet is constructed, so we
need to store copies in VAAPIEncodePicture.
---
Is this better?
---
 libavcodec/vaapi_encode.c | 22 +++++++++++++++++-----
 libavcodec/vaapi_encode.h |  4 ++++
 2 files changed, 21 insertions(+), 5 deletions(-)

Comments

Xiang, Haihao Feb. 1, 2023, 2:59 a.m. UTC | #1 
On Di, 2023-01-31 at 14:14 +0100, Anton Khirnov wrote:
> input_image is freed by the time the output packet is constructed, so we
> need to store copies in VAAPIEncodePicture.
> ---
> Is this better?
> ---
>  libavcodec/vaapi_encode.c | 22 +++++++++++++++++-----
>  libavcodec/vaapi_encode.h |  4 ++++
>  2 files changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> index 6787b90e8d6..bfca315a7ad 100644
> --- a/libavcodec/vaapi_encode.c
> +++ b/libavcodec/vaapi_encode.c
> @@ -695,7 +695,7 @@ static int vaapi_encode_output(AVCodecContext *avctx,
>          pkt->flags |= AV_PKT_FLAG_KEY;
>  
>      pkt->pts = pic->pts;
> -    pkt->duration = pic->input_image->duration;
> +    pkt->duration = pic->duration;
>  
>      vas = vaUnmapBuffer(ctx->hwctx->display, pic->output_buffer);
>      if (vas != VA_STATUS_SUCCESS) {
> @@ -706,10 +706,11 @@ static int vaapi_encode_output(AVCodecContext *avctx,
>      }
>  
>      // for no-delay encoders this is handled in generic codec
> -    if (avctx->codec->capabilities & AV_CODEC_CAP_DELAY) {
> -        err = ff_encode_reordered_opaque(avctx, pkt, pic->input_image);
> -        if (err < 0)
> -            goto fail;
> +    if (avctx->codec->capabilities & AV_CODEC_CAP_DELAY &&
> +        avctx->flags & AV_CODEC_FLAG_COPY_OPAQUE) {
> +        pkt->opaque     = pic->opaque;
> +        pkt->opaque_ref = pic->opaque_ref;
> +        pic->opaque_ref = NULL;
>      }
>  
>      av_buffer_unref(&pic->output_buffer_ref);
> @@ -785,6 +786,8 @@ static int vaapi_encode_free(AVCodecContext *avctx,
>      av_frame_free(&pic->input_image);
>      av_frame_free(&pic->recon_image);
>  
> +    av_buffer_unref(&pic->opaque_ref);
> +
>      av_freep(&pic->param_buffers);
>      av_freep(&pic->slices);
>      // Output buffer should already be destroyed.
> @@ -1152,6 +1155,15 @@ static int vaapi_encode_send_frame(AVCodecContext
> *avctx, AVFrame *frame)
>  
>          pic->input_surface = (VASurfaceID)(uintptr_t)frame->data[3];
>          pic->pts = frame->pts;
> +        pic->duration = frame->duration;
> +
> +        if (avctx->flags & AV_CODEC_FLAG_COPY_OPAQUE) {
> +            err = av_buffer_replace(&pic->opaque_ref, frame->opaque_ref);
> +            if (err < 0)
> +                goto fail;
> +
> +            pic->opaque = frame->opaque;
> +        }
>  
>          av_frame_move_ref(pic->input_image, frame);
>  
> diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
> index 359f954ffff..a1e639f56b0 100644
> --- a/libavcodec/vaapi_encode.h
> +++ b/libavcodec/vaapi_encode.h
> @@ -75,8 +75,12 @@ typedef struct VAAPIEncodePicture {
>      int64_t         display_order;
>      int64_t         encode_order;
>      int64_t         pts;
> +    int64_t         duration;
>      int             force_idr;
>  
> +    void           *opaque;
> +    AVBufferRef    *opaque_ref;
> +
>  #if VA_CHECK_VERSION(1, 0, 0)
>      // ROI regions.
>      VAEncROI       *roi;

LGTM and the command for transcoding with vaapi works well now. 

BRs
Haihao
Xiang, Haihao Feb. 2, 2023, 2:47 a.m. UTC | #2 
On Wo, 2023-02-01 at 02:59 +0000, Xiang, Haihao wrote:
> On Di, 2023-01-31 at 14:14 +0100, Anton Khirnov wrote:
> > input_image is freed by the time the output packet is constructed, so we
> > need to store copies in VAAPIEncodePicture.
> > ---
> > Is this better?
> > ---
> >  libavcodec/vaapi_encode.c | 22 +++++++++++++++++-----
> >  libavcodec/vaapi_encode.h |  4 ++++
> >  2 files changed, 21 insertions(+), 5 deletions(-)
> > 
> > diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> > index 6787b90e8d6..bfca315a7ad 100644
> > --- a/libavcodec/vaapi_encode.c
> > +++ b/libavcodec/vaapi_encode.c
> > @@ -695,7 +695,7 @@ static int vaapi_encode_output(AVCodecContext *avctx,
> >          pkt->flags |= AV_PKT_FLAG_KEY;
> >  
> >      pkt->pts = pic->pts;
> > -    pkt->duration = pic->input_image->duration;
> > +    pkt->duration = pic->duration;
> >  
> >      vas = vaUnmapBuffer(ctx->hwctx->display, pic->output_buffer);
> >      if (vas != VA_STATUS_SUCCESS) {
> > @@ -706,10 +706,11 @@ static int vaapi_encode_output(AVCodecContext *avctx,
> >      }
> >  
> >      // for no-delay encoders this is handled in generic codec
> > -    if (avctx->codec->capabilities & AV_CODEC_CAP_DELAY) {
> > -        err = ff_encode_reordered_opaque(avctx, pkt, pic->input_image);
> > -        if (err < 0)
> > -            goto fail;
> > +    if (avctx->codec->capabilities & AV_CODEC_CAP_DELAY &&
> > +        avctx->flags & AV_CODEC_FLAG_COPY_OPAQUE) {
> > +        pkt->opaque     = pic->opaque;
> > +        pkt->opaque_ref = pic->opaque_ref;
> > +        pic->opaque_ref = NULL;
> >      }
> >  
> >      av_buffer_unref(&pic->output_buffer_ref);
> > @@ -785,6 +786,8 @@ static int vaapi_encode_free(AVCodecContext *avctx,
> >      av_frame_free(&pic->input_image);
> >      av_frame_free(&pic->recon_image);
> >  
> > +    av_buffer_unref(&pic->opaque_ref);
> > +
> >      av_freep(&pic->param_buffers);
> >      av_freep(&pic->slices);
> >      // Output buffer should already be destroyed.
> > @@ -1152,6 +1155,15 @@ static int vaapi_encode_send_frame(AVCodecContext
> > *avctx, AVFrame *frame)
> >  
> >          pic->input_surface = (VASurfaceID)(uintptr_t)frame->data[3];
> >          pic->pts = frame->pts;
> > +        pic->duration = frame->duration;
> > +
> > +        if (avctx->flags & AV_CODEC_FLAG_COPY_OPAQUE) {
> > +            err = av_buffer_replace(&pic->opaque_ref, frame->opaque_ref);
> > +            if (err < 0)
> > +                goto fail;
> > +
> > +            pic->opaque = frame->opaque;
> > +        }
> >  
> >          av_frame_move_ref(pic->input_image, frame);
> >  
> > diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
> > index 359f954ffff..a1e639f56b0 100644
> > --- a/libavcodec/vaapi_encode.h
> > +++ b/libavcodec/vaapi_encode.h
> > @@ -75,8 +75,12 @@ typedef struct VAAPIEncodePicture {
> >      int64_t         display_order;
> >      int64_t         encode_order;
> >      int64_t         pts;
> > +    int64_t         duration;
> >      int             force_idr;
> >  
> > +    void           *opaque;
> > +    AVBufferRef    *opaque_ref;
> > +
> >  #if VA_CHECK_VERSION(1, 0, 0)
> >      // ROI regions.
> >      VAEncROI       *roi;
> 
> LGTM and the command for transcoding with vaapi works well now. 
> 

Pushed because this patch fixed critical regression in vaapi path.

Thanks
Haihao
diff mbox series

Patch

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 6787b90e8d6..bfca315a7ad 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -695,7 +695,7 @@  static int vaapi_encode_output(AVCodecContext *avctx,
         pkt->flags |= AV_PKT_FLAG_KEY;
 
     pkt->pts = pic->pts;
-    pkt->duration = pic->input_image->duration;
+    pkt->duration = pic->duration;
 
     vas = vaUnmapBuffer(ctx->hwctx->display, pic->output_buffer);
     if (vas != VA_STATUS_SUCCESS) {
@@ -706,10 +706,11 @@  static int vaapi_encode_output(AVCodecContext *avctx,
     }
 
     // for no-delay encoders this is handled in generic codec
-    if (avctx->codec->capabilities & AV_CODEC_CAP_DELAY) {
-        err = ff_encode_reordered_opaque(avctx, pkt, pic->input_image);
-        if (err < 0)
-            goto fail;
+    if (avctx->codec->capabilities & AV_CODEC_CAP_DELAY &&
+        avctx->flags & AV_CODEC_FLAG_COPY_OPAQUE) {
+        pkt->opaque     = pic->opaque;
+        pkt->opaque_ref = pic->opaque_ref;
+        pic->opaque_ref = NULL;
     }
 
     av_buffer_unref(&pic->output_buffer_ref);
@@ -785,6 +786,8 @@  static int vaapi_encode_free(AVCodecContext *avctx,
     av_frame_free(&pic->input_image);
     av_frame_free(&pic->recon_image);
 
+    av_buffer_unref(&pic->opaque_ref);
+
     av_freep(&pic->param_buffers);
     av_freep(&pic->slices);
     // Output buffer should already be destroyed.
@@ -1152,6 +1155,15 @@  static int vaapi_encode_send_frame(AVCodecContext *avctx, AVFrame *frame)
 
         pic->input_surface = (VASurfaceID)(uintptr_t)frame->data[3];
         pic->pts = frame->pts;
+        pic->duration = frame->duration;
+
+        if (avctx->flags & AV_CODEC_FLAG_COPY_OPAQUE) {
+            err = av_buffer_replace(&pic->opaque_ref, frame->opaque_ref);
+            if (err < 0)
+                goto fail;
+
+            pic->opaque = frame->opaque;
+        }
 
         av_frame_move_ref(pic->input_image, frame);
 
diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
index 359f954ffff..a1e639f56b0 100644
--- a/libavcodec/vaapi_encode.h
+++ b/libavcodec/vaapi_encode.h
@@ -75,8 +75,12 @@  typedef struct VAAPIEncodePicture {
     int64_t         display_order;
     int64_t         encode_order;
     int64_t         pts;
+    int64_t         duration;
     int             force_idr;
 
+    void           *opaque;
+    AVBufferRef    *opaque_ref;
+
 #if VA_CHECK_VERSION(1, 0, 0)
     // ROI regions.
     VAEncROI       *roi;