Jpeg image decoded as grayscale #464

Activity

1. JimBobSquarePants commented on Feb 14, 2018

   

   JimBobSquarePants

   on Feb 14, 2018

   Member

   More actions

   Thanks @denisivan0v for all the info and the sample program. We really, really, appreciate it when people like you make the extra effort. 👍

   @antonfirsov This is an odd one.

   JpegSnoop is reporting a YCCK color profile same as our YCCK sample in the repo so I don't know off the top of my head what the cause would be.

   *** Marker: APP14 (xFFEE) ***
     OFFSET: 0x00000014
     Length            = 14
     DCTEncodeVersion  = 25600
     APP14Flags0       = 0
     APP14Flags1       = 0
     ColorTransform    = 2 [YCCK]
   
   

   Could you have a look when you can?I'm not gonna have a chance for at least a week and a half.
2. JimBobSquarePants

   added

   bug

   formats:jpeg

   on Feb 14, 2018
3. antonfirsov commented on Feb 15, 2018

   

   antonfirsov

   on Feb 15, 2018

   Member

   More actions

   Maybe I can check this in the next few days. Will definitely do it for beta3.

   👍React with 👍1Reacted by James Jackson-South
4. JimBobSquarePants commented on Feb 21, 2018

   

   JimBobSquarePants

   on Feb 21, 2018

   Member

   More actions

   I've just checked this using the PDFJS port and it decodes correctly. I should be able to use it as a reference for the Golang one.

   👍React with 👍1Reacted by Denis Ivanov
5. JimBobSquarePants commented on Feb 21, 2018

   

   JimBobSquarePants

   on Feb 21, 2018

   Member

   More actions

   Ok.... The conversion algorithm itself looks correct, the input data less so.

   For [0,0] the k value should be 255 but we're getting 128. The Golang port is simply too complicated for me to understand without a lot of picking apart and testing so I don't know why that is the case. Maybe we are pulling the wrong component list somehow. cb and cr are both 128.

   I still find the readability of that decoder an issue. I only wish we'd managed to make the other one a bit faster, I've forgotten most of the code I wrote for it but I could understand the pipeline more.

   jpeg is hard ☹️
6. antonfirsov commented on Feb 21, 2018

   

   antonfirsov

   on Feb 21, 2018

   Member

   More actions

   @JimBobSquarePants I will also try to check it as soon as I'm finished with the MemoryManager stuff.

   Can you point me to the code parts you investigated? ("k value")
7. JimBobSquarePants commented on Feb 21, 2018

   

   JimBobSquarePants

   on Feb 21, 2018

   Member

   More actions

   @antonfirsov Yeah of course.

   The PDFJs method that I used to determine the correct k value is here.

   ImageSharp/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsYCbCrToRgbTables.cs

   Line 102 in 98a7773

   public static void PackYccK<TPixel>(ref TPixel packed, byte y, byte cb, byte cr, byte k)

   For the Golang port we are looking here.

   ImageSharp/src/ImageSharp/Formats/Jpeg/Common/Decoder/ColorConverters/JpegColorConverter.FromYccK.cs

   Line 18 in 98a7773

   public override void ConvertToRGBA(ComponentValues values, Span<Vector4> result)

   Using the sample image we should get input YCbCrk values for each component at [0,0] of 0, 128, 128, 255 which when converted to RGBA gives us white. The Golang values are 0, 128, 128, 128.
8. antonfirsov commented on Feb 21, 2018

   

   antonfirsov

   on Feb 21, 2018

   Last edited by antonfirsov

   Member

   More actions

   @JimBobSquarePants as far as I understand, the conversion methods are correct in both decoders, they are just consuming the wrong k in the "golang" case. I guess the "golang" spectral data is wrong for the input provided in this issue (the "pdfjs" one is inaccurate for others).

   We have these tests to verify the spectral coefficients produced in the stream parsing phase directly against libjpeg-turbo launched as a separate process (windows only execution).

   Currently the pdfjs spectral test cases are skipped (many failing!) but you can enable them to play around. For the input in this issue I expect the opposite: pdfjs fails & golang succeeds. I mean: golang fails pdfjs succeeds - seems like bed time for me! :)

   Any further investingation, or even a branch with new failing spectral tests is a progress IMO 😄
9. JimBobSquarePants commented on Feb 22, 2018

   

   JimBobSquarePants

   on Feb 22, 2018

   Member

   More actions

   @JimBobSquarePants as far as I understand, the conversion methods are correct in both decoders, they are just consuming the wrong k in the "golang" case. I guess the "golang" spectral data is wrong for the input provided in this issue (the "pdfjs" one is inaccurate for others).

   @antonfirsov Yeah that's right, the maths is correct, just the input is funky. I'll see what I can do.
10. antonfirsov

    added a commit that references this issue on Feb 28, 2018

    added failing tests for #464

    69db4e9
11. JimBobSquarePants

    added a commit that references this issue on Feb 28, 2018

    Fix #464

    This issue will close once commit 8c97fa4 is merged into the 'main' branch.

    8c97fa4
12. antonfirsov

    added a commit that references this issue on Feb 28, 2018

    removing the "Issue" label from #464-s test image + ExifUndefType.jpg…

    ...

    0760cb7
13. JimBobSquarePants

    closed this as completedin 8c97fa4on Feb 28, 2018
14. antonfirsov

    added 2 commits that reference this issue on Nov 11, 2019

    added failing tests for SixLabors#464

    2998a80

    removing the "Issue" label from SixLabors#464-s test image + ExifUnde…

    ...

    cc03c23