jamsinclair / jSquash

Browser & Web Worker focussed wasm bundles derived from the Squoosh App.
Apache License 2.0
222 stars 14 forks source link

@jsquash/png fails to decode some images #44

Closed CodeF53 closed 8 months ago

CodeF53 commented 8 months ago

Describe the bug Given some pngs, jsquash fails to decode and throws an error.

To Reproduce Steps to reproduce the behavior:

  1. Download broken.zip and unzip broken.png out of it
  2. Load broken.png's buffer
  3. call decode() on broken.png buffer

Reproduction In this reproduction, there is 2 images, working.png and broken.png. The reproduction loads each image and attempts to decode it.

Expected behavior

decoding ./src/broken.png
    success!

Actual behavior

decoding ./src/broken.png
error: Uncaught (in promise) Error: `unwrap_throw` failed
    throw new Error(getStringFromWasm0(arg0, arg1));
          ^
    at imports.wbg.__wbindgen_throw (https://unpkg.com/@jsquash/png@2.2.0/codec/squoosh_png.js?module:157:11)
    at <anonymous> (https://unpkg.com/@jsquash/png@2.2.0/codec/squoosh_png_bg.wasm:1:91616)
    at <anonymous> (https://unpkg.com/@jsquash/png@2.2.0/codec/squoosh_png_bg.wasm:1:40587)
    at decode (https://unpkg.com/@jsquash/png@2.2.0/codec/squoosh_png.js?module:109:18)
    at decode (https://unpkg.com/@jsquash/png@2.2.0/decode.js?module:23:27)
    at eventLoopTick (ext:core/01_core.js:181:11)
    at async tryDecode (file:///workspace/src/index.ts:6:21)
    at async file:///workspace/src/index.ts:13:1

Module Versions @jsquash/png 2.2.0 (latest)

Additional Context I have no reason to believe the broken.png in question has problematic encoding, as it functions perfectly with the following:

  1. Paint.NET
  2. Windows Photo viewer
  3. Discord's upload
  4. Github's upload
  5. Minecraft resource pack
jamsinclair commented 8 months ago

Thanks for the report @CodeF53. After playing around it seems like there might be an issue with the PNG decoding codec. Squoosh's implementation was quite basic and just repurposed the rust lib/png library.

When I find some time I'll see if I can find the exact error that is being reported in Rust and see if that helps us narrow it down. Alternatively, if you're running this code in the browser, you can likely utilize the Canvas API in the interim for a lightweight and faster png decoder to get the imagedata (See MDN docs).

jamsinclair commented 8 months ago

So the PNG itself is technically corrupted. It has an invalid ICC PROFILE checksum (Also reported by the tool https://www.nayuki.io/page/png-file-chunk-inspector). However, most systems seem to ignore this and still render the image.

The rust error is:

CRC error: expected 0x768af32e have 0x723989cd while decoding ChunkType { type: iCCP, critical: false, private: false, reserved: false, safecopy: false } chunk.

I'll need to make some time to comeback to this and see how to ignore this checksum check. In the interim, you could try use an image editor to resave the PNG correctly.

CodeF53 commented 8 months ago

(Repost because github mobile app showed I sent it twice. So I tried deleting the duplicate, only to have both comments dissappear. Thanks github mobile!)

I kind of expected it would be something like that, but thought it shouldn't matter because so many other applications don't seem to care.

I implemented a canvas based encode/decode fallback into my app for when this error occurs, but it is 10x slower than this library.

And that sucks because my app is used by people who aren't the person who made the images they are processing. So they have no clue their images could be improperly encoded. So my app just looks buggy/slow to them.

jamsinclair commented 8 months ago

@CodeF53 I've released @jsquash/png@v3.0.0 which should now work with your broken pngs 👌

As it was going to require a major version release, I also included some other breaking changes. Please see the PR (#45) or Changelog file for details 🙇 .

Hope this helps with your app, happy new year! 🎉

DeniDoman commented 5 months ago

@jamsinclair could you please double check your fix? Because in the same example: Reproduction - broken.png decoding is fixed, but working.png decoding now fails with the following error:

error: Uncaught (in promise) IndexSizeError: Failed to construct 'ImageData': The input data length is not equal to (4 width height).

jamsinclair commented 5 months ago

Thanks for the report @DeniDoman. Not sure how this worked before... there now seems to be a bug with how we handle non-RGBA PNG files.

I'll have a fix ready shortly.

jamsinclair commented 5 months ago

@DeniDoman, thanks for the wait! I believe version 3.0.1 should now fix the issue 🙌

Thanks for reporting the bug.

DeniDoman commented 5 months ago

Thank you for the fix, @jamsinclair ! And huge thank you for the project overall, you rock!

MeFelixWang commented 2 months ago

@jamsinclair thanks for this great project! I'm using "@jsquash/png": "^3.0.1", but Error: 'unwrap_throw' failed still exists, any updates?

Here is the png file:

king-vulture-8350517_640

jamsinclair commented 2 months ago

Thanks @MeFelixWang, that appears to be because the file is actually a jpeg and not a png. It has been given the wrong file extension.

Reading the file with the UNIX file command returns:

JPEG image data, JFIF standard 1.01, aspect ratio, density 1x1, segment length 16, progressive, precision 8, 640x451, components 3
jamsinclair commented 2 months ago

I'll see if I can improve the error messaging 🤔

MeFelixWang commented 2 months ago

@jamsinclair thanks so much! I never thought about the extension. 😭

jamsinclair commented 2 months ago

@MeFelixWang No worries, easy mistake. Happens to all of us! Best of luck with your project.

MeFelixWang commented 2 months ago

@jamsinclair Cheers for all the mistakes we made before! 😆