Gif and Quantization Improvements #637

JimBobSquarePants · 2018-06-26T14:39:37Z

Prerequisites

I have written a descriptive pull-request title
I have verified that there are no overlapping pull-requests open
I have verified that I am following matches the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
I have provided test coverage for my change (where applicable)

Description

These changes do a few things (All related) Fixing #630 and #632

Adds palette options to gif encoding Global (default) vs Local. This means smaller animated gifs.
Improves performance and memory usage of quantizers ~3x faster
Improves performance and quality of dithering algorithms.

codecov · 2018-06-27T08:07:05Z

Codecov Report

Merging #637 into master will increase coverage by <.01%.
The diff coverage is 92.68%.

@@            Coverage Diff             @@
##           master     #637      +/-   ##
==========================================
+ Coverage   89.16%   89.16%   +<.01%     
==========================================
  Files         890      890              
  Lines       37889    37946      +57     
  Branches     2652     2661       +9     
==========================================
+ Hits        33782    33835      +53     
- Misses       3299     3302       +3     
- Partials      808      809       +1

Impacted Files	Coverage Δ
...ts/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs	`98.44% <ø> (ø)`	⬆️
...sts/Processing/Processors/Dithering/DitherTests.cs	`100% <ø> (ø)`	⬆️
...ssing/Quantization/Processors/QuantizeProcessor.cs	`0% <0%> (-100%)`	⬇️
.../Processing/Quantization/QuantizedFrame{TPixel}.cs	`87.5% <100%> (+12.5%)`	⬆️
...Dithering/Processors/PaletteDitherProcessorBase.cs	`100% <100%> (ø)`	⬆️
...sts/ImageSharp.Tests/Formats/GeneralFormatTests.cs	`97.43% <100%> (-0.04%)`	⬇️
src/ImageSharp/PixelFormats/Rgba64.cs	`97.26% <100%> (ø)`	⬆️
src/ImageSharp/Common/Constants.cs	`100% <100%> (ø)`	⬆️
...tion/FrameQuantizers/FrameQuantizerBase{TPixel}.cs	`97.91% <100%> (+0.77%)`	⬆️
...zation/FrameQuantizers/WuFrameQuantizer{TPixel}.cs	`94.38% <100%> (-0.29%)`	⬇️
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6949cde...ad8c2e2. Read the comment docs.

JimBobSquarePants · 2018-06-27T08:11:21Z

Ok. Good to review once tests have passed.

antonfirsov · 2018-06-27T10:10:12Z

tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs

-        private const float ToleranceThresholdForPaletteEncoder = 0.2f / 100;
+        // This is bull. Failing online for no good reason. 
+        // The images are an exact match. Maybe the submodule isn't updating?
+        private const float ToleranceThresholdForPaletteEncoder = 0.2273F;


Super-strange. I hope it's not my algorithm that sucks.

Maybe we could print the "last modified" timestamp of submodule files, or something like that.

It’s weird. The exact same difference before and after the update.

@JimBobSquarePants I think I figured it out!

Actually, it fails for me locally when I change the tolerance back to the original value. This is my actual output image for palette size 80:

It differs from both the current and the past version of the same image in the ReferenceOutput repository. Can you confirm that it means that the actual output of the execution on my PC differs from yours? This could be only explained by hardware (CPU) differences. I suspect a method like WuFrameQuantizer<T>.Volume() as the source.

If that is the case, this is exactly the reason why we have a tolerance in our comparer :) I think you have overshoot the tolerance however, based on my local tests 1.5F/100 should be good enough. It's not a percentage value in this class, you defined 22.73% actually! (I usually postfix my stuff with ***Percent when I mean percentage.)

Wow yeah! It's definitely different to the output on my machine. That explains that then.

I need to get my head around the tolerance values. At the moment writing 1.5F/100 (Is that 1% over 100 pixels?) isn't immediately intuitive and we should write a helper that makes it more clear.

@JimBobSquarePants tolerance == 0.01 means 1%, which can be interpreted as the following:

Either: An RGB image should be white, but 1% of white pixels are black (the rest is white)

Or: all the pixels in the image have a 1% difference compared to the expected values

I tried to explain this here but I think I wasn't clear enough.

ImageComparer.TolerantPercentage() is a helper that expects tolerance value scaled to 0-100. The value will be scaled back to the 0-1.0 interval internally. It's useful because tolerance values are typically very small.

That's much more clear, thanks!

JimBobSquarePants · 2018-06-27T12:14:21Z

Hey @iamcarbon Check this, 6x speedup!

With the new global palette mode we're down to 56 seconds with 11.3Mb output.

With the local palette mode it's 1 minute with 13Mb output.

Here's the two images to compare. Local, of course is flawless, global suffers slightly since the image reuses the palette from the first frame.

gifs.zip

iamcarbon · 2018-06-27T16:50:15Z

@JimBobSquarePants Heck yes! This is a gigantic speedup!!!!

antonfirsov · 2018-06-28T00:10:03Z

src/ImageSharp/Formats/Gif/GifColorTableMode.cs

+    public enum GifColorTableMode
+    {
+        /// <summary>
+        /// A single color table is calculated from the first frame and reused for subsequent frames.


We should probably allow the user to select the baseline frame.

I did think about this but if someone creates a custom PaletteQuantizer implementation then it doesn't matter what frame is used. Plus it would vary from image to image.

antonfirsov

The implementation looks good. We should probably consider allowing the selection of the palette frame to be used for quantization in GifColorTableMode.Global mode to support users with strange gifs.

antonfirsov · 2018-06-28T00:13:58Z

src/ImageSharp/Formats/Gif/GifEncoderCore.cs

-            this.WriteLogicalScreenDescriptor(image, stream, index);
+            // Write the LSD.
+            int index = this.GetTransparentIndex(quantized);
+            bool useGlobalTable = this.colorTableMode.Equals(GifColorTableMode.Global);


Do you swear you never coded in Java? 😄

I've read a lot of Java source 😄

antonfirsov · 2018-06-28T00:24:01Z

src/ImageSharp/Processing/Dithering/ErrorDiffusion/ErrorDiffuserBase.cs

@@ -74,9 +74,21 @@ public void Dither<TPixel>(ImageFrame<TPixel> image, TPixel source, TPixel trans
        {
            image[x, y] = transformed;

+            // Equal? Break out as there's nothing to pass.


There are still many optimization opportunities left here, but we need to change (extend?) the IErrorDiffuser API for that.

Not related to this PR but [MethodImpl(MethodImplOptions.AggressiveInlining)] is unnecessary on Dither<TPixel>(). The compiler won't be able to inline it.

I thought there were improvements to what can be inlined that would allow this. I could be wrong though. 🤷‍♂️

If you invoke a method through interface or virtual call on a base class, it can't be inlined, because the code is not known at JIT time. (The virtual invocation is responsible to dispatch the method at runtime.)

JimBobSquarePants added 9 commits June 24, 2018 01:57

Stub color table mode enum.

ad65b30

Can now encode gifs with global palette

b208be8

Minor improvements to diffusion

9b1270f

Use DistanceSquared

4ca807e

Use single cache base.

b71c74d

Improve lookup logic

cfdc75e

Make correct method virtual

becc80b

Smarter dithering.

07064e9

Merge branch 'master' into js/faster-gif

72acc7d

JimBobSquarePants added formats:png formats:gif area:quantizers labels Jun 26, 2018

JimBobSquarePants added 2 commits June 27, 2018 17:27

Refactor to better use base classes.

c3d38ba

Update tests

9a12d09

JimBobSquarePants added the area:performance label Jun 27, 2018

JimBobSquarePants changed the title ~~WIP Gif and Quantization Improvements~~ Gif and Quantization Improvements Jun 27, 2018

JimBobSquarePants requested review from antonfirsov, tocsoft and dlemstra June 27, 2018 08:11

JimBobSquarePants added this to the 1.0.0-beta5 milestone Jun 27, 2018

JimBobSquarePants mentioned this pull request Jun 27, 2018

[WIP] Quantization Refactoring #545

Closed

6 tasks

JimBobSquarePants added 2 commits June 27, 2018 18:41

Update reference images

4c04817

Handle CI craziness.

ab234b3

antonfirsov reviewed Jun 27, 2018

View reviewed changes

Merge branch 'master' into js/faster-gif

891f6eb

try to fine-tune tolerance in PngEncoderTests + better Rgba64.ToString()

93585fd

antonfirsov reviewed Jun 28, 2018

View reviewed changes

pushed a bad value accidentally in my previous commit

52c9ca4

antonfirsov approved these changes Jun 28, 2018

View reviewed changes

Merge branch 'master' into js/faster-gif

ad8c2e2

JimBobSquarePants merged commit d7bd82b into master Jun 28, 2018

JimBobSquarePants deleted the js/faster-gif branch June 28, 2018 04:37

This was referenced Jun 28, 2018

Diffuser modifies quantized image with same palette #632

Closed

GIF dithering causes massive encoding slowdown #630

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gif and Quantization Improvements #637

Gif and Quantization Improvements #637

JimBobSquarePants commented Jun 26, 2018 •

edited

Loading

codecov bot commented Jun 27, 2018 •

edited

Loading

JimBobSquarePants commented Jun 27, 2018

antonfirsov Jun 27, 2018

JimBobSquarePants Jun 27, 2018

antonfirsov Jun 28, 2018

JimBobSquarePants Jun 28, 2018

antonfirsov Jun 28, 2018 •

edited

Loading

JimBobSquarePants Jun 29, 2018

JimBobSquarePants commented Jun 27, 2018

iamcarbon commented Jun 27, 2018

antonfirsov Jun 28, 2018

JimBobSquarePants Jun 28, 2018

antonfirsov left a comment

antonfirsov Jun 28, 2018

JimBobSquarePants Jun 28, 2018

antonfirsov Jun 28, 2018

JimBobSquarePants Jun 28, 2018

antonfirsov Jun 28, 2018

Gif and Quantization Improvements #637

Gif and Quantization Improvements #637

Conversation

JimBobSquarePants commented Jun 26, 2018 • edited Loading

Prerequisites

Description

codecov bot commented Jun 27, 2018 • edited Loading

Codecov Report

JimBobSquarePants commented Jun 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antonfirsov Jun 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JimBobSquarePants commented Jun 27, 2018

iamcarbon commented Jun 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antonfirsov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JimBobSquarePants commented Jun 26, 2018 •

edited

Loading

codecov bot commented Jun 27, 2018 •

edited

Loading

antonfirsov Jun 28, 2018 •

edited

Loading