On Thu, Apr 2, 2015 at 3:33 PM, Martin Storsjö <[email protected]> wrote:
> On Thu, 2 Apr 2015, Vittorio Giovara wrote:
>
>> On Thu, Apr 2, 2015 at 2:17 PM, Martin Storsjö <[email protected]> wrote:
>>>
>>> The previous documentation was very vague and almost misleading.
>>> ---
>>>  libavcodec/internal.h | 9 +++++++--
>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
>>> index a681329..c3dfaa1 100644
>>> --- a/libavcodec/internal.h
>>> +++ b/libavcodec/internal.h
>>> @@ -34,11 +34,16 @@
>>>  #include "config.h"
>>>
>>>  /**
>>> - * Codec is thread safe.
>>> + * The codec doesn't touch global variables in the init function,
>>> allowing
>>> + * running the init function without locking any global mutexes.
>>
>>
>> How about "The codec doesn't access any global variable in the init
>
> variables, not variable
>
>> function, allowing to call the init function without locking any
>> global mutexes."?
>
> Sure

Ok so gathering everyone's input this should be "The codec does not
modify any global variables in the init function, allowing to call the
init function without locking any global mutexes."

>>>   */
>>>  #define FF_CODEC_CAP_INIT_THREADSAFE        (1 << 0)
>>>  /**
>>> - * Codec cleans up memory on init failure.
>>> + * The codec allows (and requires) calling the close function for
>>> deallocation
>>> + * even if the init function returned a failure. (Previously, without
>>> this
>>> + * capability flag, a codec does such cleanup internally when returning
>>> + * failures from the init function and does not expect the close
>>> function
>>> + * to be called at all.)
>
> Also I prefer writing this from the point of view of the codec
> implementation, i.e., what does my codec code guarantee to the framework
> (because the consumer of this flag is the framework), not what the
> framework+codec together do when this is set.
>
>
> My updated suggested text is this:
>
>> The codec allows calling the close function for deallocation even if the
>> init function returned a failure. (Previously, without this capability flag,
>> a codec does such cleanup internally when returning failures from the init
>> function and does not expect the close function to be called at all.)
>
> Compared to the previous version, I removed the parentheses saying that it
> requires calling close since that's not strictly true.
>
> I'm open to other alternative wordings as well, but they should be crystal
> clear to the implementer of a codec what they imply.

I like this wording, my only suggestion would be to drop the temporal
reference and the parentheses, like:
"The codec allows calling the close function for deallocation even if
the init function returned a failure. Without this capability flag, a
codec does such cleanup internally when returning failures from the
init function and does not expect the close function to be called at
all."
-- 
Vittorio
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to