Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Png's are decoding wrong after 1.0.0-alpha-000061 #28

Closed
KLuuKer opened this issue Nov 30, 2016 · 4 comments · Fixed by #30
Closed

Png's are decoding wrong after 1.0.0-alpha-000061 #28

KLuuKer opened this issue Nov 30, 2016 · 4 comments · Fixed by #30

Comments

@KLuuKer
Copy link

KLuuKer commented Nov 30, 2016

png's are decoded incorrectly on version v1.0.0-alpha-000062 and higher (including 95)

last know good version is v1.0.0-alpha-000061
so i think something went wrong in this commit 27522da

i think it's the same as #24 but this time it's in the decoder, i only have 1 specific image that gives me the issue. all of my other images decode&encode just fine (on version 95)

tested with output format: png,jpeg,bmp,gif (all give the same issue)
used the same simple console app as in #24 (comment)

please don't forget to sleep this time 😄

original source file:
source-file

single-thread output:
single-thread

multi-thread output:
bad-file

@KLuuKer
Copy link
Author

KLuuKer commented Nov 30, 2016

it could also be a issue with the png itself, because if i resave the image with irfanview it multithread decodes & encodes correctly

overview

@JimBobSquarePants
Copy link
Member

Hmmmm... Probably the ArrayPool stuff again. Odd, it really doesn't seem to be threadsafe.

Would you be able to fork the repo and conduct your tests with a reverted decoder? I'll test myself when I can.

@KLuuKer
Copy link
Author

KLuuKer commented Nov 30, 2016

Well i checked and the ArrayPool is definitely thread safe

BUT you will eventually get MORE bytes than you asked for. the docs say it will return a array of MINIMAL the specified length. so great care must be taken in using the bytes that are returned by the pool.

https://github.com/JimBobSquarePants/ImageSharp/blob/master/src/ImageSharp/Formats/Png/PngDecoderCore.cs#L338
i replaced those pool rent's with new byte[this.bytesPerScanline];
and that worked, so the issue would probably be in the processing of
https://github.com/JimBobSquarePants/ImageSharp/blob/master/src/ImageSharp/Formats/Png/PngDecoderCore.cs#L365
as that doesn't have the correct length passed to it

sorry if it sounds like i'm ranting, it must be hard trying to keep your head straight with this huge amount of code

@JimBobSquarePants
Copy link
Member

Och not at all, at least it looks like it should be an easy fix then. Would you like to give it a go? You have the advantage of tests that work.

I really need to beef up my parallel test.

@KLuuKer KLuuKer mentioned this issue Dec 1, 2016
antonfirsov pushed a commit that referenced this issue Jan 18, 2020
Changed duplicate InlineData from MinValue to MaxValue
JimBobSquarePants added a commit that referenced this issue Feb 17, 2021
Changed duplicate InlineData from MinValue to MaxValue
JimBobSquarePants added a commit that referenced this issue Feb 17, 2021
Changed duplicate InlineData from MinValue to MaxValue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants