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

[Jerry Jiang]

Thanks for looking at our patch!

> What's the performance impact like when you enable it? Any reason why it
> shouldn't be made the default?

>From local testing on converting a random video using the mjpeg codec, we
were able to encode at a speed of 207 fps with the -huffman optimal flag, a
speed of 224 fps without the flag, and a speed of 232 fps if the patch was
reverted. I can supply more thorough results if you give me some idea of
what you want. We chose to gate the behaviour behind the flag because
originally our implementation suffered performance issues, and because
(since this is our first time submitting a patch) we didn't want to change
the existing behaviour. We can change the flag/make it default if you wish.

> 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.

I ran the following commands, which is what I think you wanted:

# ls -l temp/test.png
-rw-r--r--@ 1 tpcstld  staff  3118 27 Dec 03:59 temp/test.png
# ./ffmpeg -y -i temp/test.png -huffman optimal temp/optimal.jpg
# ./ffmpeg -y -i temp/test.png  temp/default.jpg
# ls -l temp/*.jpg
-rw-r--r--  1 tpcstld  staff  9501 27 Dec 04:19 temp/default.jpg
-rw-r--r--  1 tpcstld  staff  7128 27 Dec 04:18 temp/optimal.jpg
# ./ffmpeg -i temp/default.jpg -i temp/optimal.jpg -filter_complex ssim -f
null -
[Parsed_ssim_0 @ 0x7f8ea0510440] SSIM Y:1.000000 (inf) U:1.000000 (inf)
V:1.000000 (inf) All:1.000000 (inf)

I'm happy to provide anything else you may want too.

> Is all the error checking required? Couldn't the encode_block() function
do
> error checking which just returns instantly if there was a failure?

The reason why the return code is even needed is to handle av_malloc
returning NULL. Since av_malloc is not called always called inside
encode_block it makes things a bit complicated. Lastly, we needed to store
the error return code (or that it errored) to act on it in the future. This
is why we chose to code it this way.