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

[Lukasz Marek]

>>>>   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 ?
>>
>> OK, but why it is used in other places? If no use case in here than
>> what's the use case in av_buffer_is_writable for example?
>
>> If you are
>> sure it is not required then feel free to ignore this patch.
>> Probably OK, I haven't investigate it before, just guessed from
>> other uses of this function.
>
> Iam not sure, which is why i asked
> and also why ive not suggested to remove it from
> av_buffer_is_writable()
> I dont immedeatly see why it would be needed, this does not mean its
> not needed
> but i would suggest to leave it on all functions that are not speed
> critical

I promised to ask about it. My colleague had look on it and didn't get 
straight answer it should merged or not. He said just that, it is always 
good to use such getters when you want to be pedantic, but reading 
refcount should be usually safe within this scope when it is aligned 
properly etc without this getter. For me this patch can be dropped. Also 
mentioned such function should not be used for serious reasons, because 
refcount may change right after returning from function. This makes me 
still wondering about sense of av_buffer_is_writable function. It may be 
unsafe when user don't understand it deeply.


-- 
Best Regards,
Lukasz Marek

If you can't explain it simply, you don't understand it well enough. - 
Albert Einstein