bitbank2 / JPEGDEC

An optimized JPEG decoder for Arduino
Apache License 2.0
365 stars 47 forks source link

Flip pixel buffer between draw calls to allow async DMA #49

Open MichaelBell opened 1 year ago

MichaelBell commented 1 year ago

The examples suggest that you can asynchronously DMA the pixel data to the LCD and then return from the draw callback without waiting for the DMA to finish.

However, when I tried this, I got corruption. The problem is that the decoder reuses the pixel buffer provided to the draw call, so depending on the relative speed of the screen and the decoding, the pixel data might get overwritten before it is sent to the screen.

The fix here splits the pixel buffer into two, resulting in ~twice as many draw calls, but allowing asynchronous DMA to be used safely.

bitbank2 commented 1 year ago

I'm not sure this is something that should be inside the JPEGDEC library. You can manage a ping-pong buffer scheme in your own code. It feels a bit too specific a feature and one of my goals of this library is to keep it as simple as possible. I'm not saying that your code is complicated, but it feels a bit outside of the scope of this library. What do you think?

MichaelBell commented 1 year ago

I appreciate this fix is simplistic, but it does allow the library to work in accordance with the documentation/examples.

From the wiki: "It also helps if your MCU can use DMA to write those pixels so that it doesn't have to wait for the each write transaction to finish." And in adafruit_gfx_demo.ino the example waits for DMA completion only on the next callback to the draw function, which risks this corruption.

Waiting for the DMA to complete before continuing the decode does noticeably reduce performance, so I think allowing for async DMA is in line with the goals of the library. However - maybe a better fix would be to make it easier for the user to provide (and ping-pong) the buffer to decode into? Currently that is hardcoded unless you are doing a dither decode (which I didn't handle in this PR).

bitbank2 commented 1 year ago

How about a compromise? Maybe have a "prep-for-DMA" flag that will split the buffer in 2 and do the ping-pong logic (default=off). That way people who don't know or can't use DMA won't have half of the buffer size, but expert users like yourself can enable that feature.

MichaelBell commented 1 year ago

Yes, that sounds like a good solution to me. I'll look at making that change.