[FFmpeg-devel] [PATCH 1/4] blowfish: add av_blowfish_alloc()

[James Almer]

On 31/07/15 11:09 PM, Michael Niedermayer wrote:
> On Fri, Jul 31, 2015 at 05:58:48PM -0300, James Almer wrote:
>> On 31/07/15 5:40 PM, Michael Niedermayer wrote:
>>> On Fri, Jul 31, 2015 at 02:18:07PM -0300, James Almer wrote:
>>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>>> ---
>>>>  doc/APIchanges       |  3 +++
>>>>  libavutil/blowfish.c | 15 +++++++++++++++
>>>>  libavutil/blowfish.h | 10 ++++++++++
>>>>  libavutil/version.h  |  5 ++++-
>>>>  4 files changed, 32 insertions(+), 1 deletion(-)
>>> [...]
>>>> @@ -30,12 +31,21 @@
>>>>   * @{
>>>>   */
>>>>  
>>>> +#if FF_API_CRYPTO_CONTEXT
>>>>  #define AV_BF_ROUNDS 16
>>>>  
>>>>  typedef struct AVBlowfish {
>>>>      uint32_t p[AV_BF_ROUNDS + 2];
>>>>      uint32_t s[4][256];
>>>>  } AVBlowfish;
>>>> +#else
>>>> +typedef struct AVBlowfish AVBlowfish;
>>>> +#endif
>>>
>>> Is it intended to remove all means to allocate the context on the
>>> stack ?
>>>
>>> this would avoid dealing with malloc/free and malloc failure handling
>>> and could be a signifiant advantage also if any usecase is in a API
>>> that lacks the possibility to fail, like maybe some crypto callbacks
>>> with "void return"
>>
>> I have no preferences regarding the context, really, I'm just trying to get the
>> API for all the modules consistent. Not sure what would be the use of an alloc
>> function if the context is not opaque, though.
> 
> ive no oppinion on the API at all, just remembered that usage without
> alloc was something we thought about in some of the things in avutil
> long ago. That is some of the APIs in there where designed so they
> could be used without *alloc() and the implied complexity.
> iam not against droping that if thats what people prefer
> 
> It would also be possible to add the alloc code and leave the on stack
> usage possibility or try to change the struct to be opaque and still
> usable o the stack. There are many possibilities, i dont know which
> people prefer.
> 

Ok. Since the scheduled removal can be undone at any time (effectively making
this patchset only add the alloc functions) it's not something we would be stuck
with as soon as it's committed, so I'll push this set as is in a few days if
nobody is against it. We can then see how to proceed.

Thanks.