search

saucecontrol / PhotoSauce

MagicScaler high-performance, high-quality image processing pipeline for .NET
http://photosauce.net/
MIT License
569 stars 49 forks source link

libpng throws bad adaptive filter will abort writing result #139

Closed svenclaesson closed 4 months ago

svenclaesson commented 8 months ago

Hi!

Image within attached project is partially corrupt but is shown in windows (relying on WIC codec) and browsers (that uses libpng) MagicScalerTest.zip This code will throw System.InvalidOperationException: 'Libpng decoder failed. bad adaptive filter value'

It works on windows when this part CodecManager.Configure(codecs => { codecs.UseLibpng(); }); is comment out and we fallback to the WIC codec. On Linux we can't rely on this to get around the issue.

You are already using the PNG_CRC_QUIET_USE flag to ignore CRC errors so some corrupt png images works.

I would expect the image to still be partially converted. Any input?

saucecontrol commented 8 months ago

Howdy! Thanks for the test case. My current test suite only has a limited collection of corrupt images, so there are definitely places where libpng might return an error that we could ignore. I prefer to err on the side of decoding whatever is possible, so this is something I'd like to improve.

saucecontrol commented 8 months ago

I had a quick look at the Chromium fork of libpng to see if they were doing something different here, and they're not, at least at the libpng level. https://source.chromium.org/chromium/chromium/src/+/main:third_party/libpng/pngread.c;l=551 https://source.chromium.org/chromium/chromium/src/+/main:third_party/libpng/pngpread.c;l=757

The issue is that libpng uses png_error here instead of png_benign_error, which is meant for recoverable faults and which I have set my wrapper to allow. Interestingly, Chrome looks like it stops decoding on the faulted row, whereas WIC appears to decode the remaining rows, which seem to clear up at the bottom of the image.

My original intent was to forward libpng errors that are the fault of the consumer, such as errors reading the input stream. I think we can probably easily take on the Chrome behavior of stopping decode and returning garbage (or empty) pixels on png_error as long as it's not related to the input stream read.

saucecontrol commented 4 months ago

I've finally published a new set of packages with this fix.