[FFmpeg-devel] [PATCH] Implement optimal huffman encoding for (M)JPEG.

[Rostislav Pehlivanov]

On 27 December 2016 at 07:37, Jerry Jiang <jerryjiang1128 at gmail.com> wrote:

> Hey everyone,
>
> 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/
>
>
Hi, thanks for your patch

It's good to see someone else work on encoders too!


> Activated by passing the flag "-huffman optimal" to the mjpeg encoder,
> otherwise the default huffman tables are used.
>

What's the performance impact like when you enable it? Any reason why it
shouldn't be made the default?
Encoders have a lot more lenience with respect to performance regressions
than decoders (given they affect quality) so if the encoding time isn't
doubled or so and the compression gains are decent I don't see a problem
having it on by default.

Would be nice to see SSIM or such comparisons as well. You can make such
using "ffmpeg -i <source> -i <image1> -filter_complex ssim -f null -" which
will work on compressed images. Be sure to give the compressed size (in
bytes) too.


> -        encode_block(s, block[0], 0);
> -        encode_block(s, block[2], 2);
> -        encode_block(s, block[4], 4);
> -        encode_block(s, block[8], 8);
> -        encode_block(s, block[5], 5);
> -        encode_block(s, block[9], 9);
> +        if (!ret) ret = encode_block(s, block[0], 0);
> +        if (!ret) ret = encode_block(s, block[2], 2);
> +        if (!ret) ret = encode_block(s, block[4], 4);
> +        if (!ret) ret = encode_block(s, block[8], 8);
> +        if (!ret) ret = encode_block(s, block[5], 5);
> +        if (!ret) ret = encode_block(s, block[9], 9);
>
>
Is all the error checking required? Couldn't the encode_block() function do
error checking which just returns instantly if there was a failure?

On first look this patch looks nice, I'll be able to do a more thorough
review and some testing once I get some sleep now.