bitbank2 / PNGdec

Arduino PNG image decoder library
Apache License 2.0
162 stars 25 forks source link

[RP2040, ARDUINO, PLATFORMIO] PNGDec decode causes infinite loop #25

Open basgoossen opened 4 weeks ago

basgoossen commented 4 weeks ago

Describe the bug When running pngDEC with the example octocat files on a RP2040 (platformIO, both earlephilhower cora and pico core). the png header is read correctly, however a call to png.decode(nullptr, 0); results in an infinite loop (in zlib inflate).

To Reproduce Steps to reproduce the behavior:

  1. Create a project in platform IO for the rp2040 (pico core or earlephilhower).
  2. include PNGdec.h and an octocat.h file of preference.
  3. call png.openRAM(octocat, len, drawPNGLine);
  4. verify returned value to be 0 (PNG_SUCCESS);
  5. run png.decode(nullptr,0);
  6. Device will be stuck in an infinite loop from line: err = inflate(&d_stream, Z_NO_FLASH, iOptions & PNG_CHECK_CRC);

Expected behavior The expected behaviour whould be that the png is decoded and for each line of image drawPNGLine is called.

Image you're having trouble with example Octocat files from the PNGdec library.

basgoossen commented 4 weeks ago

Correction, it does not enter an infinite loop, it just crashes the RP2040. (infinite loop was created by adding debug lines and the omitted {} in the infinite for loop / switch loop in inflate.

The CPU seems to crash on a write to put++ on the line: put++ = (unsigned char)(state->length);

I can't seem to find where and how the put pointer is initialized, the loop crashes when it writes a value from state->length to put pointing to address 0x2000D640

Ok traced put back to probably strm->next_out;

strm->next_out is last set to pCurr (which in this case still on the first round seems to be set to pPage->ucPixels which is initialized as a uint8_t[5122] in PNGIMAGE _png, the private variable of PNG.

Debug info: Drawing PNG Image! Read imageBuffer from PROGMEM: size: 1800, head: 89 50 4E 47 openRAM, _png.ucPixels address: 2000D640 PNGDec decode read 32b from RAM, fileSize: 1800, iPos: 32 Successfully read png image: image specs: (240 x 200), 8 bpp, pixel type: 3 PNGDec decode read 1792b from RAM, fileSize: 1800, iPos: 1800 PNGDec read 1792b from file to file buffer. (from iPos: 8, fileSize: 1800) d_stream.next_out set to address: 2000D640, size: 4 Starting deflate on d_stream: (in 0, out 0), cur: 2000D640, head(00 00 00 00) state->mode: 16191 state->mode: 16196 state->mode: 16205 writing to put * 2000D640 -> CRASH

bitbank2 commented 4 weeks ago

I believe this is due to the RP2040 having an exception for unaligned pointer access. I think I regressed this code to undo that optimization. Clone the latest code instead of using the last release and see if it's resolved. If not, try removing "ALLOWS_UNALIGNED" from inffast.c

basgoossen commented 4 weeks ago

First of all thanks for your reply! I've tested both proposed solutions, they however do not change the situation. The master in Github indeed is different from the library version that PlatformIO offers, but also the newer version (directly cloned from github) still causes the freeze in the same location. I'm actually quite confused why, since it seems like a pretty ligit write to a initialized pointer location.

Actually the proposed solution evenlually lead to a fix! After removing the ALLOWS_UNALIGNED blocks from both the inflate.c and inffast.c it worked!

The RP2040 does not like the UNALIGNED access it seems. It has some unstriped memory area's though, maybe it is possible to force a variable in those blocks to fix the issue as well and keep the performance of UNALIGNED access? just guessing here.

Just a remark, but should maybe have a different topic, the image still comes out pretty mangled. Colors are all over the place and beside some parts that seem to resemble a shape, most of it seems to be noise. So it seems there is still some issues with the RP2040 and PNGDec besides the UNALIGNED crash.

That seems to have been caused by the debugging lines, reloading the code only modifying the UNALIGNED blocks fixes the issue and produces good PNG representation. Thanks.

bitbank2 commented 4 weeks ago

Did you change the current code on Github to get it to work? When you say "removed the unaligned blocks" do you mean you disabled the #define or actually deleted the lines? The code on Github should work unchanged on your RP2040. If you confirm this, I'll do a new release.

basgoossen commented 4 weeks ago

Hi BitBank2, First i removed the entire #ifdef blocks, but later i also tested removing the 2 occasions of:

define ALLOWS_UNALIGNED

In both inflate.c and inffast.c, and i can confirm that the latter also works.

Thanks again for your efforts and for creating this tool.