You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
JPEG decoder will generate wrong pixel colors sometime #155
The above code will get the color of the pixel at (1000, 555) location, and the value of c will be RGB(249,243,222). However the actual color in this point should be RGB(250, 243, 224). This problem will further image processings such as saving it using SaveAsBmp method. However Photoshop and Micorosft Paint app can correctly decode the pixels.
System Configuration
ImageSharp version: 1.0.0-alpha4-00048
Other ImageSharp packages and versions: None
Environment (Operating system, version and so on): Windows 10 Build 1607 with Jan 2017 Update
.NET Framework version: .NET Core App 1.1 (the version of Micorosft.NETCore.App is 1.1.1)
changed the title [-]JPEG decoder will generate run pixel colors sometime[/-][+]JPEG decoder will generate wrong pixel colors sometime[/+]on Mar 26, 2017
@tocsoft We had a similar issue in our testing. I don't think it is an ICC issue, because ICC is non-destructive, so the underlying pixel is the same with or without ICC profile, only the monitor display color changes. I didn't test the image, but I bet if we open it in Photoshop and remove the ICC profile we will always get the same color values for this pixel.
I'd like to do a proper test against the supplied image comparing multiple decoders to see what the result is. There may be more variance than we think.
Thanks for the detailed feedback!
There are severalroundingconversions in the Jpeg Decoder code resulting in cumulated roundoff errors bigger than necessary.
I'm planning to do a floating-point-all-the-way refactor for Jpeg decoder; @JimBobSquarePants will implement a Vector4-based pixel type. Let's see the results after these changes have been implemented!
However I'm afraid we can't provide 100% numerical correctness. because ImageSharp codebase depends heavily on floating point SIMD operations.
I've been doing some experimenting in LinqPad with the following methods: An equivalent of our shifting method and a known, accurate formula.
Given an input YCbCr with the following values.
byte Y = 243;
byte Cb = 117;
byte Cr = 133;
We get the following results:
Shifting rgb (250, 243, 224)
Accurate rgb(250, 244, 223)
As you can see, our method is slightly inaccurate but nowhere near the level of inaccuracy that the later pixel value is. This means we must be somehow introducing error elsewhere.
publicvoidToRGBShift(bytey,bytecb,bytecr){intccb=cb-128;intccr=cr-128;// Speed up the algorithm by removing floating point calculation// Scale by 65536, add .5F and truncate value. We use bit shifting to divide the resultintr0=(int)((1.402F*65536)+.5F)*ccr;// (1.402F * 65536) + .5Fintg0=(int)((0.344136F*65536)+.5F)*ccb;// (0.344136F * 65536) + .5Fintg1=(int)((0.714136F*65536)+.5F)*ccr;// (0.714136F * 65536) + .5Fintb0=(int)((1.772F*65536)+.5F)*ccb;// (1.772F * 65536) + .5Fbyter=(byte)(y+(r0>>16)).Clamp(0,255);byteg=(byte)(y-(g0>>16)-(g1>>16)).Clamp(0,255);byteb=(byte)(y+(b0>>16)).Clamp(0,255);Color.FromArgb(255,r,g,b).Dump();}publicvoidToRGB(bytey,bytebcb,bytebcr){doublecb=bcb-128;doublecr=bcr-128;byter=(byte)(Math.Round(y+(1.402*cr),MidpointRounding.AwayFromZero).Clamp(0,255));byteg=(byte)(Math.Round(y-(0.344136*cb)-(0.714136*cr),MidpointRounding.AwayFromZero).Clamp(0,255));byteb=(byte)(Math.Round(y+(1.772*cb),MidpointRounding.AwayFromZero)).Clamp(0,255);Color.FromArgb(255,r,g,b).Dump();}
@antonfirsov So I would like to know how Photoshop and MS Paint do to decode the pixels, I don't think they both rely on the same system library, (Photoshop should has its own decoder and encoder implementations), and we should also be able to get the same result if they can do it since they are using the same CPU and its instuctions set are identical.
@JimBobSquarePants thank you for your reply, And for my understanding the direct implementation way is simple but slow, and you are making some accelations with errors inroduced? Yep the differences for pixels is slight, however for image processing targeted applications, these slight differences may also have a huge effect. I'm not saying we should copy the code of Photoshop since I believe coding for sciencific computation is a piece of cake for all your guys, however the method how Photoshop balances between effeiciency and accuracy may be more useful for ImageSharp :-)
I'm quite sure the Jpeg refactor I'm planning will bring benefits in both performance and accuracy.
It's a lot of work though, and my list is pretty long. :(
Yeah, I'm planning to open a "JpegDecoder Improvements" issue, summarizing all the Jpeg Decoder related TODO-s, so anyone who wants to join the project could pick it ;)
I've just pushed code that makes our color conversion code the same as libjpeg and ran some tests decoding the image and saving it as a bmp.
I'm still getting differences and the color conversion code is now not a contributing factor. I tested this by running both our fast and a high-precision version of the conversion code against the image.
I'm wondering whether it has something to do with our float/byte conversion when doing our DCT transform?
@antonfirsov I think I might be onto something with the float/byte conversion. Upping this to 127.5F get's us closer.
Reacted by Anton Firszov and Aristarkh Zagorodnikov
Activity
[-]JPEG decoder will generate run pixel colors sometime[/-][+]JPEG decoder will generate wrong pixel colors sometime[/+]tocsoft commentedon Mar 26, 2017
I guess will be this is due to the fact that this image has an ICC profile associated with it.
Currently we don't have ICC profile support, but we are currently actively working on getting it in.
blackcity commentedon Mar 26, 2017
@tocsoft We had a similar issue in our testing. I don't think it is an ICC issue, because ICC is non-destructive, so the underlying pixel is the same with or without ICC profile, only the monitor display color changes. I didn't test the image, but I bet if we open it in Photoshop and remove the ICC profile we will always get the same color values for this pixel.
JimBobSquarePants commentedon Mar 26, 2017
I'd like to do a proper test against the supplied image comparing multiple decoders to see what the result is. There may be more variance than we think.
antonfirsov commentedon Mar 26, 2017
Thanks for the detailed feedback!
There are several rounding conversions in the Jpeg Decoder code resulting in cumulated roundoff errors bigger than necessary.
I'm planning to do a floating-point-all-the-way refactor for Jpeg decoder; @JimBobSquarePants will implement a
Vector4-based pixel type. Let's see the results after these changes have been implemented!However I'm afraid we can't provide 100% numerical correctness. because ImageSharp codebase depends heavily on floating point SIMD operations.
JimBobSquarePants commentedon Mar 27, 2017
@sgjsakura @antonfirsov There's something more to this.
I've been doing some experimenting in LinqPad with the following methods: An equivalent of our shifting method and a known, accurate formula.
Given an input YCbCr with the following values.
We get the following results:
As you can see, our method is slightly inaccurate but nowhere near the level of inaccuracy that the later pixel value is. This means we must be somehow introducing error elsewhere.
sgjsakura commentedon Mar 27, 2017
@antonfirsov So I would like to know how Photoshop and MS Paint do to decode the pixels, I don't think they both rely on the same system library, (Photoshop should has its own decoder and encoder implementations), and we should also be able to get the same result if they can do it since they are using the same CPU and its instuctions set are identical.
JimBobSquarePants commentedon Mar 27, 2017
Wouldn't we all! 😄
The maths are pretty simple all in all, we just need to find a better middle ground between speed and accuracy than we have just now.
sgjsakura commentedon Mar 27, 2017
@JimBobSquarePants thank you for your reply, And for my understanding the direct implementation way is simple but slow, and you are making some accelations with errors inroduced? Yep the differences for pixels is slight, however for image processing targeted applications, these slight differences may also have a huge effect. I'm not saying we should copy the code of Photoshop since I believe coding for sciencific computation is a piece of cake for all your guys, however the method how Photoshop balances between effeiciency and accuracy may be more useful for ImageSharp :-)
antonfirsov commentedon Mar 27, 2017
I'm quite sure the Jpeg refactor I'm planning will bring benefits in both performance and accuracy.
It's a lot of work though, and my list is pretty long. :(
JimBobSquarePants commentedon Mar 27, 2017
@antonfirsov Don't let this stress you out my friend. We have plenty of time and I'm sure more people will join in to help out. 😄
antonfirsov commentedon Mar 27, 2017
Yeah, I'm planning to open a "JpegDecoder Improvements" issue, summarizing all the Jpeg Decoder related TODO-s, so anyone who wants to join the project could pick it ;)
JimBobSquarePants commentedon Apr 27, 2017
Update:
I've just pushed code that makes our color conversion code the same as libjpeg and ran some tests decoding the image and saving it as a bmp.
I'm still getting differences and the color conversion code is now not a contributing factor. I tested this by running both our fast and a high-precision version of the conversion code against the image.
I'm wondering whether it has something to do with our float/byte conversion when doing our DCT transform?
@antonfirsov I think I might be onto something with the float/byte conversion. Upping this to
127.5Fget's us closer.antonfirsov commentedon Apr 27, 2017
Yeah, maybe I could rethink the rounding logic here one day with a clean afternoon brain :)
antonfirsov commentedon Sep 16, 2017
@sgjsakura our Jpeg accuracy has been improved a lot with
beta-1(now available on NuGet). Do you find the results good enough now?JimBobSquarePants commentedon Nov 23, 2017
I'm gonna close this. Our testing shows a massive improvement