diff mbox series

[FFmpeg-devel,4/5] avcodec/videotoolboxenc: Fix concurrent access to CVPixelBufferRef

Message ID tencent_D6931D5C700BBB661750A3A6E727FEEA8006@qq.com
State New
Headers show
Series [FFmpeg-devel,1/5] avcodec/videotoolboxenc: Don't ignore ENOMEM | expand

Checks

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

Commit Message

Zhao Zhili July 7, 2024, 10:21 a.m. UTC 
From: Zhao Zhili <zhilizhao@tencent.com>

For a frame comes from AV_HWDEVICE_TYPE_VIDEOTOOLBOX, it's
CVPixelBufferRef is maintained by a pool. CVPixelBufferRef returned
to the pool when frame buffer reference reached to zero. However,
VTCompressionSessionEncodeFrame also hold a reference to the
CVPixelBufferRef. So a new frame get from av_hwframe_get_buffer
may access a CVPixelBufferRef which still used by the encoder.
It's only after vtenc_output_callback that we can make sure
CVPixelBufferRef has been released by the encoder.

The issue can be tested with sample from trac #10884.
ffmpeg -hwaccel videotoolbox \
	-hwaccel_output_format videotoolbox_vld \
        -i input.mp4 \
	-c:v hevc_videotoolbox \
	-profile:v main \
        -b:v 3M \
        -vf scale_vt=w=iw/2:h=ih/2:color_matrix=bt709:color_primaries=bt709:color_transfer=bt709 \
        -c:a copy \
	-tag:v hvc1 \
	output.mp4

Withtout the patch, there are some out of order images in output.mp4.
---
 libavcodec/videotoolboxenc.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)
diff mbox series

Patch

diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
index 213bfa8b49..9bb9b0b457 100644
--- a/libavcodec/videotoolboxenc.c
+++ b/libavcodec/videotoolboxenc.c
@@ -228,6 +228,7 @@  typedef struct ExtraSEI {
 typedef struct BufNode {
     CMSampleBufferRef cm_buffer;
     ExtraSEI *sei;
+    AVBufferRef *frame_buf;
     struct BufNode* next;
 } BufNode;
 
@@ -727,6 +728,7 @@  static void vtenc_free_buf_node(BufNode *info)
     if (info->cm_buffer)
         CFRelease(info->cm_buffer);
 
+    av_buffer_unref(&info->frame_buf);
     av_free(info);
 }
 
@@ -741,6 +743,7 @@  static void vtenc_output_callback(
     VTEncContext   *vtctx = avctx->priv_data;
     BufNode *info = sourceFrameCtx;
 
+    av_buffer_unref(&info->frame_buf);
     if (vtctx->async_error) {
         vtenc_free_buf_node(info);
         return;
@@ -2459,7 +2462,8 @@  static int copy_avframe_to_pixel_buffer(AVCodecContext   *avctx,
 
 static int create_cv_pixel_buffer(AVCodecContext   *avctx,
                                   const AVFrame    *frame,
-                                  CVPixelBufferRef *cv_img)
+                                  CVPixelBufferRef *cv_img,
+                                  BufNode          *node)
 {
     int plane_count;
     int color;
@@ -2478,6 +2482,12 @@  static int create_cv_pixel_buffer(AVCodecContext   *avctx,
         av_assert0(*cv_img);
 
         CFRetain(*cv_img);
+        if (frame->buf[0]) {
+            node->frame_buf = av_buffer_ref(frame->buf[0]);
+            if (!node->frame_buf)
+                return AVERROR(ENOMEM);
+        }
+
         return 0;
     }
 
@@ -2585,7 +2595,7 @@  static int vtenc_send_frame(AVCodecContext *avctx,
     if (!node)
         return AVERROR(ENOMEM);
 
-    status = create_cv_pixel_buffer(avctx, frame, &cv_img);
+    status = create_cv_pixel_buffer(avctx, frame, &cv_img, node);
     if (status)
         goto out;