ImageSharp
ImageSharp copied to clipboard
Jpeg encoder complete rewrite
Prerequisites
- [x] I have written a descriptive pull-request title
- [x] I have verified that there are no overlapping pull-requests open
- [x] I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules :cop:.
- [x] I have provided test coverage for my change (where applicable)
New architecture
Jpeg encoder now have a brand new architecture which allows us to define encoding 'configs' like this:
// YCbCr 4:1:0
new JpegFrameConfig(
JpegColorSpace.YCbCr,
JpegEncodingColor.YCbCrRatio410,
new JpegComponentConfig[]
{
new JpegComponentConfig(id: 1, hsf: 4, vsf: 2, quantIndex: 0, dcIndex: 0, acIndex: 0),
new JpegComponentConfig(id: 2, hsf: 1, vsf: 1, quantIndex: 1, dcIndex: 1, acIndex: 1),
new JpegComponentConfig(id: 3, hsf: 1, vsf: 1, quantIndex: 1, dcIndex: 1, acIndex: 1),
},
yCbCrHuffmanConfigs,
yCbCrQuantTableConfigs)
Which allow us to define any allowed subsampling setup or even a 'custom' scenario with any number of components if user needs to. This API is internal for now - user can only choose encoding mode via enum
which we had a long time ago. Maybe one day we can make it public - who knows.
Due to this change: https://github.com/SixLabors/ImageSharp/issues/1713 is resolved and https://github.com/SixLabors/ImageSharp/issues/1476 is resolved.
It would also be fairly easy to implement optimized huffman tables or progressive encoding with current architecture.
API changes
- All of these encoding modes are implemented now:
YCbCrRatio420 = 0,
YCbCrRatio444 = 1,
YCbCrRatio422 = 2,
YCbCrRatio411 = 3,
YCbCrRatio410 = 4,
Luminance = 5,
Rgb = 6,
Cmyk = 7,
Ycck = 8,
- Grayscale encoding now has a vectorized color conversion which gained a lot of performance.
~~3. I've removed Ycck
encoding simply because it's not popular and we didn't support it anyway (we still can decode YccK
though). This issue is partially done: https://github.com/SixLabors/ImageSharp/issues/808~~
- Added non-interleaved encoding for any color type.
Performance
Encoder got a bit slower but I think new architecture's worth it - we were really fast before, now we are still fast enough.
Main branch
Method | TargetColorSpace | Quality | Mean |
---|---|---|---|
Benchmark | Luminance | 75 | 6.923 ms |
Benchmark | Rgb | 75 | 12.116 ms |
Benchmark | YCbCrRatio420 | 75 | 6.484 ms |
Benchmark | YCbCrRatio444 | 75 | 8.263 ms |
Benchmark | Luminance | 90 | 7.046 ms |
Benchmark | Rgb | 90 | 13.349 ms |
Benchmark | YCbCrRatio420 | 90 | 6.714 ms |
Benchmark | YCbCrRatio444 | 90 | 9.198 ms |
Benchmark | Luminance | 100 | 9.057 ms |
Benchmark | Rgb | 100 | 19.099 ms |
Benchmark | YCbCrRatio420 | 100 | 10.194 ms |
Benchmark | YCbCrRatio444 | 100 | 15.372 ms |
PR
Method | TargetColorSpace | Quality | Mean |
---|---|---|---|
Benchmark | Luminance | 75 | 4.618 ms |
Benchmark | Rgb | 75 | 12.543 ms |
Benchmark | YCbCrRatio420 | 75 | 6.639 ms |
Benchmark | YCbCrRatio444 | 75 | 8.590 ms |
Benchmark | Luminance | 90 | 5.456 ms |
Benchmark | Rgb | 90 | 13.447 ms |
Benchmark | YCbCrRatio420 | 90 | 7.218 ms |
Benchmark | YCbCrRatio444 | 90 | 9.150 ms |
Benchmark | Luminance | 100 | 6.731 ms |
Benchmark | Rgb | 100 | 19.831 ms |
Benchmark | YCbCrRatio420 | 100 | 10.541 ms |
Benchmark | YCbCrRatio444 | 100 | 15.345 ms |
Need to write a ton of tests now....
Update
Added YccK encoding so this PR completely resolves https://github.com/SixLabors/ImageSharp/issues/808.
Hmm, this failed test passes on my local machine with .net6 and fails only on macos on .net7. Wondering if this is the same problem as in https://github.com/SixLabors/ImageSharp/issues/2117.
Hmm, this failed test passes on my local machine with .net6 and fails only on macos on .net7. Wondering if this is the same problem as in #2117.
I have not seen issue #2117 on mac yet, only on windows so far, but it could be related.
I will restart the CI to see, if it was a fluke or always happens.
I will restart the CI to see, if it was a fluke or always happens.
Looks like it's not a fluke. I'll start writing tests this week for this PR and try to add debug checks for memory access, maybe it's related to some out of bounds index access.
Finally had a moment to finish this:
- All colors are supported for encoding
- Implemented non-interleaved encoding aka a separate scan for each component
- Added tests and removed obsolete benchmark tests - we have proper benchmarks for that
- Restored
ProfilingSandbox.cs
@JimBobSquarePants @brianpopow
Looks like Skew
tests fails on net7 is still a thing. Everything else should be working just fine. I've added tests for all new output color types with a very precise tolerance.
New encoder architecture mirrors decoder architecture. It should allow us to write encoding expansions like optimized huffman table generation or progressive encoding.
P.S. Please be advised that this PR is a breaking change due to new jpeg output color type enum.
Very exciting! Will review this asap.
Looks like I've messed up linked issues, here's the complete list of what's resolved by this PR:
- https://github.com/SixLabors/ImageSharp/issues/1476 - all color conversions now support scalar/vector/avx paths
- https://github.com/SixLabors/ImageSharp/issues/808 - all output color types are supported
- https://github.com/SixLabors/ImageSharp/issues/1713 - only required quantization table(s) is(are) written
- https://github.com/SixLabors/ImageSharp/issues/1238 - CMYK input jpeg now should be resaved to CMYK output jpeg if not explicitely overriden in encoder settings