pimoroni / pimoroni-pico

Libraries and examples to support Pimoroni Pico add-ons in C++ and MicroPython.
https://shop.pimoroni.com/collections/pico
MIT License
1.31k stars 495 forks source link

Display Pack 2.0 / JPEGDEC behaviour - files vs bytes #435

Closed Rookeh closed 7 months ago

Rookeh commented 2 years ago

Hi,

I have a Pico W and a Display Pack 2.0, with which I am attempting to obtain a JPEG image from a URL (a small one, 320x240, approx 12kb) and display it on the screen.

If I use urequests to GET the image, then pass the response.content to JPEGDEC's open_RAM function, the Pico W locks up with OSError: [Errno 2] ENOENT and needs to be power cycled.

However, if I instead take the bytes from the response, dump them to a file, then pass the same file to JPEGDEC using open_file, the image displays fine.

This seems a bit odd, surely we should be able to pass bytes to this library instead of saving to and reading from the same file, putting extra wear on the flash? Is there some functionality of JPEGDEC that is not implemented yet?

I am testing with 1.19.2.

TIA!

Gadgetoid commented 2 years ago

Have you tried reading the response data into a bytearray and transferring that? Is "response.content" actually bytes, or a string? Or a buffer?

I usually do something like this:

socket = urequest.urlopen(url)
data = bytearray(1024 * 10)
socket.readinto(data)
socket.close()

And JPEGDEC will happily work with the data bytearray.

Rookeh commented 2 years ago

The response.content I get back does appear to be a bytes object (e.g. b'\x00\x00...') and not a string - I haven't tried explicitly converting to a bytearray, will give that a shot later.

Gadgetoid commented 2 years ago

Well I guess this would explain it:

https://github.com/pimoroni/pimoroni-pico/blob/33b2c3908df63f6e3951bbe93789661ae5526dbe/micropython/modules/jpegdec/jpegdec.cpp#L215-L233

The first check is explicitly for a string/bytes which always gets treated as a filename, whereas a buffer (bytearray) gets treated as raw data because it falls through to the second case.

The first check should probably be mp_obj_is_str albeit I think mp_get_buffer_raise will fail on bytes so we'd need to squeeze in a third case, albeit I have no idea what that would look like.

I suspect open_RAM(bytearray(response.content)) might work, but might also cause a temporary allocation for a bytearray of the content size which could be nasty. I wonder if open_RAM(memoryview(response.content)) works...

(All my boards are locked away beyond the searing midday heat :grimacing:)

Rookeh commented 2 years ago

Aha, yep that would definitely explain the current behaviour. Cool. I will have a play around after work and see if I can get this working. (Definitely with you on the heat...it's "only" 35° here so far, we were forecast 39°.) 🔥

Rookeh commented 2 years ago

Yep, wrapping the bytes object with memoryview before handing it off to open_RAM did the trick - although the conversion has to happen at the last minute; if I attempt to have my API helper function return a memoryview object directly, I get MemoryError: memory allocation failed, allocating 17820 bytes when constructing the decoder object with decoder = jpegdec.JPEG(self.display)

But, if I have the helper function just return bytes, then I do not get the above exception, this lets execution continue to decoder.open_RAM(memoryview(imgBytes)), and everything works normally from that point on.

So, seems like a bit of an odd workaround, but I guess issues like this are bound to crop up in such a RAM constrained environment. Everything now seems to be working fine, so I'll close this issue. Cheers for the help!

skorokithakis commented 1 year ago

@Rookeh can you post some code for how you got this working? I'm trying the same thing as you but getting RuntimeError: JPEG: could not read file/buffer., which seems odd. The image data looks ok in the inspector, even.

Gadgetoid commented 1 year ago

This is, I think, a bug in JPEGDEC that I caught in PNGDEC, the line in question is:

https://github.com/pimoroni/pimoroni-pico/blob/fc777ff0cabac7fddb8e21d20719a063d4f5982c/micropython/modules/jpegdec/jpegdec.cpp#L252

It checks to see if the filename/buffer supplied is a string or bytes, and treats it as a filename if so. It should only check if it's a string.

Edit: Well apparently I already said this but I didn't fix or even recall the bug despite casually fixing it in PNGDEC!? Can you say "Crippling ADHD." Sigh.

skorokithakis commented 1 year ago

@Gadgetoid sorry, it turned out that my JPEG was progressive. It works without that, even though I still can't figure out how to make the Pico deep-sleep on a timer.

Gadgetoid commented 7 months ago

Closing as fixed!