[FFmpeg-devel] [PATCH] lavu/buffer: add release function

Michael Niedermayer michaelni at gmx.at
Tue Feb 25 01:19:36 CET 2014


On Tue, Feb 25, 2014 at 12:58:12AM +0100, Lukasz Marek wrote:
> On 24.02.2014 02:18, Michael Niedermayer wrote:
> >On Sun, Feb 23, 2014 at 11:19:23PM +0100, Lukasz Marek wrote:
> >>new function allows to unref buffer and obtain its data.
> >>
> >>Signed-off-by: Lukasz Marek <lukasz.m.luki at gmail.com>
> >>---
> >>  libavutil/buffer.c | 26 ++++++++++++++++++++++++++
> >>  libavutil/buffer.h | 12 ++++++++++++
> >>  2 files changed, 38 insertions(+)
> >>
> >>diff --git a/libavutil/buffer.c b/libavutil/buffer.c
> >>index e9bf54b..a68b0be 100644
> >>--- a/libavutil/buffer.c
> >>+++ b/libavutil/buffer.c
> >>@@ -117,6 +117,32 @@ void av_buffer_unref(AVBufferRef **buf)
> >>      }
> >>  }
> >>
> >>+int av_buffer_release(AVBufferRef **buf, uint8_t **data)
> >>+{
> >>+    AVBuffer *b;
> >>+
> >>+    if (!buf || !*buf) {
> >>+        if (data)
> >>+            *data = NULL;
> >>+        return 0;
> >>+    }
> >>+    b = (*buf)->buffer;
> >>+    av_freep(buf);
> >>+
> >>+    if (!avpriv_atomic_int_add_and_fetch(&b->refcount, -1)) {
> >>+        if (data)
> >>+            *data = b->data;
> >>+        else
> >>+            b->free(b->opaque, b->data);
> >>+        av_freep(&b);
> >
> >>+    } else if (data) {
> >>+        *data = av_memdup(b->data, b->size);
> >>+        if (!*data)
> >>+            return AVERROR(ENOMEM);
> >
> >this is not safe
> >
> >you decreased the ref count and afterwards copy
> >but between the 2 the memory could have been deallocated
> 
> Yep,
> I attached updated patach - hopely better, and one more which is not
> relevant to the first one, but kinda trivial, so don't want to spam
> too much.
> 
> I assumed you talked about unref from other thread while memory is
> being copied. It is true it is not safe, but I think there are
> already rece condition in buffer.c.
> 
> For example there is AVBufferRef pointer shared across 2 threads
> Thread A has reference and calls unref


> Thread B has no reference and calls ref

Thread B cannot call ref if it has no reference, thats violating
the API


[...]

>  buffer.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 802258605a7bfb649be51b57b10830b2c7b35bda  0002-lavu-buffer-do-not-touch-refcount-directly.patch
> From 220010aa7b2971f33346fdb0a78bd95b5f91be25 Mon Sep 17 00:00:00 2001
> From: Lukasz Marek <lukasz.m.luki at gmail.com>
> Date: Tue, 25 Feb 2014 00:38:20 +0100
> Subject: [PATCH 2/2] lavu/buffer: do not touch refcount directly
> 
> Signed-off-by: Lukasz Marek <lukasz.m.luki at gmail.com>
> ---
>  libavutil/buffer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavutil/buffer.c b/libavutil/buffer.c
> index 76582f2..2a41150 100644
> --- a/libavutil/buffer.c
> +++ b/libavutil/buffer.c
> @@ -161,7 +161,7 @@ void *av_buffer_get_opaque(const AVBufferRef *buf)
>  
>  int av_buffer_get_ref_count(const AVBufferRef *buf)
>  {
> -    return buf->buffer->refcount;
> +    return avpriv_atomic_int_get(&buf->buffer->refcount);

do you know of a case where this needs to be atomic ?

[...]


-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

DNS cache poisoning attacks, popular search engine, Google internet authority
dont be evil, please
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140225/64e45d92/attachment.asc>


More information about the ffmpeg-devel mailing list