Currently the frame pool used by the default get_buffer2()
implementation is a single struct, allocated when opening the decoder.
A pointer to it is simply copied to each frame thread and we assume that
no thread attempts to modify it at an unexpected time. This is rather
fragile and potentially dangerous.

With this commit, the frame pool is made refcounted, with the reference
being propagated across threads along with other context variables. The
frame pool is now also immutable - when the stream parameters change we
drop the old reference and create a new one.
---
 libavcodec/decode.c        | 102 +++++++++++++++++++++++++++++++++++----------
 libavcodec/internal.h      |  21 +---------
 libavcodec/pthread_frame.c |  12 ++++++
 libavcodec/utils.c         |  14 +------
 4 files changed, 94 insertions(+), 55 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index f4088cd..d5210d0 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -37,6 +37,25 @@
 #include "internal.h"
 #include "thread.h"
 
+typedef struct FramePool {
+    /**
+     * Pools for each data plane. For audio all the planes have the same size,
+     * so only pools[0] is used.
+     */
+    AVBufferPool *pools[4];
+
+    /*
+     * Pool parameters
+     */
+    int format;
+    int width, height;
+    int stride_align[AV_NUM_DATA_POINTERS];
+    int linesize[4];
+    int planes;
+    int channels;
+    int samples;
+} FramePool;
+
 static int apply_param_change(AVCodecContext *avctx, AVPacket *avpkt)
 {
     int size = 0, ret;
@@ -823,10 +842,61 @@ int ff_get_format(AVCodecContext *avctx, const enum 
AVPixelFormat *fmt)
     return ret;
 }
 
+static void frame_pool_free(void *opaque, uint8_t *data)
+{
+    FramePool *pool = (FramePool*)data;
+    int i;
+
+    for (i = 0; i < FF_ARRAY_ELEMS(pool->pools); i++)
+        av_buffer_pool_uninit(&pool->pools[i]);
+
+    av_freep(&data);
+}
+
+static AVBufferRef *frame_pool_alloc(void)
+{
+    FramePool *pool = av_mallocz(sizeof(*pool));
+    AVBufferRef *buf;
+
+    if (!pool)
+        return NULL;
+
+    buf = av_buffer_create((uint8_t*)pool, sizeof(*pool),
+                           frame_pool_free, NULL, 0);
+    if (!buf) {
+        av_freep(&pool);
+        return NULL;
+    }
+
+    return buf;
+}
+
 static int update_frame_pool(AVCodecContext *avctx, AVFrame *frame)
 {
-    FramePool *pool = avctx->internal->pool;
-    int i, ret;
+    FramePool *pool = avctx->internal->pool ?
+                      (FramePool*)avctx->internal->pool->data : NULL;
+    AVBufferRef *pool_buf;
+    int i, ret, ch, planar, planes;
+
+    if (avctx->codec_type == AVMEDIA_TYPE_AUDIO) {
+        ch     = av_get_channel_layout_nb_channels(frame->channel_layout);
+        planar = av_sample_fmt_is_planar(frame->format);
+        planes = planar ? ch : 1;
+    }
+
+    if (pool && pool->format == frame->format) {
+        if (avctx->codec_type == AVMEDIA_TYPE_VIDEO &&
+            pool->width == frame->width && pool->height == frame->height)
+            return 0;
+        if (avctx->codec_type == AVMEDIA_TYPE_AUDIO && pool->planes == planes 
&&
+            pool->channels == ch && frame->nb_samples == pool->samples)
+            return 0;
+    }
+
+    pool_buf = frame_pool_alloc();
+    if (!pool_buf)
+        return AVERROR(ENOMEM);
+    pool = (FramePool*)pool_buf->data;
 
     switch (avctx->codec_type) {
     case AVMEDIA_TYPE_VIDEO: {
@@ -837,10 +907,6 @@ static int update_frame_pool(AVCodecContext *avctx, 
AVFrame *frame)
         int h = frame->height;
         int tmpsize, unaligned;
 
-        if (pool->format == frame->format &&
-            pool->width == frame->width && pool->height == frame->height)
-            return 0;
-
         avcodec_align_dimensions2(avctx, &w, &h, pool->stride_align);
 
         do {
@@ -865,7 +931,6 @@ static int update_frame_pool(AVCodecContext *avctx, AVFrame 
*frame)
         size[i] = tmpsize - (data[i] - data[0]);
 
         for (i = 0; i < 4; i++) {
-            av_buffer_pool_uninit(&pool->pools[i]);
             pool->linesize[i] = linesize[i];
             if (size[i]) {
                 pool->pools[i] = av_buffer_pool_init(size[i] + 16, NULL);
@@ -882,15 +947,6 @@ static int update_frame_pool(AVCodecContext *avctx, 
AVFrame *frame)
         break;
         }
     case AVMEDIA_TYPE_AUDIO: {
-        int ch     = av_get_channel_layout_nb_channels(frame->channel_layout);
-        int planar = av_sample_fmt_is_planar(frame->format);
-        int planes = planar ? ch : 1;
-
-        if (pool->format == frame->format && pool->planes == planes &&
-            pool->channels == ch && frame->nb_samples == pool->samples)
-            return 0;
-
-        av_buffer_pool_uninit(&pool->pools[0]);
         ret = av_samples_get_buffer_size(&pool->linesize[0], ch,
                                          frame->nb_samples, frame->format, 0);
         if (ret < 0)
@@ -910,19 +966,19 @@ static int update_frame_pool(AVCodecContext *avctx, 
AVFrame *frame)
         }
     default: av_assert0(0);
     }
+
+    av_buffer_unref(&avctx->internal->pool);
+    avctx->internal->pool = pool_buf;
+
     return 0;
 fail:
-    for (i = 0; i < 4; i++)
-        av_buffer_pool_uninit(&pool->pools[i]);
-    pool->format = -1;
-    pool->planes = pool->channels = pool->samples = 0;
-    pool->width  = pool->height = 0;
+    av_buffer_unref(&pool_buf);
     return ret;
 }
 
 static int audio_get_buffer(AVCodecContext *avctx, AVFrame *frame)
 {
-    FramePool *pool = avctx->internal->pool;
+    FramePool *pool = (FramePool*)avctx->internal->pool->data;
     int planes = pool->planes;
     int i;
 
@@ -965,7 +1021,7 @@ fail:
 
 static int video_get_buffer(AVCodecContext *s, AVFrame *pic)
 {
-    FramePool *pool = s->internal->pool;
+    FramePool *pool = (FramePool*)s->internal->pool->data;
     int i;
 
     if (pic->data[0]) {
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index f302f86..2b0eb91 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -86,25 +86,6 @@
 
 #define FF_SIGNBIT(x) (x >> CHAR_BIT * sizeof(x) - 1)
 
-typedef struct FramePool {
-    /**
-     * Pools for each data plane. For audio all the planes have the same size,
-     * so only pools[0] is used.
-     */
-    AVBufferPool *pools[4];
-
-    /*
-     * Pool parameters
-     */
-    int format;
-    int width, height;
-    int stride_align[AV_NUM_DATA_POINTERS];
-    int linesize[4];
-    int planes;
-    int channels;
-    int samples;
-} FramePool;
-
 typedef struct DecodeSimpleContext {
     AVPacket *in_pkt;
     AVFrame  *out_frame;
@@ -132,7 +113,7 @@ typedef struct AVCodecInternal {
 
     AVFrame *to_free;
 
-    FramePool *pool;
+    AVBufferRef *pool;
 
     void *thread_ctx;
 
diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index 12d5481..f897871 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -271,6 +271,17 @@ FF_ENABLE_DEPRECATION_WARNINGS
                     return AVERROR(ENOMEM);
             }
         }
+
+        if (!!dst->internal->pool != !!src->internal->pool ||
+            (dst->internal->pool && dst->internal->pool->data != 
src->internal->pool->data)) {
+            av_buffer_unref(&dst->internal->pool);
+
+            if (src->internal->pool) {
+                dst->internal->pool = av_buffer_ref(src->internal->pool);
+                if (!dst->internal->pool)
+                    return AVERROR(ENOMEM);
+            }
+        }
     }
 
     if (for_user) {
@@ -644,6 +655,7 @@ void ff_frame_thread_free(AVCodecContext *avctx, int 
thread_count)
         }
 
         av_buffer_unref(&p->avctx->hw_frames_ctx);
+        av_buffer_unref(&p->avctx->internal->pool);
 
         av_freep(&p->avctx->internal);
         av_freep(&p->avctx);
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 2978109..304a946 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -410,12 +410,6 @@ int attribute_align_arg avcodec_open2(AVCodecContext 
*avctx, const AVCodec *code
         goto end;
     }
 
-    avctx->internal->pool = av_mallocz(sizeof(*avctx->internal->pool));
-    if (!avctx->internal->pool) {
-        ret = AVERROR(ENOMEM);
-        goto free_and_end;
-    }
-
     avctx->internal->to_free = av_frame_alloc();
     if (!avctx->internal->to_free) {
         ret = AVERROR(ENOMEM);
@@ -737,7 +731,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
 
         av_packet_free(&avctx->internal->ds.in_pkt);
 
-        av_freep(&avctx->internal->pool);
+        av_buffer_unref(&avctx->internal->pool);
     }
     av_freep(&avctx->internal);
     avctx->codec = NULL;
@@ -768,8 +762,6 @@ av_cold int avcodec_close(AVCodecContext *avctx)
     int i;
 
     if (avcodec_is_open(avctx)) {
-        FramePool *pool = avctx->internal->pool;
-
         if (HAVE_THREADS && avctx->internal->thread_ctx)
             ff_thread_free(avctx);
         if (avctx->codec && avctx->codec->close)
@@ -782,9 +774,7 @@ av_cold int avcodec_close(AVCodecContext *avctx)
 
         av_packet_free(&avctx->internal->ds.in_pkt);
 
-        for (i = 0; i < FF_ARRAY_ELEMS(pool->pools); i++)
-            av_buffer_pool_uninit(&pool->pools[i]);
-        av_freep(&avctx->internal->pool);
+        av_buffer_unref(&avctx->internal->pool);
 
         if (avctx->hwaccel && avctx->hwaccel->uninit)
             avctx->hwaccel->uninit(avctx);
-- 
2.0.0

_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to