[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