On Wed, Nov 26, 2014 at 04:37:51PM -0600, Christian Suloway wrote: > Signed-off-by: Christian Suloway <csulo...@globaleagleent.com> > ---
> libavformat/crypto.c | 233 ++++++++++++++++++++++++++++---
> libavformat/hlsenc.c | 387
> ++++++++++++++++++++++++++++++++++++++++++++++-----
these should be split in 2 or more patches
also each should contain a verbose commit messsage and each should be
atomic and self contained not mix unrelated changes
> 2 files changed, 567 insertions(+), 53 deletions(-)
>
> diff --git a/libavformat/crypto.c b/libavformat/crypto.c
> index a9b6e47..dc3dc88 100644
> --- a/libavformat/crypto.c
> +++ b/libavformat/crypto.c
> @@ -38,17 +38,35 @@ typedef struct {
> int indata, indata_used, outdata;
> int eof;
> uint8_t *key;
> - int keylen;
> + int key_len;
> uint8_t *iv;
> - int ivlen;
> + int iv_len;
cosmetic changes should be seperate from functional ones
that is, in a seperate patch
> - struct AVAES *aes;
> + uint8_t *decrypt_key;
> + int decrypt_key_len;
> + uint8_t *decrypt_iv;
> + int decrypt_iv_len;
> + uint8_t *encrypt_key;
> + int encrypt_key_len;
> + uint8_t *encrypt_iv;
> + int encrypt_iv_len;
> + struct AVAES *aes_decrypt;
> + struct AVAES *aes_encrypt;
> +
> + uint8_t pad[BLOCKSIZE];
> + int pad_len;
> +
> } CryptoContext;
>
> #define OFFSET(x) offsetof(CryptoContext, x)
> #define D AV_OPT_FLAG_DECODING_PARAM
> +#define E AV_OPT_FLAG_ENCODING_PARAM
> static const AVOption options[] = {
> - {"key", "AES decryption key", OFFSET(key), AV_OPT_TYPE_BINARY, .flags =
> D },
> - {"iv", "AES decryption initialization vector", OFFSET(iv),
> AV_OPT_TYPE_BINARY, .flags = D },
> + {"key", "AES encryption/decryption key", OFFSET(key),
> AV_OPT_TYPE_BINARY, .flags = D|E },
> + {"iv", "AES encryption/decryption initialization vector", OFFSET(iv),
> AV_OPT_TYPE_BINARY, .flags = D|E },
> + {"decryption_key", "AES decryption key",
> OFFSET(decrypt_key), AV_OPT_TYPE_BINARY, .flags = D },
> + {"decryption_iv", "AES decryption initialization vector",
> OFFSET(decrypt_iv), AV_OPT_TYPE_BINARY, .flags = D },
> + {"encryption_key", "AES encryption key",
> OFFSET(encrypt_key), AV_OPT_TYPE_BINARY, .flags = E },
> + {"encryption_iv", "AES encryption initialization vector",
> OFFSET(encrypt_iv), AV_OPT_TYPE_BINARY, .flags = E },
> { NULL }
> };
>
> @@ -72,28 +90,145 @@ static int crypto_open2(URLContext *h, const char *uri,
> int flags, AVDictionary
> goto err;
> }
>
> - if (c->keylen < BLOCKSIZE || c->ivlen < BLOCKSIZE) {
> - av_log(h, AV_LOG_ERROR, "Key or IV not set\n");
> - ret = AVERROR(EINVAL);
> - goto err;
> + if (flags & AVIO_FLAG_READ) {
> + if (!c->decrypt_key_len) {
> + if (!c->key_len) {
> + av_log(h, AV_LOG_ERROR, "decryption key not set\n");
> + ret = AVERROR(EINVAL);
> + goto err;
> + } else if (c->key_len != BLOCKSIZE) {
> + av_log(h, AV_LOG_ERROR, "invalid key size "
> + "(%d bytes, block size is %d)\n",
> + c->key_len, BLOCKSIZE);
> + ret = AVERROR(EINVAL);
> + goto err;
> + }
> + c->decrypt_key = av_malloc(c->key_len);
> + if (!c->decrypt_key) {
> + ret = AVERROR(ENOMEM);
> + goto err;
> + }
> + memcpy(c->decrypt_key, c->key, c->key_len);
> + c->decrypt_key_len = c->key_len;
> + } else if (c->decrypt_key_len != BLOCKSIZE) {
> + av_log(h, AV_LOG_ERROR, "invalid decryption key size "
> + "(%d bytes, block size is %d)\n",
> + c->decrypt_key_len, BLOCKSIZE);
> + ret = AVERROR(EINVAL);
> + goto err;
> + }
> + if (!c->decrypt_iv_len) {
> + if (!c->iv_len) {
> + av_log(h, AV_LOG_ERROR, "decryption IV not set\n");
> + ret = AVERROR(EINVAL);
> + goto err;
> + } else if (c->iv_len != BLOCKSIZE) {
> + av_log(h, AV_LOG_ERROR, "invalid IV size "
> + "(%d bytes, block size is %d)\n",
> + c->iv_len, BLOCKSIZE);
> + ret = AVERROR(EINVAL);
> + goto err;
> + }
> + c->decrypt_iv = av_malloc(c->iv_len);
> + if (!c->decrypt_iv) {
> + ret = AVERROR(ENOMEM);
> + goto err;
> + }
> + memcpy(c->decrypt_iv, c->iv, c->iv_len);
> + c->decrypt_iv_len = c->iv_len;
> + } else if (c->decrypt_iv_len != BLOCKSIZE) {
> + av_log(h, AV_LOG_ERROR, "invalid decryption IV size "
> + "(%d bytes, block size is %d)\n",
> + c->decrypt_iv_len, BLOCKSIZE);
> + ret = AVERROR(EINVAL);
> + goto err;
> + }
that looks like the same code twice with different variables
this should be factored into a seperate function
also there are more occurances of similar looking code
[...]
> +static int hex_string(char **string, uint8_t *buf, int size)
> +{
> + int i;
> +
> + *string = av_mallocz(size*2 + 1);
> + if (!*string)
> + return AVERROR(ENOMEM);
> + for (i = 0; i < size; i++)
> + snprintf(&(*string)[i*2], 3, "%02X", buf[i]);
the len parameter of snprintf() is intended to be the remaining
length of the array. setting that to a constant like that can
be dangerous
also for large enough size, size*2+1 will overflow
also theres ff_data_to_hex()
> +
> + return 0;
> +}
> +
> +
> +static void hls_free_segment(HLSSegment *en)
> +{
> + av_free(en->filename);
> + av_free(en->key_uri);
> + av_free(en->iv_string);
> + av_free(en);
these should be av_freep()
[...]
> + if ((ret = hls_encryption_init(s) < 0))
the () are wrong
[...]
> #define OFFSET(x) offsetof(HLSContext, x)
> #define E AV_OPT_FLAG_ENCODING_PARAM
> static const AVOption options[] = {
> - {"start_number", "set first number in the sequence",
> OFFSET(start_sequence),AV_OPT_TYPE_INT64, {.i64 = 0}, 0, INT64_MAX, E},
> - {"hls_time", "set segment length in seconds",
> OFFSET(time), AV_OPT_TYPE_FLOAT, {.dbl = 2}, 0, FLT_MAX, E},
> - {"hls_list_size", "set maximum number of playlist entries",
> OFFSET(max_nb_segments), AV_OPT_TYPE_INT, {.i64 = 5}, 0, INT_MAX,
> E},
> - {"hls_ts_options","set hls mpegts list of options for the container
> format used for hls", OFFSET(format_options_str), AV_OPT_TYPE_STRING, {.str =
> NULL}, 0, 0, E},
> - {"hls_wrap", "set number after which the index wraps",
> OFFSET(wrap), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, E},
> - {"hls_allow_cache", "explicitly set whether the client MAY (1) or MUST
> NOT (0) cache media segments", OFFSET(allowcache), AV_OPT_TYPE_INT, {.i64 =
> -1}, INT_MIN, INT_MAX, E},
> - {"hls_base_url", "url to prepend to each playlist entry",
> OFFSET(baseurl), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, E},
> - {"hls_flags", "set flags affecting HLS playlist and media file
> generation", OFFSET(flags), AV_OPT_TYPE_FLAGS, {.i64 = 0 }, 0, UINT_MAX, E,
> "flags"},
> - {"single_file", "generate a single media file indexed with byte
> ranges", 0, AV_OPT_TYPE_CONST, {.i64 = HLS_SINGLE_FILE }, 0, UINT_MAX, E,
> "flags"},
> -
> + {"start_number", "set first number in the sequence",
> OFFSET(start_sequence),
> AV_OPT_TYPE_INT64, {.i64 = 0}, 0, INT64_MAX, E},
> + {"hls_time", "set segment length in seconds",
> OFFSET(time),
> AV_OPT_TYPE_FLOAT, {.dbl = 2}, 0, FLT_MAX, E},
> + {"hls_list_size", "set maximum number of playlist entries",
> OFFSET(max_nb_segments),
> AV_OPT_TYPE_INT, {.i64 = 5}, 0, INT_MAX, E},
> + {"hls_ts_options", "set hls mpegts list of options for the
> container format used for hls", OFFSET(format_options_str),
> AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, E},
> + {"hls_wrap", "set number after which the index wraps",
> OFFSET(wrap),
> AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, E},
> + {"hls_delete", "delete segment files that are no longer part
> of the playlist", OFFSET(delete_segments),
> AV_OPT_TYPE_INT, {.i64 = 0}, 0, 1, E},
> + {"hls_allow_cache", "explicitly set whether the client MAY (1) or
> MUST NOT (0) cache media segments", OFFSET(allowcache),
> AV_OPT_TYPE_INT, {.i64 = -1}, INT_MIN, INT_MAX, E},
> + {"hls_base_url", "url to prepend to each playlist entry",
> OFFSET(baseurl),
> AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, E},
> + {"hls_flags", "set flags affecting HLS playlist and media
> file generation", OFFSET(flags),
> AV_OPT_TYPE_FLAGS, {.i64 = 0}, 0, UINT_MAX, E, "flags"},
> + {"single_file", "generate a single media file indexed with byte
> ranges", 0,
> AV_OPT_TYPE_CONST, {.i64 = HLS_SINGLE_FILE}, 0, UINT_MAX, E, "flags"},
> + {"hls_segment_filename", "filename template for segment files",
> OFFSET(segment_filename),
> AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, E},
> + {"hls_key_info_file", "file with key URL and key file path",
> OFFSET(key_info_file),
> AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, E},
> + {"hls_random_iv", "randomize initialization vector",
> OFFSET(random_iv),
> AV_OPT_TYPE_INT, {.i64 = 0}, 0, 1, E},
> { NULL },
> };
this mixes functional and cosmetic changes
[...]
--
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
