Jpeg 12 bit support #784

feliwir · 2018-12-06T22:42:46Z

Description

Adds 12-bit jpeg support as implemented in libjpeg and libjpeg-turbo
Adds a 12-bit jpeg testfile
Make jpeg decoder bitdepth invariant

CLAassistant · 2018-12-06T22:42:53Z

All committers have signed the CLA.

JimBobSquarePants

Great effort here, really appreciated!

A couple of minor changes required plus you're failing the Stylecop checks so if you can get those fixed we're rocking.

src/ImageSharp/Common/Tuples/Vector4Pair.cs

JimBobSquarePants · 2018-12-09T20:01:12Z

src/ImageSharp/Formats/Jpeg/Components/Block8x8F.Generated.tt

        {
+            Vector4 CMin4 = new Vector4(0F);


These values are being calculated more than once in the same partial struct.

What we can do is to refactor the normalization logic into it's own stateful type (preferably a struct), where minimum/maximum/half values are nonstatic members initialized in the constructor.

JimBobSquarePants · 2018-12-09T20:02:15Z

src/ImageSharp/Formats/Jpeg/Components/Decoder/ColorConverters/JpegColorConverter.cs

+        private float MaximumValue { get; }
+
+        /// <summary>
+        /// Gets the maximum value of a sample


Repeated description

JimBobSquarePants · 2018-12-12T00:43:06Z

It occurs to me that this adds decoding support not encoding so the image data will be lost. We should get the work done to allow supporting encoding also.

antonfirsov

@feliwir good job, and thanks again for this improvement!

Just a quick initial web review, will clone the code later this week. Still left a few imrovement requests in comments.

Question:

Is it possible to have 12 bit precision jpegs for colorspaces other than YCbCr? (eg. CMYK, YCCK, RGB)
If yes, can we get some test images for these cases. adding them to our test suite?

antonfirsov · 2018-12-12T23:44:02Z

src/ImageSharp/Formats/Jpeg/Components/Block8x8F.Generated.tt

        {
+            Vector4 CMin4 = new Vector4(0F);


What we can do is to refactor the normalization logic into it's own stateful type (preferably a struct), where minimum/maximum/half values are nonstatic members initialized in the constructor.

antonfirsov · 2018-12-12T23:47:44Z

src/ImageSharp/Formats/Jpeg/Components/Decoder/ColorConverters/JpegColorConverter.FromCmyk.cs

@@ -10,8 +10,8 @@ internal abstract partial class JpegColorConverter
    {
        internal class FromCmyk : JpegColorConverter
        {
-            public FromCmyk()
-                : base(JpegColorSpace.Cmyk)
+            public FromCmyk(int precision)


I prefer to have more type safety in the codebase. (-1, 17 and 222 are invalid values for precision ;) ). I suggest an enum having the following form, so you can easily cast it to int when necessary.

enum JpegPrecision { EightBits = 8, TwelveBits = 12 }

Hmmm... Read section 4.7 here. Lossless jpeg supports additional precision so we should bear that in mind when naming our enum. https://www.w3.org/Graphics/JPEG/itu-t81.pdf

For DCT-based processes, two alternative sample precisions are specified: either 8 bits or 12 bits per sample. Applications which use samples with other precisions can use either 8-bit or 12-bit precision by shifting their source image samples appropriately. The baseline process uses only 8-bit precision. DCT-based implementations which handle 12-bit source image samples are likely to need greater computational resources than those which handle only 8-bit source images. Consequently in this Specification separate normative requirements are defined for 8-bit and 12-bit DCT-based processes.

For lossless processes the sample precision is specified to be from 2 to 16 bits

Oops. The options I see:

Keep the type as is (int), and introduce some DebugGuard checks in the code

Introduce an enum defining all entries for 2-16

Since we do not support lossless jpeg, define only EightBits and TwelveBits for now, and YAGNI out the rest, letting our future selves to find out how to manage the lossless case.

I'm pretty much for option 3.

antonfirsov · 2018-12-12T23:50:05Z

tests/ImageSharp.Tests/Formats/Jpg/JpegColorConverterTests.cs

@@ -44,7 +44,7 @@ public JpegColorConverterTests(ITestOutputHelper output)
        public void ConvertFromYCbCrBasic(int inputBufferLength, int resultBufferLength, int seed)
        {
            ValidateRgbToYCbCrConversion(
-                new JpegColorConverter.FromYCbCrBasic(),
+                new JpegColorConverter.FromYCbCrBasic(8),


We should add tests for 12 bit precision case.

JimBobSquarePants · 2018-12-13T00:05:16Z

@antonfirsov

Is it possible to have 12 bit precision jpegs for colorspaces other than YCbCr? (eg. CMYK, YCCK, RGB)

Yes, when 12bit support was added to the jpeg specification it covered all color spaces. On that note we cannot preserve that color data when saving the jpeg. We'll need to add the equivalent colorspace transformation code to the encoder with any additional requirements.

antonfirsov · 2018-12-13T00:37:13Z

@JimBobSquarePants then we should find or generate 12 bit test jpegs for all the colorspaces, the feature is only partially tested otherwise!
We can decode the reference output with the ImageMagick reference decoder. The encoder could be implemented/tested separately.

JimBobSquarePants · 2018-12-13T01:00:15Z

@antonfirsov Just spent the last 20 mins scouring the internet for an official source of test images and I've come up with nothing. libjpeg-turbo's repo contains 1 12bit file that doesn't appear to be supported by anything I have on my machine. https://github.com/libjpeg-turbo/libjpeg-turbo/tree/master/testimages

antonfirsov · 2018-12-13T11:11:16Z

Ok, this means that those aren't real use cases on pure statistical basis, so we can leave them untested.

Standard meets reality.

JimBobSquarePants · 2018-12-21T07:18:12Z

@feliwir @antonfirsov I had some time so I updated everything in the branch, removed some of the Linq stuff from the color converter and merged the reference image.

We're getting very close to having this ready to merge 🎉

Question though. Shall we had encoding 12bit jpegs to this PR or open a new Issue/PR. We'll need to create optimized color conversion methods to handle this.

feliwir · 2018-12-22T17:37:56Z

We could add encoding, but not sure if i could help with that too much :D

JimBobSquarePants · 2018-12-29T12:45:19Z

@antonfirsov When you have a chance can you please conduct a final review?

antonfirsov · 2018-12-29T19:26:01Z

I the improvement opportunities I mentioned are still there (introducing an enum for 8bit/12bit, test cases for 12 bit color conversion), but let's just merge this as-is, and do the improvements later!

JimBobSquarePants · 2018-12-30T08:15:21Z

Merged. Thanks @feliwir !

Jpeg 12 bit support

Start work on jpeg 12 bit support

5ee5da8

feliwir added 2 commits December 9, 2018 12:26

Work on 12-bit support

ede120d

Add missing Non-SIMD implementation

5ef3f19

JimBobSquarePants reviewed Dec 9, 2018

View reviewed changes

Fix style issues

b17bda5

feliwir changed the title ~~[WIP] Jpeg 12 bit support~~ Jpeg 12 bit support Dec 10, 2018

Fix style issues

4636fe6

antonfirsov requested changes Dec 12, 2018

View reviewed changes

JimBobSquarePants added 3 commits December 21, 2018 11:56

Merge remote-tracking branch 'upstream/master' into jpeg-12bit

8cba23f

Minor optimizations

133c854

Update reference images and decoder

537dbcf

antonfirsov approved these changes Dec 29, 2018

View reviewed changes

JimBobSquarePants merged commit db0fc0d into SixLabors:master Dec 30, 2018

qdraw mentioned this pull request Apr 13, 2019

JpegEncoder producing incorrect output on linux-arm (since Beta006) #871

Closed

5 tasks

antonfirsov pushed a commit to antonfirsov/ImageSharp that referenced this pull request Nov 11, 2019

Merge pull request SixLabors#784 from feliwir/jpeg-12bit

a5a387e

Jpeg 12 bit support

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jpeg 12 bit support #784

Jpeg 12 bit support #784

feliwir commented Dec 6, 2018

CLAassistant commented Dec 6, 2018 •

edited

Loading

JimBobSquarePants left a comment

JimBobSquarePants Dec 9, 2018

antonfirsov Dec 12, 2018

JimBobSquarePants Dec 9, 2018

JimBobSquarePants commented Dec 12, 2018

antonfirsov left a comment

antonfirsov Dec 12, 2018

antonfirsov Dec 12, 2018

JimBobSquarePants Dec 13, 2018

antonfirsov Dec 13, 2018

antonfirsov Dec 12, 2018

JimBobSquarePants commented Dec 13, 2018 •

edited

Loading

antonfirsov commented Dec 13, 2018

JimBobSquarePants commented Dec 13, 2018

antonfirsov commented Dec 13, 2018

JimBobSquarePants commented Dec 21, 2018

feliwir commented Dec 22, 2018

JimBobSquarePants commented Dec 29, 2018

antonfirsov commented Dec 29, 2018

JimBobSquarePants commented Dec 30, 2018

Jpeg 12 bit support #784

Jpeg 12 bit support #784

Conversation

feliwir commented Dec 6, 2018

Description

CLAassistant commented Dec 6, 2018 • edited Loading

JimBobSquarePants 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

JimBobSquarePants commented Dec 12, 2018

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 Dec 13, 2018 • edited Loading

antonfirsov commented Dec 13, 2018

JimBobSquarePants commented Dec 13, 2018

antonfirsov commented Dec 13, 2018

JimBobSquarePants commented Dec 21, 2018

feliwir commented Dec 22, 2018

JimBobSquarePants commented Dec 29, 2018

antonfirsov commented Dec 29, 2018

JimBobSquarePants commented Dec 30, 2018

CLAassistant commented Dec 6, 2018 •

edited

Loading

JimBobSquarePants commented Dec 13, 2018 •

edited

Loading