[1/7] blowfish: add av_blowfish_alloc()

Message ID 1438032992-4680-2-git-send-email-jamrial@gmail.com
State New
Headers show

Commit Message

James Almer July 27, 2015, 9:36 p.m.
Signed-off-by: James Almer <jamrial@gmail.com>
---
 doc/APIchanges       |  3 +++
 libavutil/blowfish.c | 15 +++++++++++++++
 libavutil/blowfish.h | 10 ++++++++++
 libavutil/version.h  |  5 ++++-
 4 files changed, 32 insertions(+), 1 deletion(-)

Comments

Anton Khirnov July 28, 2015, 12:19 p.m. | #1
Quoting James Almer (2015-07-27 23:36:26)
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  doc/APIchanges       |  3 +++
>  libavutil/blowfish.c | 15 +++++++++++++++
>  libavutil/blowfish.h | 10 ++++++++++
>  libavutil/version.h  |  5 ++++-
>  4 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 4ee7f41..4db1a3c 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -13,6 +13,9 @@ libavutil:     2014-08-09
>  
>  API changes, most recent first:
>  
> +2015-xx-xx - lavu 54.16.0
> +  xxxxxxx -  Add av_blowfish_alloc().
> +
>  2015-xx-xx - lavc 56.35.0 - avcodec.h
>    xxxxxxxxx - Rename CODEC_FLAG* defines to AV_CODEC_FLAG*.
>    xxxxxxxxx - Rename CODEC_CAP_* defines to AV_CODEC_CAP_*.
> diff --git a/libavutil/blowfish.c b/libavutil/blowfish.c
> index 8437dd6..97a10a7 100644
> --- a/libavutil/blowfish.c
> +++ b/libavutil/blowfish.c
> @@ -24,8 +24,18 @@
>  #include "avutil.h"
>  #include "common.h"
>  #include "intreadwrite.h"
> +#include "mem.h"
>  #include "blowfish.h"
>  
> +#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;
> +#endif
> +
>  static const uint32_t orig_p[AV_BF_ROUNDS + 2] = {
>      0x243F6A88, 0x85A308D3, 0x13198A2E, 0x03707344,
>      0xA4093822, 0x299F31D0, 0x082EFA98, 0xEC4E6C89,
> @@ -312,6 +322,11 @@ static void F(AVBlowfish *ctx, uint32_t *xl, uint32_t *xr, int i)
>      *xr = Xl;
>  }
>  
> +struct AVBlowfish *av_blowfish_alloc(void)
> +{
> +    return av_mallocz(sizeof(struct AVBlowfish));
> +}
> +
>  av_cold void av_blowfish_init(AVBlowfish *ctx, const uint8_t *key, int key_len)
>  {
>      uint32_t data, data_l, data_r;
> diff --git a/libavutil/blowfish.h b/libavutil/blowfish.h
> index 8c29536..ef0e88f 100644
> --- a/libavutil/blowfish.h
> +++ b/libavutil/blowfish.h
> @@ -22,6 +22,7 @@
>  #define AVUTIL_BLOWFISH_H
>  
>  #include <stdint.h>
> +#include "version.h"
>  
>  /**
>   * @defgroup lavu_blowfish Blowfish
> @@ -29,12 +30,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
> +struct AVBlowfish;

The struct is currently typedeffed and should remain so even after the
bump, since almost all other structs in libav are typedeffed.

The new function should also return the typedeffed name.
Same applies to the xtea patch (and technically to the other ones as
well, but there the structs are currently not typedeffed so you don't
have to fix that if you don't want to).
James Almer July 28, 2015, 5:05 p.m. | #2
On 28/07/15 9:19 AM, Anton Khirnov wrote:
> Quoting James Almer (2015-07-27 23:36:26)
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>  doc/APIchanges       |  3 +++
>>  libavutil/blowfish.c | 15 +++++++++++++++
>>  libavutil/blowfish.h | 10 ++++++++++
>>  libavutil/version.h  |  5 ++++-
>>  4 files changed, 32 insertions(+), 1 deletion(-)
>>
>> diff --git a/doc/APIchanges b/doc/APIchanges
>> index 4ee7f41..4db1a3c 100644
>> --- a/doc/APIchanges
>> +++ b/doc/APIchanges
>> @@ -13,6 +13,9 @@ libavutil:     2014-08-09
>>  
>>  API changes, most recent first:
>>  
>> +2015-xx-xx - lavu 54.16.0
>> +  xxxxxxx -  Add av_blowfish_alloc().
>> +
>>  2015-xx-xx - lavc 56.35.0 - avcodec.h
>>    xxxxxxxxx - Rename CODEC_FLAG* defines to AV_CODEC_FLAG*.
>>    xxxxxxxxx - Rename CODEC_CAP_* defines to AV_CODEC_CAP_*.
>> diff --git a/libavutil/blowfish.c b/libavutil/blowfish.c
>> index 8437dd6..97a10a7 100644
>> --- a/libavutil/blowfish.c
>> +++ b/libavutil/blowfish.c
>> @@ -24,8 +24,18 @@
>>  #include "avutil.h"
>>  #include "common.h"
>>  #include "intreadwrite.h"
>> +#include "mem.h"
>>  #include "blowfish.h"
>>  
>> +#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;
>> +#endif
>> +
>>  static const uint32_t orig_p[AV_BF_ROUNDS + 2] = {
>>      0x243F6A88, 0x85A308D3, 0x13198A2E, 0x03707344,
>>      0xA4093822, 0x299F31D0, 0x082EFA98, 0xEC4E6C89,
>> @@ -312,6 +322,11 @@ static void F(AVBlowfish *ctx, uint32_t *xl, uint32_t *xr, int i)
>>      *xr = Xl;
>>  }
>>  
>> +struct AVBlowfish *av_blowfish_alloc(void)
>> +{
>> +    return av_mallocz(sizeof(struct AVBlowfish));
>> +}
>> +
>>  av_cold void av_blowfish_init(AVBlowfish *ctx, const uint8_t *key, int key_len)
>>  {
>>      uint32_t data, data_l, data_r;
>> diff --git a/libavutil/blowfish.h b/libavutil/blowfish.h
>> index 8c29536..ef0e88f 100644
>> --- a/libavutil/blowfish.h
>> +++ b/libavutil/blowfish.h
>> @@ -22,6 +22,7 @@
>>  #define AVUTIL_BLOWFISH_H
>>  
>>  #include <stdint.h>
>> +#include "version.h"
>>  
>>  /**
>>   * @defgroup lavu_blowfish Blowfish
>> @@ -29,12 +30,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
>> +struct AVBlowfish;
> 
> The struct is currently typedeffed and should remain so even after the
> bump, since almost all other structs in libav are typedeffed.

None of the other crypto modules with opaque contexts are typedeffed in the
header (aes, sha, md5, rc4, des, etc). Same with tree.h but that one is not
part of crypto.

> The new function should also return the typedeffed name.
> Same applies to the xtea patch (and technically to the other ones as
> well, but there the structs are currently not typedeffed so you don't
> have to fix that if you don't want to).

My intention was to keep all the crypto modules as consistent as possible
without making big changes to the most used modules, and currently that means
removing the typedef from blowfish and xtea rather than doing the opposite to
rc4, des, md5, sha and aes, but if you insist I'll keep the typedef on these
two.

For that matter, should i rebase this patchset and directly apply the changes
without the whole deprecation dance now that a bump is in the works, or schedule
it for the next one instead? If the former then the rtmp/oma/asf patches should
also go in now.
Anton Khirnov July 28, 2015, 7:20 p.m. | #3
Quoting James Almer (2015-07-28 19:05:26)
> On 28/07/15 9:19 AM, Anton Khirnov wrote:
> > Quoting James Almer (2015-07-27 23:36:26)
> >> Signed-off-by: James Almer <jamrial@gmail.com>
> >> ---
> >>  doc/APIchanges       |  3 +++
> >>  libavutil/blowfish.c | 15 +++++++++++++++
> >>  libavutil/blowfish.h | 10 ++++++++++
> >>  libavutil/version.h  |  5 ++++-
> >>  4 files changed, 32 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/doc/APIchanges b/doc/APIchanges
> >> index 4ee7f41..4db1a3c 100644
> >> --- a/doc/APIchanges
> >> +++ b/doc/APIchanges
> >> @@ -13,6 +13,9 @@ libavutil:     2014-08-09
> >>  
> >>  API changes, most recent first:
> >>  
> >> +2015-xx-xx - lavu 54.16.0
> >> +  xxxxxxx -  Add av_blowfish_alloc().
> >> +
> >>  2015-xx-xx - lavc 56.35.0 - avcodec.h
> >>    xxxxxxxxx - Rename CODEC_FLAG* defines to AV_CODEC_FLAG*.
> >>    xxxxxxxxx - Rename CODEC_CAP_* defines to AV_CODEC_CAP_*.
> >> diff --git a/libavutil/blowfish.c b/libavutil/blowfish.c
> >> index 8437dd6..97a10a7 100644
> >> --- a/libavutil/blowfish.c
> >> +++ b/libavutil/blowfish.c
> >> @@ -24,8 +24,18 @@
> >>  #include "avutil.h"
> >>  #include "common.h"
> >>  #include "intreadwrite.h"
> >> +#include "mem.h"
> >>  #include "blowfish.h"
> >>  
> >> +#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;
> >> +#endif
> >> +
> >>  static const uint32_t orig_p[AV_BF_ROUNDS + 2] = {
> >>      0x243F6A88, 0x85A308D3, 0x13198A2E, 0x03707344,
> >>      0xA4093822, 0x299F31D0, 0x082EFA98, 0xEC4E6C89,
> >> @@ -312,6 +322,11 @@ static void F(AVBlowfish *ctx, uint32_t *xl, uint32_t *xr, int i)
> >>      *xr = Xl;
> >>  }
> >>  
> >> +struct AVBlowfish *av_blowfish_alloc(void)
> >> +{
> >> +    return av_mallocz(sizeof(struct AVBlowfish));
> >> +}
> >> +
> >>  av_cold void av_blowfish_init(AVBlowfish *ctx, const uint8_t *key, int key_len)
> >>  {
> >>      uint32_t data, data_l, data_r;
> >> diff --git a/libavutil/blowfish.h b/libavutil/blowfish.h
> >> index 8c29536..ef0e88f 100644
> >> --- a/libavutil/blowfish.h
> >> +++ b/libavutil/blowfish.h
> >> @@ -22,6 +22,7 @@
> >>  #define AVUTIL_BLOWFISH_H
> >>  
> >>  #include <stdint.h>
> >> +#include "version.h"
> >>  
> >>  /**
> >>   * @defgroup lavu_blowfish Blowfish
> >> @@ -29,12 +30,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
> >> +struct AVBlowfish;
> > 
> > The struct is currently typedeffed and should remain so even after the
> > bump, since almost all other structs in libav are typedeffed.
> 
> None of the other crypto modules with opaque contexts are typedeffed in the
> header (aes, sha, md5, rc4, des, etc). Same with tree.h but that one is not
> part of crypto.
> 

I consider that an oversight that should be fixed eventually. It's
better to have one common naming convention everywhere, don't you think?

> > The new function should also return the typedeffed name.
> > Same applies to the xtea patch (and technically to the other ones as
> > well, but there the structs are currently not typedeffed so you don't
> > have to fix that if you don't want to).
> 
> My intention was to keep all the crypto modules as consistent as possible
> without making big changes to the most used modules, and currently that means
> removing the typedef from blowfish and xtea rather than doing the opposite to
> rc4, des, md5, sha and aes, but if you insist I'll keep the typedef on these
> two.

Well, there's no harm in adding typedefs, since you can still use the
'struct AVFoo' name. And I really think it's better to be consistent
globally, especially when it doesn't break compatibility.

> 
> For that matter, should i rebase this patchset and directly apply the changes
> without the whole deprecation dance now that a bump is in the works, or schedule
> it for the next one instead? If the former then the rtmp/oma/asf patches should
> also go in now.

The general pattern is that a major bump only drops things that have
already been deprecated for a while (at least a year), so that the users
have time to transition to new APIs. So schedule for the next one.

Patch

diff --git a/doc/APIchanges b/doc/APIchanges
index 4ee7f41..4db1a3c 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -13,6 +13,9 @@  libavutil:     2014-08-09
 
 API changes, most recent first:
 
+2015-xx-xx - lavu 54.16.0
+  xxxxxxx -  Add av_blowfish_alloc().
+
 2015-xx-xx - lavc 56.35.0 - avcodec.h
   xxxxxxxxx - Rename CODEC_FLAG* defines to AV_CODEC_FLAG*.
   xxxxxxxxx - Rename CODEC_CAP_* defines to AV_CODEC_CAP_*.
diff --git a/libavutil/blowfish.c b/libavutil/blowfish.c
index 8437dd6..97a10a7 100644
--- a/libavutil/blowfish.c
+++ b/libavutil/blowfish.c
@@ -24,8 +24,18 @@ 
 #include "avutil.h"
 #include "common.h"
 #include "intreadwrite.h"
+#include "mem.h"
 #include "blowfish.h"
 
+#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;
+#endif
+
 static const uint32_t orig_p[AV_BF_ROUNDS + 2] = {
     0x243F6A88, 0x85A308D3, 0x13198A2E, 0x03707344,
     0xA4093822, 0x299F31D0, 0x082EFA98, 0xEC4E6C89,
@@ -312,6 +322,11 @@  static void F(AVBlowfish *ctx, uint32_t *xl, uint32_t *xr, int i)
     *xr = Xl;
 }
 
+struct AVBlowfish *av_blowfish_alloc(void)
+{
+    return av_mallocz(sizeof(struct AVBlowfish));
+}
+
 av_cold void av_blowfish_init(AVBlowfish *ctx, const uint8_t *key, int key_len)
 {
     uint32_t data, data_l, data_r;
diff --git a/libavutil/blowfish.h b/libavutil/blowfish.h
index 8c29536..ef0e88f 100644
--- a/libavutil/blowfish.h
+++ b/libavutil/blowfish.h
@@ -22,6 +22,7 @@ 
 #define AVUTIL_BLOWFISH_H
 
 #include <stdint.h>
+#include "version.h"
 
 /**
  * @defgroup lavu_blowfish Blowfish
@@ -29,12 +30,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
+struct AVBlowfish;
+#endif
+
+/**
+ * Allocate an AVBlowfish context.
+ */
+struct AVBlowfish *av_blowfish_alloc(void);
 
 /**
  * Initialize an AVBlowfish context.
diff --git a/libavutil/version.h b/libavutil/version.h
index 4c3b7f4..ae8924f 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -54,7 +54,7 @@ 
  */
 
 #define LIBAVUTIL_VERSION_MAJOR 54
-#define LIBAVUTIL_VERSION_MINOR 15
+#define LIBAVUTIL_VERSION_MINOR 16
 #define LIBAVUTIL_VERSION_MICRO  0
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
@@ -114,6 +114,9 @@ 
 #ifndef FF_API_DLOG
 #define FF_API_DLOG                     (LIBAVUTIL_VERSION_MAJOR < 55)
 #endif
+#ifndef FF_API_CRYPTO_CONTEXT
+#define FF_API_CRYPTO_CONTEXT           (LIBAVUTIL_VERSION_MAJOR < 55)
+#endif
 
 
 /**