Re: [FFmpeg-devel] [PATCH] avutil: install des.h, rc4.h and tree.h as public headers

2015-10-20 Thread Andreas Cadhalpun
On 14.10.2015 14:07, Michael Niedermayer wrote:
> On Wed, Oct 14, 2015 at 12:37:31AM +0200, Andreas Cadhalpun wrote:
>> These headers contain functions supposed to be public.
>>
>> libavutil/des.h:
>>  av_des_alloc
>>  av_des_crypt
>>  av_des_init
>>  av_des_mac
>> libavutil/rc4.h:
>>  av_rc4_alloc
>>  av_rc4_crypt
>>  av_rc4_init
>> libavutil/tree.h
>>  av_tree_destroy
>>  av_tree_enumerate
>>  av_tree_find
>>  av_tree_insert
>>  av_tree_node_alloc
>>  av_tree_node_size
> 
> LGTM, but maybe you want to wait a day or 2 in case others have
> comments

FF_API_CRYPTO_CONTEXT has been removed, so I pushed this now.

Best regards,
Andreas

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avutil: install des.h, rc4.h and tree.h as public headers

2015-10-14 Thread Nicolas George
Le duodi 22 vendémiaire, an CCXXIV, James Almer a écrit :
> Since these two were never installed, we can remove the FF_API_CRYPTO_CONTEXT
> wrapper from them before applying this patch, as this would be the first time
> the API becomes public.
> 
> Now, what i want to know is what will it be in the end for the actual
> deprecation in question? Reimar and Nicolas were against making the context
> opaque as having them in stack has its uses.
> I want to know what people want, since the deprecation is not present on any
> release and we still have time to rollback. Do we keep the deprecation in
> place for des, rc4, blowfish and xtea, or remove it altogether?

The reason for my objection are that the odds that the contents of the
context change in the future are very small for low-level standardized
crypto functions (this is the reason for making structure opaques). On the
other hand, foregoing the checks for memory allocations is nice, and crypto
functions may be used in speed-critical portions of the code where memory
allocations are not negligible.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avutil: install des.h, rc4.h and tree.h as public headers

2015-10-14 Thread James Almer
On 10/14/2015 2:16 PM, Ronald S. Bultje wrote:
> Hi,
> 
> On Wed, Oct 14, 2015 at 10:50 AM, Nicolas George  wrote:
> 
>> Le duodi 22 vendémiaire, an CCXXIV, James Almer a écrit :
>>> Since these two were never installed, we can remove the
>> FF_API_CRYPTO_CONTEXT
>>> wrapper from them before applying this patch, as this would be the first
>> time
>>> the API becomes public.
>>>
>>> Now, what i want to know is what will it be in the end for the actual
>>> deprecation in question? Reimar and Nicolas were against making the
>> context
>>> opaque as having them in stack has its uses.
>>> I want to know what people want, since the deprecation is not present on
>> any
>>> release and we still have time to rollback. Do we keep the deprecation in
>>> place for des, rc4, blowfish and xtea, or remove it altogether?
>>
>> The reason for my objection are that the odds that the contents of the
>> context change in the future are very small for low-level standardized
>> crypto functions (this is the reason for making structure opaques). On the
>> other hand, foregoing the checks for memory allocations is nice, and crypto
>> functions may be used in speed-critical portions of the code where memory
>> allocations are not negligible.
> 
> 
> I think this is a reasonable argument, so +1 from me for "let's expose the
> struct and remove deprecations".
> 
> Ronald

Ok, i'll wait a bit in case someone else wants to comment then i'll send a
patch undoing the deprecations.

Thanks all for your input.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avutil: install des.h, rc4.h and tree.h as public headers

2015-10-14 Thread Michael Niedermayer
On Wed, Oct 14, 2015 at 12:37:31AM +0200, Andreas Cadhalpun wrote:
> These headers contain functions supposed to be public.
> 
> libavutil/des.h:
>  av_des_alloc
>  av_des_crypt
>  av_des_init
>  av_des_mac
> libavutil/rc4.h:
>  av_rc4_alloc
>  av_rc4_crypt
>  av_rc4_init
> libavutil/tree.h
>  av_tree_destroy
>  av_tree_enumerate
>  av_tree_find
>  av_tree_insert
>  av_tree_node_alloc
>  av_tree_node_size

LGTM, but maybe you want to wait a day or 2 in case others have
comments

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avutil: install des.h, rc4.h and tree.h as public headers

2015-10-14 Thread Ronald S. Bultje
Hi,

On Wed, Oct 14, 2015 at 10:50 AM, Nicolas George  wrote:

> Le duodi 22 vendémiaire, an CCXXIV, James Almer a écrit :
> > Since these two were never installed, we can remove the
> FF_API_CRYPTO_CONTEXT
> > wrapper from them before applying this patch, as this would be the first
> time
> > the API becomes public.
> >
> > Now, what i want to know is what will it be in the end for the actual
> > deprecation in question? Reimar and Nicolas were against making the
> context
> > opaque as having them in stack has its uses.
> > I want to know what people want, since the deprecation is not present on
> any
> > release and we still have time to rollback. Do we keep the deprecation in
> > place for des, rc4, blowfish and xtea, or remove it altogether?
>
> The reason for my objection are that the odds that the contents of the
> context change in the future are very small for low-level standardized
> crypto functions (this is the reason for making structure opaques). On the
> other hand, foregoing the checks for memory allocations is nice, and crypto
> functions may be used in speed-critical portions of the code where memory
> allocations are not negligible.


I think this is a reasonable argument, so +1 from me for "let's expose the
struct and remove deprecations".

Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avutil: install des.h, rc4.h and tree.h as public headers

2015-10-13 Thread James Almer
On 10/13/2015 7:37 PM, Andreas Cadhalpun wrote:
> These headers contain functions supposed to be public.
> 
> libavutil/des.h:
>  av_des_alloc
>  av_des_crypt
>  av_des_init
>  av_des_mac
> libavutil/rc4.h:
>  av_rc4_alloc
>  av_rc4_crypt
>  av_rc4_init

Since these two were never installed, we can remove the FF_API_CRYPTO_CONTEXT
wrapper from them before applying this patch, as this would be the first time
the API becomes public.

Now, what i want to know is what will it be in the end for the actual
deprecation in question? Reimar and Nicolas were against making the context
opaque as having them in stack has its uses.
I want to know what people want, since the deprecation is not present on any
release and we still have time to rollback. Do we keep the deprecation in
place for des, rc4, blowfish and xtea, or remove it altogether?

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel