Re: [FFmpeg-devel] [PATCH 1/4] blowfish: add av_blowfish_alloc()
On Sat, Aug 01, 2015 at 01:31:16PM -0300, James Almer wrote: [...] Also, rc4 currently can't encrypt, only decrypt. No idea if adding encryption functionality will require changes to the struct. There is no distinction between encryption and decryption with RC4 -- Clément B. pgp83sWRCrhNW.pgp Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/4] blowfish: add av_blowfish_alloc()
On Sat, Aug 29, 2015 at 02:54:06AM -0300, James Almer wrote: On 8/1/2015 1:31 PM, James Almer wrote: On 01/08/15 7:15 AM, Nicolas George wrote: Le quartidi 14 thermidor, an CCXXIII, Reimar Döffinger a écrit : If the goal is consistency, wouldn't an API like av_hash be the better solution? Hear, hear! (Actually, when reading that discussion for the first time, I had not noticed it was about ciphers and not hashes, and my first reaction was there is an unified API already.) And yes, on-stack allocations were desired, even though they make changes to the structs hard... Pointing the obvious: I very much doubt we ever need to change the struct used just for the implementation of a 30-years-old block cipher. One example i can give that took advantage of the hidden nature of the struct is ripemd. See aa70801aaf4038faaf673558c241fa19d5dcd181. Admittedly one could blame this on my original design decision to reduce code duplication (The object size grew considerably after that change), but in any case had the struct not been opaque we would be stuck with a useless element and its relevant ugly deprecation code for the next year or so, not to mention how to handle the transform() function pointer. Also, rc4 currently can't encrypt, only decrypt. No idea if adding encryption functionality will require changes to the struct. In any case, I have no preferences. I can push the alloc() function without the deprecation code, or push both and decide later if the deprecation code is kept or not. Which do you prefer? This should be the guiding question: weight the potential hassle of a change in the struct versus the practical hassle of heap allocations. Regards, Since this set was merged from libav as-is and we're about to make a 2.8 release i want to know if we should abort the deprecations right now before the release, or leave them alone and decide later on. I think if theres no consens on deprecating it then its better not to deprecate in the release I tried updating the rtmp code to dynamically allocate the structs and that not only meant some considerable changes to the functions in order to propagate allocation failure errors, but i also got some odd behavior with the test served i tried. I could have done something wrong, though. Why is it a good idea to decide before release? Because if we get a release out there with the deprecations in place, we would then have (or at least it would be proper) to inform users with a line in APIChanges about the deprecation being aborted if that's what we choose to do. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Many things microsoft did are stupid, but not doing something just because microsoft did it is even more stupid. If everything ms did were stupid they would be bankrupt already. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/4] blowfish: add av_blowfish_alloc()
On 8/29/2015 8:56 AM, Michael Niedermayer wrote: On Sat, Aug 29, 2015 at 02:54:06AM -0300, James Almer wrote: On 8/1/2015 1:31 PM, James Almer wrote: On 01/08/15 7:15 AM, Nicolas George wrote: Le quartidi 14 thermidor, an CCXXIII, Reimar Döffinger a écrit : If the goal is consistency, wouldn't an API like av_hash be the better solution? Hear, hear! (Actually, when reading that discussion for the first time, I had not noticed it was about ciphers and not hashes, and my first reaction was there is an unified API already.) And yes, on-stack allocations were desired, even though they make changes to the structs hard... Pointing the obvious: I very much doubt we ever need to change the struct used just for the implementation of a 30-years-old block cipher. One example i can give that took advantage of the hidden nature of the struct is ripemd. See aa70801aaf4038faaf673558c241fa19d5dcd181. Admittedly one could blame this on my original design decision to reduce code duplication (The object size grew considerably after that change), but in any case had the struct not been opaque we would be stuck with a useless element and its relevant ugly deprecation code for the next year or so, not to mention how to handle the transform() function pointer. Also, rc4 currently can't encrypt, only decrypt. No idea if adding encryption functionality will require changes to the struct. In any case, I have no preferences. I can push the alloc() function without the deprecation code, or push both and decide later if the deprecation code is kept or not. Which do you prefer? This should be the guiding question: weight the potential hassle of a change in the struct versus the practical hassle of heap allocations. Regards, Since this set was merged from libav as-is and we're about to make a 2.8 release i want to know if we should abort the deprecations right now before the release, or leave them alone and decide later on. I think if theres no consens on deprecating it then its better not to deprecate in the release Ok, I'll remove them from the release branch once it's created. We can then decide what to do for the next. Unless someone has a better idea of how to approach this. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/4] blowfish: add av_blowfish_alloc()
On 8/1/2015 1:31 PM, James Almer wrote: On 01/08/15 7:15 AM, Nicolas George wrote: Le quartidi 14 thermidor, an CCXXIII, Reimar Döffinger a écrit : If the goal is consistency, wouldn't an API like av_hash be the better solution? Hear, hear! (Actually, when reading that discussion for the first time, I had not noticed it was about ciphers and not hashes, and my first reaction was there is an unified API already.) And yes, on-stack allocations were desired, even though they make changes to the structs hard... Pointing the obvious: I very much doubt we ever need to change the struct used just for the implementation of a 30-years-old block cipher. One example i can give that took advantage of the hidden nature of the struct is ripemd. See aa70801aaf4038faaf673558c241fa19d5dcd181. Admittedly one could blame this on my original design decision to reduce code duplication (The object size grew considerably after that change), but in any case had the struct not been opaque we would be stuck with a useless element and its relevant ugly deprecation code for the next year or so, not to mention how to handle the transform() function pointer. Also, rc4 currently can't encrypt, only decrypt. No idea if adding encryption functionality will require changes to the struct. In any case, I have no preferences. I can push the alloc() function without the deprecation code, or push both and decide later if the deprecation code is kept or not. Which do you prefer? This should be the guiding question: weight the potential hassle of a change in the struct versus the practical hassle of heap allocations. Regards, Since this set was merged from libav as-is and we're about to make a 2.8 release i want to know if we should abort the deprecations right now before the release, or leave them alone and decide later on. I tried updating the rtmp code to dynamically allocate the structs and that not only meant some considerable changes to the functions in order to propagate allocation failure errors, but i also got some odd behavior with the test served i tried. I could have done something wrong, though. Why is it a good idea to decide before release? Because if we get a release out there with the deprecations in place, we would then have (or at least it would be proper) to inform users with a line in APIChanges about the deprecation being aborted if that's what we choose to do. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/4] blowfish: add av_blowfish_alloc()
On 31.07.2015, at 19:18, James Almer jamr...@gmail.com wrote: Signed-off-by: James Almer jamr...@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 29e9da9..d222fc6 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -15,6 +15,9 @@ libavutil: 2014-08-09 API changes, most recent first: +2015-xx-xx - lavu 54.30.0 + xxx - Add av_blowfish_alloc(). If the goal is consistency, wouldn't an API like av_hash be the better solution? And yes, on-stack allocations were desired, even though they make changes to the structs hard... ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/4] blowfish: add av_blowfish_alloc()
On 01/08/15 1:31 PM, James Almer wrote: Also, rc4 currently can't encrypt, only decrypt. No idea if adding encryption functionality will require changes to the struct. Whoops. Apparently that's not the case. Next time I'll read how a given algorithm works before commenting on an implementation. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/4] blowfish: add av_blowfish_alloc()
On 01/08/15 7:15 AM, Nicolas George wrote: Le quartidi 14 thermidor, an CCXXIII, Reimar Döffinger a écrit : If the goal is consistency, wouldn't an API like av_hash be the better solution? Hear, hear! (Actually, when reading that discussion for the first time, I had not noticed it was about ciphers and not hashes, and my first reaction was there is an unified API already.) And yes, on-stack allocations were desired, even though they make changes to the structs hard... Pointing the obvious: I very much doubt we ever need to change the struct used just for the implementation of a 30-years-old block cipher. One example i can give that took advantage of the hidden nature of the struct is ripemd. See aa70801aaf4038faaf673558c241fa19d5dcd181. Admittedly one could blame this on my original design decision to reduce code duplication (The object size grew considerably after that change), but in any case had the struct not been opaque we would be stuck with a useless element and its relevant ugly deprecation code for the next year or so, not to mention how to handle the transform() function pointer. Also, rc4 currently can't encrypt, only decrypt. No idea if adding encryption functionality will require changes to the struct. In any case, I have no preferences. I can push the alloc() function without the deprecation code, or push both and decide later if the deprecation code is kept or not. Which do you prefer? This should be the guiding question: weight the potential hassle of a change in the struct versus the practical hassle of heap allocations. Regards, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/4] blowfish: add av_blowfish_alloc()
Le quartidi 14 thermidor, an CCXXIII, Reimar Döffinger a écrit : If the goal is consistency, wouldn't an API like av_hash be the better solution? Hear, hear! (Actually, when reading that discussion for the first time, I had not noticed it was about ciphers and not hashes, and my first reaction was there is an unified API already.) And yes, on-stack allocations were desired, even though they make changes to the structs hard... Pointing the obvious: I very much doubt we ever need to change the struct used just for the implementation of a 30-years-old block cipher. This should be the guiding question: weight the potential hassle of a change in the struct versus the practical hassle of heap allocations. 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 1/4] blowfish: add av_blowfish_alloc()
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 jamr...@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.
And you're right, rtmp for example needs some considerable changes to actually
use the alloc functions.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/4] blowfish: add av_blowfish_alloc()
On 31/07/15 3:33 PM, Paul B Mahol wrote: For what is this useful? So these four modules are consistent with the rest of the crypto modules. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/4] blowfish: add av_blowfish_alloc()
Signed-off-by: James Almer jamr...@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 29e9da9..d222fc6 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,9 @@ libavutil: 2014-08-09
API changes, most recent first:
+2015-xx-xx - lavu 54.30.0
+ xxx - Add av_blowfish_alloc().
+
2015-xx-xx - lavc 56.35.0 - avcodec.h
x - Rename CODEC_FLAG* defines to AV_CODEC_FLAG*.
x - Rename CODEC_CAP_* defines to AV_CODEC_CAP_*.
diff --git a/libavutil/blowfish.c b/libavutil/blowfish.c
index 3821427..4f7e4df 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
+
+struct AVBlowfish {
+uint32_t p[AV_BF_ROUNDS + 2];
+uint32_t s[4][256];
+};
+#endif
+
static const uint32_t orig_p[AV_BF_ROUNDS + 2] = {
0x243F6A88, 0x85A308D3, 0x13198A2E, 0x03707344,
0xA4093822, 0x299F31D0, 0x082EFA98, 0xEC4E6C89,
@@ -300,6 +310,11 @@ static const uint32_t orig_s[4][256] = {
+ ctx-s[3][ Xl 0xFF])\
^ P;
+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 0b00453..d163fd3 100644
--- a/libavutil/blowfish.h
+++ b/libavutil/blowfish.h
@@ -23,6 +23,7 @@
#define AVUTIL_BLOWFISH_H
#include stdint.h
+#include version.h
/**
* @defgroup lavu_blowfish Blowfish
@@ -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
+
+/**
+ * Allocate an AVBlowfish context.
+ */
+AVBlowfish *av_blowfish_alloc(void);
/**
* Initialize an AVBlowfish context.
diff --git a/libavutil/version.h b/libavutil/version.h
index b10f3e1..653f530 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -56,7 +56,7 @@
*/
#define LIBAVUTIL_VERSION_MAJOR 54
-#define LIBAVUTIL_VERSION_MINOR 29
+#define LIBAVUTIL_VERSION_MINOR 30
#define LIBAVUTIL_VERSION_MICRO 100
#define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
@@ -125,6 +125,9 @@
#ifndef FF_API_HMAC
#define FF_API_HMAC (LIBAVUTIL_VERSION_MAJOR 55)
#endif
+#ifndef FF_API_CRYPTO_CONTEXT
+#define FF_API_CRYPTO_CONTEXT (LIBAVUTIL_VERSION_MAJOR 56)
+#endif
#ifndef FF_CONST_AVUTIL55
#if LIBAVUTIL_VERSION_MAJOR = 55
--
2.5.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/4] blowfish: add av_blowfish_alloc()
On Fri, Jul 31, 2015 at 02:18:07PM -0300, James Almer wrote:
Signed-off-by: James Almer jamr...@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
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Into a blind darkness they enter who follow after the Ignorance,
they as if into a greater darkness enter who devote themselves
to the Knowledge alone. -- Isha Upanishad
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/4] blowfish: add av_blowfish_alloc()
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 jamr...@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.
And you're right, rtmp for example needs some considerable changes to actually
use the alloc functions.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/4] blowfish: add av_blowfish_alloc()
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 jamr...@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.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
