On Tue, Dec 27, 2016 at 07:37:09 +0000, Jerry Jiang wrote:
> This is my first patch submitted to FFmpeg, so I'm sure that I missed
> something. Please bear with me. :P This patch implements the solution
> outlined here: https://guru.multimedia.cx/small-tasks-for-ffmpeg/
Apart from the newline-corrupted patch:
> + if (s->out_format == FMT_MJPEG && s->huffman == 2) {
^
[...]
> + if (s->huffman == 2) {
^
and
> +{ "huffman", "Huffman table strategy", OFFSET(huffman), AV_OPT_TYPE_INT, {
> .i64 = 1 }, 1, 2, VE, "huffman" },
^
> + { "default", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 1 }, INT_MIN, INT_MAX,
> VE, "huffman" },
^
> + { "optimal", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 2 }, INT_MIN, INT_MAX,
> VE, "huffman" },
^
For readability, '1' and '2', which are just arbitrary numbers, could
be expressed with enums (starting with 0, as we do in the C world ;-)).
> +static int compare_by_prob(const void *a, const void *b) {
> + PTable a_val = *(PTable *)a;
> + PTable b_val = *(PTable *)b;
> + if (a_val.prob < b_val.prob) {
> + return -1;
> + }
> + if (a_val.prob > b_val.prob) {
> + return 1;
> + }
> + return 0;
> +}
Doesn't ffmpeg have a macro for this? FFSIGN(a->prob - b->prob); ?
> +static int compare_by_length(const void *a, const void *b) {
> + HuffTable a_val = *(HuffTable *)a;
> + HuffTable b_val = *(HuffTable *)b;
> + if (a_val.length < b_val.length) {
> + return -1;
> + }
> + if (a_val.length > b_val.length) {
> + return 1;
> + }
> + return 0;
> +}
Same here. Perhaps a bit overengineered, unless I'm missing something.
Moritz
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
