Skip to content

Pixeldata not correct when reading certain JPEG #245

Description

@devedse

Description

I'm writing a small tool that compares images after doing lossless optimizations on them using another tool: https://encode.ru/threads/1589-FileOptimizer

I took a picture with my own Camera (6000x4000) and ran it through the FileOptimizer. After that I used the tool I created to compare the source and target image to see if there were any pixel differences. The comparison failed on pixel X: 5990 Y: 3992

For image1 the debugger shows: 36, 15, 10, 255 (RGBA)
For image2 the debugger shows: 36, 15, 12, 255 (RGBA)

The strange this is, if I open the same images in Paint.NET the pixel value is: 38, 14, 12, 255 (RGBA). (And it's the same for both images).

image

I also tested what the pixel value would be of the images saved as BMP. In Paint.NET they read again as the same values we saw before:

  1. Image1: 36, 15, 10, 255 (RGBA)
  2. Image2: 36, 15, 12, 255 (RGBA)

Steps to Reproduce

Images that should be equal:
SourceImagesThatShouldBeEqual.zip

Code I'm using to reproduce the bug:

private static bool AreImagesEqual(string image1Path, string image2Path)
{
    var w = Stopwatch.StartNew();

    var image1 = Image.Load(image1Path);
    var image2 = Image.Load(image2Path);


    if (image1.Width != image2.Width || image1.Height != image2.Height)
    {
        return false;
    }

    int width = image1.Width;
    int height = image1.Height;

    using (var fs = new FileStream("image1.bmp", FileMode.Create, FileAccess.ReadWrite, FileShare.ReadWrite))
    {
        image1.SaveAsBmp(fs);
    }

    using (var fs = new FileStream("image2.bmp", FileMode.Create, FileAccess.ReadWrite, FileShare.ReadWrite))
    {
        image2.SaveAsBmp(fs);
    }

    for (int y = 0; y < height; y++)
    {
        for (int x = 0; x < width; x++)
        {
            var pixel1 = image1.Pixels[y * width + x];
            var pixel2 = image2.Pixels[y * width + x];

            if (pixel1 != pixel2)
            {
                if (pixel1.A == 0 && pixel2.A == 0)
                {
                    //Optimization that happens to better be able to compress png's sometimes
                    //Fully transparent pixel so the pixel is still the same no matter what the RGB values
                }
                else
                {
                    Console.WriteLine($"Failed, elapsed time: {w.Elapsed}");
                    return false;
                }
            }
        }
    }

    Console.WriteLine($"Elapsed time: {w.Elapsed}");

    return true;
}

System Configuration

  • ImageSharp version: Did a clone today at 06-06-2017
  • Other ImageSharp packages and versions:
  • Environment (Operating system, version and so on): Windows 10 with a WPF app using the ImageSharp library
  • .NET Framework version: 4.6.2 using ImageSharp as a reference
  • Additional information: -

Activity

  1. JimBobSquarePants commented on Jun 7, 2017

    @JimBobSquarePants
    Member

    Hi @devedse

    Thanks for supplying us with so much information; this is very odd indeed. I believe this relates to #155

    We need to get to the bottom of it.

    Cheers

    James

  2. devedse commented on Jun 7, 2017

    @devedse
    ContributorAuthor

    Aha, didn't do a good enough check through the open issues then, my apologies for that :). Do you know the piece of code/class that is most likely to cause this issue? I could also do some investigation to see if I can find out anything.

  3. JimBobSquarePants commented on Jun 7, 2017

    @JimBobSquarePants
    Member

    You're additional info is very important so don't worry. 😄 Tbh understanding jpeg eludes me. We're going wrong somewhere but I'm not sure where.

  4. antonfirsov commented on Jun 7, 2017

    @antonfirsov
    Member

    @devedse in the best case this might be caused by rounding inaccuracy in Block8x8F.CopyColorsTo(). Adding 0.5 before casting to bytes might help here.

    In the worst case we need a deep refactor of the codebase to fix this; introducing an elegant management of Speed <-> Memory <-> Quality tradeoffs (see #192).

  5. JimBobSquarePants commented on Jun 8, 2017

    @JimBobSquarePants
    Member

    @antonfirsov I had a look at the rounding there and we probably should be adding .5F there but that isn't the cause of our issue. I've previously checked the YCbCr conversion and that meets accuracy standards. (I used slow but high precision conversion to compare and our decoder was still incorrect) so it's not that either.

    The error must be elsewhere, I just don't know where that is. We're sometimes showing an error of +-3 per color component.

  6. devedse commented on Jun 8, 2017

    @devedse
    ContributorAuthor

    In the JpegDecoderCore I added a breakpoint if Pixel 5990, 3992 was being handled. I manually entered the YUV values in an online YUV to RGB converted and saw the same values as the ones that came out of your conversion function:
    image

    Website:
    https://www.mikekohn.net/file_formats/yuv_rgb_converter.php

    The expected YUV values:
    image

    For this specific pixel it seems that:
    Y: 20 instead of 21
    U: 122 (Correct)
    V: 140 instead of 139

    When I print out the surrounding values of the Y table for X values:
    5985: 22
    5986: 20
    5987: 20
    5988: 21
    5989: 21
    5990: 21 <---- Should be 20
    5991: 21
    5992: 19
    5993: 20
    5994: 20

    My next question would be, where does it fill the this.ycbcrImage.YChannel table?

  7. Toxantron commented on Jun 14, 2017

    @Toxantron
    Contributor

    @devedse quick advice: VS supports conditional breakpoints. Just right click on it and add the condition from the if statement. This way you can debug without manipulating the code.

  8. devedse commented on Jun 14, 2017

    @devedse
    ContributorAuthor

    @Toxantron, I know, I just don't like the way they work. This works easier for me ^.^

  9. devedse commented on Jun 18, 2017

    @devedse
    ContributorAuthor

    Did some more debugging through the code to find out where the 21 comes from:
    image

    Output:

    ValueSource: 21,36298
    ValueAsByte: 21
    

    Not sure if it'll help you guys anything?

  10. devedse commented on Jun 18, 2017

    @devedse
    ContributorAuthor

    I also did some tests using the following C++ script to see what the result would be of YUV to RGB conversion:

    void Test1()
    {
    	int width = 1;
    	int height = 1;
    
    
    	unsigned char* rgb_image = new unsigned char[width * height * 3]; //width and height of the image to be converted
    
    	int y;
    	int cr;
    	int cb;
    
    	double r;
    	double g;
    	double b;
    
    	int i = 0;
    	//first pixel
    	y = 21;
    	cb = 122;
    	cr = 139;
    
    	r = y + (1.4065 * (cr - 128));
    	g = y - (0.3455 * (cb - 128)) - (0.7169 * (cr - 128));
    	b = y + (1.7790 * (cb - 128));
    
    	//This prevents colour distortions in your rgb image
    	if (r < 0) r = 0;
    	else if (r > 255) r = 255;
    	if (g < 0) g = 0;
    	else if (g > 255) g = 255;
    	if (b < 0) b = 0;
    	else if (b > 255) b = 255;
    
    	rgb_image[i] = (unsigned char)r;
    	rgb_image[i + 1] = (unsigned char)g;
    	rgb_image[i + 2] = (unsigned char)b;
    
    	unsigned char rr = (unsigned char)r;
    	unsigned char gg = (unsigned char)g;
    	unsigned char bb = (unsigned char)b;
    
    	////second pixel
    	//y = yuyv_image[j + 2];
    	//cb = yuyv_image[j + 1];
    	//cr = yuyv_image[j + 3];
    
    	//r = y + (1.4065 * (cr - 128));
    	//g = y - (0.3455 * (cb - 128)) - (0.7169 * (cr - 128));
    	//b = y + (1.7790 * (cb - 128));
    
    	//if (r < 0) r = 0;
    	//else if (r > 255) r = 255;
    	//if (g < 0) g = 0;
    	//else if (g > 255) g = 255;
    	//if (b < 0) b = 0;
    	//else if (b > 255) b = 255;
    
    	//rgb_image[i + 3] = (unsigned char)r;
    	//rgb_image[i + 4] = (unsigned char)g;
    	//rgb_image[i + 5] = (unsigned char)b;
    }
    

    This also resulted in the same values for RGB:
    image

    Obtained from: https://stackoverflow.com/questions/9098881/convert-from-yuv-to-rgb-in-c-android-ndk

    This really leads me to think that the issue should be in or before the CopyColorsTo method

  11. antonfirsov commented on Jun 20, 2017

    @antonfirsov
    Member

    @devedse your feedback and investigation is really useful!

    I'm planning to have a deep dive back into the Jpeg decoder in ~4 weeks. I will start by analyzing and TDD-ing the IDCT + color conversion chain based on your observations.

  12. JimBobSquarePants commented on Jun 28, 2017

    @JimBobSquarePants
    Member

    @devedse Could you please try that image against the new decoder I have been building in the jpeg-port branch. I think the colors are all correct there.

  13. devedse commented on Jul 2, 2017

    @devedse
    ContributorAuthor

    @JimBobSquarePants I did some more testing and was initially positively surprised that Image1 now returned the right pixel data for pixel X: 5990 Y: 3992.

    However when I ran the 2 images through the whole compare again it failed on pixel X: 0 Y:0 directly :(.

    This time pixel1 (from image1) had the correct data, pixel2 (from image2) again differed by a tiny bit.

    image

  14. JimBobSquarePants commented on Jul 2, 2017

    @JimBobSquarePants
    Member

    I think there's always going to be minor differences in decoders. I've read that System.Drawing even differs slightly on 32 and 64bit machines.

  15. JimBobSquarePants commented on Jul 2, 2017

    @JimBobSquarePants
    Member

    I've tried to keep casting to an absolute minimum in my test decoder to reduce error.

  16. devedse commented on Jul 13, 2017

    @devedse
    ContributorAuthor

    I've changed up my library to now first use LibVIPS to convert the image from JPEG to PNG before doing the comparison. Doing that will actually give me equal images again.

    So basically (pseudo code):

    public void AreImagesEqual(image1, image2)
    {
        if (image1 == jpg)
              image1 = Vips.ConvertToPng(image1)
        if (image2 == jpg)
              image2 == Vips.ConvertToPng(image2)
    
        //Do the image comparison using ImageSharp here.
    }
    
  17. JimBobSquarePants commented on Jul 13, 2017

    @JimBobSquarePants
    Member

    @devedse We're in the midst of a complete rewrite of the jpeg decoder which will likely utilise floating point maths throughout (so we can use SIMD). We'll revisit this once we have completed that work.

  18. JimBobSquarePants commented on Oct 6, 2017

    @JimBobSquarePants
    Member

    I think we've done enough work now to ensure adequate accuracy for our decoder.

    Different decoders yield different results with varying levels of consistency per image. We're well within acceptable parameters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions