On 28 December 2016 at 06:22, Jerry Jiang <jerryjiang1...@gmail.com> wrote:
> -
> +{ "huffman", "Huffman table strategy", OFFSET(huffman), AV_OPT_TYPE_INT,
> { .i64 = HUFFMAN_TABLE_DEFAULT }, 1, 2, VE, "huffman" },
> + { "default", NULL, 0, AV_OPT_TYPE_CONST, { .i64 =
> HUFFMAN_TABLE_DEFAULT }, INT_MIN, INT_MAX, VE, "huffman" },
> + { "optimal", NULL, 0, AV_OPT_TYPE_CONST, { .i64 =
> HUFFMAN_TABLE_OPTIMAL }, INT_MIN, INT_MAX, VE, "huffman" },
> { NULL},
> };
>
You forgot to readjust the range on the huffman option, it should be 0, 1,
not 1, 2.
Apart from that I did some tests. The overall compression of the encoder is
improved by constant 15%, which is impressive. The performance overhead is
on average 7% which is insignificant compared to the compression
improvements, so I'd approve turning the option on by default (though I'll
agree its better if you submit a separate patch for that). The output with
huffman optimizations enabled showed a slight gain in SSIM, which is
probably from the rate control from mpegvideo_enc.
The only problem I have with the code is that it uses a linked list and
calls malloc during runtime. Couldn't the linked list be replaced with an
array allocated during init (it should be okay, I don't think resolution is
allowed to change during encoding)? That way it'll be way less messy than a
linked list and there'll be less things to go wrong. Performance will
probably improve too. You always iterate over all buffers anyway.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
