On Thu, Jun 28, 2012 at 12:57 PM, Diego Biurrun <[email protected]> wrote: > On Thu, Jun 28, 2012 at 12:13:23PM +0200, Samuel Pitoiset wrote: >> --- >> configure | 4 + >> doc/protocols.texi | 9 + >> libavformat/Makefile | 2 + >> libavformat/allformats.c | 2 + >> libavformat/rtmpdh.c | 228 ++++++++++++++++++++ >> libavformat/rtmpdh.h | 106 +++++++++ >> libavformat/rtmpenc.c | 532 >> ++++++++++++++++++++++++++++++++++++++++++++++ >> libavformat/rtmpenc.h | 58 +++++ >> libavformat/rtmpproto.c | 80 +++++++- >> 9 files changed, 1016 insertions(+), 5 deletions(-) >> create mode 100644 libavformat/rtmpdh.c >> create mode 100644 libavformat/rtmpdh.h >> create mode 100644 libavformat/rtmpenc.c >> create mode 100644 libavformat/rtmpenc.h > > This is missing a changelog entry, a version bump and an entry in > doc/general.texi. Please consult the new formats checklist in the > developer documentation (again).
Done (I forgot to add WIP as prefix...). > > Get rid of the stray tabs that snuck into your code. > > Does this pass "make check"? Yes, it works. > >> --- /dev/null >> +++ b/libavformat/rtmpdh.c >> @@ -0,0 +1,228 @@ >> + >> +#include "avformat.h" >> + >> +#include "libavutil/sha.h" >> + >> +#include "internal.h" >> +#include "rtmp.h" >> +#include "rtmpdh.h" >> +#include "rtmpenc.h" >> +#include "url.h" > > Separating the avformat.h #include looks weird, whatever... Done and I removed useless includes. > >> +/* RFC 2631, Section 2.1.5, http://www.ietf.org/rfc/rfc2631.txt */ >> +static int dh_is_valid_public_key(ff_bignum y, ff_bignum p, ff_bignum q) >> +{ >> + ff_bignum bn = NULL; >> + int ret = 0; >> + >> + ff_bn_new(bn); >> + if (!bn) { >> + ret = AVERROR(ENOMEM); >> + goto fail; >> + } > > Just return AVERROR(ENOMEM) directly here. I don't think there is a > point in freeing memory that was never allocated. Done. > >> + if (ff_bn_cmp_1(bn) != 0) { > > Drop the " != 0". Done. > >> +ff_dh *ff_dh_init(int key_len) >> +{ >> + ff_dh *dh; >> + int ret; >> + >> + if (!(dh = ff_dh_new())) { >> + ret = AVERROR(ENOMEM); >> + goto fail; >> + } > > see above Done. > >> + /* convert the public key value into big-endian form */ >> + memset(pub_key, 0, pub_key_len); >> + ff_bn_bn2bin(dh->pub_key, pub_key + (pub_key_len - len), len); > > pointless () Done. > >> + /* convert the big-endian form of the public key into a bignum */ >> + ff_bn_bin2bn(pub_key_bn, pub_key, pub_key_len); >> + if (!pub_key_bn) { >> + ret = AVERROR(ENOMEM); >> + goto fail; >> + } > > see above Done. > >> + /* when the public key is valid we have to compute the shared secret >> key */ >> + if ((ret = dh_is_valid_public_key(pub_key_bn, dh->p, q1)) < 0) { >> + goto fail; >> + } else { >> + if ((ret = ff_dh_compute_key(secret, pub_key_len, pub_key_bn, dh)) >> < 0) { >> + ret = AVERROR(EINVAL); >> + goto fail; >> + } >> + } > > Merge the else and the if. Done. > >> --- /dev/null >> +++ b/libavformat/rtmpdh.h >> @@ -0,0 +1,106 @@ >> + >> +#ifndef AVFORMAT_RTMPDH_H >> +#define AVFORMAT_RTMPDH_H >> + >> +#include "avformat.h" >> + >> +#if CONFIG_OPENSSL >> +#include <openssl/bn.h> >> +#include <openssl/dh.h> > > You are missing a config.h #include for this to work properly. Indeed, done. > >> +/* http://www.ietf.org/rfc/rfc3526.txt */ >> +/* 2^1024 - 2^960 - 1 + 2^64 * { [2^894 pi] + 129093 } */ >> +#define P1024 \ >> + "FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD1" \ >> + "29024E088A67CC74020BBEA63B139B22514A08798E3404DD" \ >> + "EF9519B3CD3A431B302B0A6DF25F14374FE1356D6D51C245" \ >> + "E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7ED" \ >> + "EE386BFB5A899FA5AE9F24117C4B1FE649286651ECE65381" \ >> + "FFFFFFFFFFFFFFFF" >> + >> +/* Group morder largest prime factor: */ >> +#define Q1024 \ >> + "7FFFFFFFFFFFFFFFE487ED5110B4611A62633145C06E0E68" \ >> + "948127044533E63A0105DF531D89CD9128A5043CC71A026E" \ >> + "F7CA8CD9E69D218D98158536F92F8A1BA7F09AB6B6A8E122" \ >> + "F242DABB312F3F637A262174D31BF6B585FFAE5B7A035BF6" \ >> + "F71C35FDAD44CFD2D74F9208BE258FF324943328F67329C0" \ >> + "FFFFFFFFFFFFFFFF" > > Align the \. Done. > >> --- /dev/null >> +++ b/libavformat/rtmpenc.c >> @@ -0,0 +1,532 @@ >> + >> +#include "internal.h" >> +#include "rtmp.h" >> +#include "rtmpdh.h" >> +#include "rtmpenc.h" >> +#include "url.h" >> + >> +#include "libavutil/rc4.h" > > Move the libavutil #include before the others. Done. > >> +static unsigned int get_dh_offset(const uint8_t *buf) >> +{ >> + int i, dh_offset = 0; >> + >> + for (i = 768; i < 772; i++) >> + dh_offset += buf[i]; >> + dh_offset = (dh_offset % 632) + 8; > > pointless () Done. > >> +static const uint32_t bf_sinit[][256] = { >> + >> + /* S-Box 0 */ >> + { 0xd1310ba6, 0x98dfb5ac, 0x2ffd72db, 0xd01adfb7, 0xb8e1afed, 0x6a267e96, >> + 0xba7c9045, 0xf12c7f99, 0x24a19947, 0xb3916cf7, 0x0801f2e2, 0x858efc16, > > 4-spaces indentation please, more below > >> +#define BF_ENC(X,S) \ >> + (((S[0][X>>24] + S[1][X>>16 & 0xff]) ^ S[2][(X>>8) & 0xff]) + S[3][X & >> 0xff]) > > Please give the operators some room to breathe. > >> +static void bf_enc(uint32_t *x, bf_key *key) >> +{ >> + uint32_t Xl; >> + uint32_t Xr; >> + uint32_t temp; >> + int i; > > 4 space indentation > >> + for (i = 0; i < BF_ROUNDS; ++i) { >> + Xl ^= key->p[i]; >> + Xr ^= BF_ENC(Xl,key->s); > > space after comma > >> + temp = Xl; >> + Xl = Xr; >> + Xr = temp; > > align the = > >> +static void bf_setkey(const unsigned char *kp, int keybytes, bf_key *key) >> +{ >> + uint32_t data, d[2]; >> + int i, j, k; >> + >> + memcpy(key->p, bf_pinit, sizeof(key->p)); >> + memcpy(key->s, bf_sinit, sizeof(key->s)); >> + >> + j = 0; > > Merge declaration and initialization. > >> + for (i = 0; i < BF_ROUNDS + 2; ++i) { >> + data = 0x00000000; >> + for (k = 0; k < 4; ++k) { >> + data = (data << 8) | kp[j]; >> + j = j + 1; >> + if (j >= keybytes) { >> + j = 0; >> + } > > Drop the {} > >> + key->s[i][j] = d[0]; >> + key->s[i][j + 1] = d[1]; > > Align the =. > >> + /* input is little-endian */ >> + d[0] = in[0] | (in[1] << 8) | (in[2] << 16) | (in[3] << 24); >> + d[1] = in[4] | (in[5] << 8) | (in[6] << 16) | (in[7] << 24); > > pointless () > >> + out[0] = d[0] & 0xff; >> + out[1] = (d[0] >> 8) & 0xff; >> + out[2] = (d[0] >> 16) & 0xff; >> + out[3] = (d[0] >> 24) & 0xff; >> + out[4] = d[1] & 0xff; >> + out[5] = (d[1] >> 8) & 0xff; >> + out[6] = (d[1] >> 16) & 0xff; >> + out[7] = (d[1] >> 24) & 0xff; > > same > > Right-align the numbers and the & for extra neatness. I'll update the part of code which implements Blowfish later. > >> --- /dev/null >> +++ b/libavformat/rtmpenc.h >> @@ -0,0 +1,58 @@ >> + >> +#ifndef AVFORMAT_RTMPE_H >> +#define AVFORMAT_RTMPE_H >> + >> +#include "url.h" > > Add stdint.h, the header needs it. Could you explain why, please? > > Diego > _______________________________________________ > libav-devel mailing list > [email protected] > https://lists.libav.org/mailman/listinfo/libav-devel -- Best regards, Samuel Pitoiset. _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
