diff mbox series

[FFmpeg-devel,1/2] avutil/hwcontext_videotoolbox: create real buffer pool

Message ID tencent_1F796B1A7DC6AD2A3845C0B9B53C683A000A@qq.com
State Accepted
Commit c3f00daa995e7438f57aba7ec2367f02345bc223
Headers show
Series [FFmpeg-devel,1/2] avutil/hwcontext_videotoolbox: create real buffer pool | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished

Commit Message

zhilizhao(赵志立) March 10, 2022, 4:37 a.m. UTC
vt_get_buffer shouldn't do buffer pool's job.
---
 libavutil/hwcontext_videotoolbox.c | 71 ++++++++++++++----------------
 1 file changed, 34 insertions(+), 37 deletions(-)

Comments

zhilizhao(赵志立) March 31, 2022, 6:48 a.m. UTC | #1
Ping for patch 1/2. rcombs has reviewed the patch on IRC. I decided to
drop patch 2/2.


> 11:05 rcombs: quink_: seems reasonable to me
> 11:06 quink_: rcombs: thanks : )
> 11:06 rcombs: not entirely sure what the deal with the second commit is but ¯\_(ツ)_/¯ it's harmless so w/e


> On Mar 10, 2022, at 12:37 PM, Zhao Zhili <quinkblack@foxmail.com> wrote:
> 
> vt_get_buffer shouldn't do buffer pool's job.
> ---
> libavutil/hwcontext_videotoolbox.c | 71 ++++++++++++++----------------
> 1 file changed, 34 insertions(+), 37 deletions(-)
> 
> diff --git a/libavutil/hwcontext_videotoolbox.c b/libavutil/hwcontext_videotoolbox.c
> index 026127d412..e442a95007 100644
> --- a/libavutil/hwcontext_videotoolbox.c
> +++ b/libavutil/hwcontext_videotoolbox.c
> @@ -210,9 +210,36 @@ static int vt_pool_alloc(AVHWFramesContext *ctx)
>     return AVERROR_EXTERNAL;
> }
> 
> -static AVBufferRef *vt_dummy_pool_alloc(void *opaque, size_t size)
> +static void videotoolbox_buffer_release(void *opaque, uint8_t *data)
> +{
> +    CVPixelBufferRelease((CVPixelBufferRef)data);
> +}
> +
> +static AVBufferRef *vt_pool_alloc_buffer(void *opaque, size_t size)
> {
> -    return NULL;
> +    CVPixelBufferRef pixbuf;
> +    AVBufferRef *buf;
> +    CVReturn err;
> +    AVHWFramesContext *ctx = opaque;
> +    VTFramesContext *fctx = ctx->internal->priv;
> +
> +    err = CVPixelBufferPoolCreatePixelBuffer(
> +        NULL,
> +        fctx->pool,
> +        &pixbuf
> +    );
> +    if (err != kCVReturnSuccess) {
> +        av_log(ctx, AV_LOG_ERROR, "Failed to create pixel buffer from pool: %d\n", err);
> +        return NULL;
> +    }
> +
> +    buf = av_buffer_create((uint8_t *)pixbuf, size,
> +                           videotoolbox_buffer_release, NULL, 0);
> +    if (!buf) {
> +        CVPixelBufferRelease(pixbuf);
> +        return NULL;
> +    }
> +    return buf;
> }
> 
> static void vt_frames_uninit(AVHWFramesContext *ctx)
> @@ -238,9 +265,9 @@ static int vt_frames_init(AVHWFramesContext *ctx)
>         return AVERROR(ENOSYS);
>     }
> 
> -    // create a dummy pool so av_hwframe_get_buffer doesn't EINVAL
>     if (!ctx->pool) {
> -        ctx->internal->pool_internal = av_buffer_pool_init2(0, ctx, vt_dummy_pool_alloc, NULL);
> +        ctx->internal->pool_internal = av_buffer_pool_init2(
> +                sizeof(CVPixelBufferRef), ctx, vt_pool_alloc_buffer, NULL);
>         if (!ctx->internal->pool_internal)
>             return AVERROR(ENOMEM);
>     }
> @@ -252,41 +279,11 @@ static int vt_frames_init(AVHWFramesContext *ctx)
>     return 0;
> }
> 
> -static void videotoolbox_buffer_release(void *opaque, uint8_t *data)
> -{
> -    CVPixelBufferRelease((CVPixelBufferRef)data);
> -}
> -
> static int vt_get_buffer(AVHWFramesContext *ctx, AVFrame *frame)
> {
> -    VTFramesContext *fctx = ctx->internal->priv;
> -
> -    if (ctx->pool && ctx->pool->size != 0) {
> -        frame->buf[0] = av_buffer_pool_get(ctx->pool);
> -        if (!frame->buf[0])
> -            return AVERROR(ENOMEM);
> -    } else {
> -        CVPixelBufferRef pixbuf;
> -        AVBufferRef *buf = NULL;
> -        CVReturn err;
> -
> -        err = CVPixelBufferPoolCreatePixelBuffer(
> -            NULL,
> -            fctx->pool,
> -            &pixbuf
> -        );
> -        if (err != kCVReturnSuccess) {
> -            av_log(ctx, AV_LOG_ERROR, "Failed to create pixel buffer from pool: %d\n", err);
> -            return AVERROR_EXTERNAL;
> -        }
> -
> -        buf = av_buffer_create((uint8_t *)pixbuf, 1, videotoolbox_buffer_release, NULL, 0);
> -        if (!buf) {
> -            CVPixelBufferRelease(pixbuf);
> -            return AVERROR(ENOMEM);
> -        }
> -        frame->buf[0] = buf;
> -    }
> +    frame->buf[0] = av_buffer_pool_get(ctx->pool);
> +    if (!frame->buf[0])
> +        return AVERROR(ENOMEM);
> 
>     frame->data[3] = frame->buf[0]->data;
>     frame->format  = AV_PIX_FMT_VIDEOTOOLBOX;
> -- 
> 2.31.1
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
zhilizhao(赵志立) April 25, 2022, 10:16 a.m. UTC | #2
> On Mar 31, 2022, at 2:48 PM, zhilizhao(赵志立) <quinkblack@foxmail.com> wrote:
> 
> Ping for patch 1/2. rcombs has reviewed the patch on IRC. I decided to
> drop patch 2/2.
> 
> 
>> 11:05 rcombs: quink_: seems reasonable to me
>> 11:06 quink_: rcombs: thanks : )
>> 11:06 rcombs: not entirely sure what the deal with the second commit is but ¯\_(ツ)_/¯ it's harmless so w/e
> 
> 
>> On Mar 10, 2022, at 12:37 PM, Zhao Zhili <quinkblack@foxmail.com> wrote:
>> 
>> vt_get_buffer shouldn't do buffer pool's job.
>> ---
>> libavutil/hwcontext_videotoolbox.c | 71 ++++++++++++++----------------
>> 1 file changed, 34 insertions(+), 37 deletions(-)
>> 
>> diff --git a/libavutil/hwcontext_videotoolbox.c b/libavutil/hwcontext_videotoolbox.c
>> index 026127d412..e442a95007 100644
>> --- a/libavutil/hwcontext_videotoolbox.c
>> +++ b/libavutil/hwcontext_videotoolbox.c

No functional change. Will apply this week unless there are objections.
diff mbox series

Patch

diff --git a/libavutil/hwcontext_videotoolbox.c b/libavutil/hwcontext_videotoolbox.c
index 026127d412..e442a95007 100644
--- a/libavutil/hwcontext_videotoolbox.c
+++ b/libavutil/hwcontext_videotoolbox.c
@@ -210,9 +210,36 @@  static int vt_pool_alloc(AVHWFramesContext *ctx)
     return AVERROR_EXTERNAL;
 }
 
-static AVBufferRef *vt_dummy_pool_alloc(void *opaque, size_t size)
+static void videotoolbox_buffer_release(void *opaque, uint8_t *data)
+{
+    CVPixelBufferRelease((CVPixelBufferRef)data);
+}
+
+static AVBufferRef *vt_pool_alloc_buffer(void *opaque, size_t size)
 {
-    return NULL;
+    CVPixelBufferRef pixbuf;
+    AVBufferRef *buf;
+    CVReturn err;
+    AVHWFramesContext *ctx = opaque;
+    VTFramesContext *fctx = ctx->internal->priv;
+
+    err = CVPixelBufferPoolCreatePixelBuffer(
+        NULL,
+        fctx->pool,
+        &pixbuf
+    );
+    if (err != kCVReturnSuccess) {
+        av_log(ctx, AV_LOG_ERROR, "Failed to create pixel buffer from pool: %d\n", err);
+        return NULL;
+    }
+
+    buf = av_buffer_create((uint8_t *)pixbuf, size,
+                           videotoolbox_buffer_release, NULL, 0);
+    if (!buf) {
+        CVPixelBufferRelease(pixbuf);
+        return NULL;
+    }
+    return buf;
 }
 
 static void vt_frames_uninit(AVHWFramesContext *ctx)
@@ -238,9 +265,9 @@  static int vt_frames_init(AVHWFramesContext *ctx)
         return AVERROR(ENOSYS);
     }
 
-    // create a dummy pool so av_hwframe_get_buffer doesn't EINVAL
     if (!ctx->pool) {
-        ctx->internal->pool_internal = av_buffer_pool_init2(0, ctx, vt_dummy_pool_alloc, NULL);
+        ctx->internal->pool_internal = av_buffer_pool_init2(
+                sizeof(CVPixelBufferRef), ctx, vt_pool_alloc_buffer, NULL);
         if (!ctx->internal->pool_internal)
             return AVERROR(ENOMEM);
     }
@@ -252,41 +279,11 @@  static int vt_frames_init(AVHWFramesContext *ctx)
     return 0;
 }
 
-static void videotoolbox_buffer_release(void *opaque, uint8_t *data)
-{
-    CVPixelBufferRelease((CVPixelBufferRef)data);
-}
-
 static int vt_get_buffer(AVHWFramesContext *ctx, AVFrame *frame)
 {
-    VTFramesContext *fctx = ctx->internal->priv;
-
-    if (ctx->pool && ctx->pool->size != 0) {
-        frame->buf[0] = av_buffer_pool_get(ctx->pool);
-        if (!frame->buf[0])
-            return AVERROR(ENOMEM);
-    } else {
-        CVPixelBufferRef pixbuf;
-        AVBufferRef *buf = NULL;
-        CVReturn err;
-
-        err = CVPixelBufferPoolCreatePixelBuffer(
-            NULL,
-            fctx->pool,
-            &pixbuf
-        );
-        if (err != kCVReturnSuccess) {
-            av_log(ctx, AV_LOG_ERROR, "Failed to create pixel buffer from pool: %d\n", err);
-            return AVERROR_EXTERNAL;
-        }
-
-        buf = av_buffer_create((uint8_t *)pixbuf, 1, videotoolbox_buffer_release, NULL, 0);
-        if (!buf) {
-            CVPixelBufferRelease(pixbuf);
-            return AVERROR(ENOMEM);
-        }
-        frame->buf[0] = buf;
-    }
+    frame->buf[0] = av_buffer_pool_get(ctx->pool);
+    if (!frame->buf[0])
+        return AVERROR(ENOMEM);
 
     frame->data[3] = frame->buf[0]->data;
     frame->format  = AV_PIX_FMT_VIDEOTOOLBOX;