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 <[email protected]>
> >> ---
> >>  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.

-- 
Anton Khirnov
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to