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

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

Open
basgoossen opened this issue May 28, 2024 · 5 comments
Open

Comments

@basgoossen
Copy link

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
Copy link
Author

basgoossen commented May 28, 2024

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
Copy link
Owner

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
Copy link
Author

basgoossen commented May 29, 2024

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
Copy link
Owner

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants