search

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.23k stars 474 forks source link

JPEGDEC: Sync with latest upstream #948

Open Gadgetoid opened 1 month ago

Gadgetoid commented 1 month ago

Mostly in response to #941

@bitbank2 there are a few commits here that might be appropriate upstream. Notably:

  1. JPEG_setFramebuffer is forward-declared static, but not defined as static
  2. ucColMask is defined but never used
  3. iTable throws a sign compare- I looked over the code and figured it could be a uint16_t rather than an int to fix this, but am not sure if I'm missing some case where it might need to go negative or VERY BIG.
Gadgetoid commented 1 month ago

Welp I just wasted my time trying to add support for a feature in a rejected pull request... :roll_eyes:

I guess this sync is probably still useful but I'm sticking it on hold for now. It would be more useful to update our bindings to better handle load/decode errors.

bitbank2 commented 1 month ago

Welp I just wasted my time trying to add support for a feature in a rejected pull request... 🙄

I guess this sync is probably still useful but I'm sticking it on hold for now. It would be more useful to update our bindings to better handle load/decode errors.

who rejected what pull request? I was going to take a look at this today.

Gadgetoid commented 1 month ago

who rejected what pull request? I was going to take a look at this today.

The linked issue linked to what I assumed was a merged commit to add cropping support. I failed to see the "Closed" at the top. :melting_face:

bitbank2 commented 1 month ago

I added JPEG cropping features and some other cleanup to my own copy, but hesitated to share it. I've been less willing to "give it all away" lately. The crop feature is done and waiting for me to share it, but I'm not sure how to move forward from here. I had a loss of faith in open source recently.

Gadgetoid commented 1 month ago

I had a loss of faith in open source recently.

Ooof. Dare I ask what happened?

To be clear, I wasn't asking for this specific feature, but since someone came along and said "hey it exists" I figured I'd support it. It just transpired that it didn't exist :laughing:

If you feel you need to hold back some shinies from the public domain for whatever reason- I totally understand!

Gadgetoid commented 1 month ago

Big ooof-

CRITICAL ERROR: Block device / binary overlap!
Binary ends at 0x100a0560, block device starts at 0x100a0000
Processing: build-pico/pico-6cbf0fa0654672e92c30b670069e87383523d95c-pimoroni-micropython.uf2
Gadgetoid commented 1 week ago

Looks like the change from:

https://github.com/pimoroni/pimoroni-pico/blob/f18f1ba259e80e6b6fce534ba16d9d26701154aa/libraries/jpegdec/jpeg.inl#L182-L224

to:

https://github.com/bitbank2/JPEGDEC/blob/4c864911ba49da30893df7e61c92834fa37f11c2/src/jpeg.inl#L256-L354

And similar, eats up more flash than we can spare. Some builds are really on the edge, so it's not surprising.

It looks like it should be perfectly possible to revert back to the older range tables, at the cost of output image quality and probably loss of SIMD support.

The difference is just a bit-shift:

https://github.com/pimoroni/pimoroni-pico/blob/f18f1ba259e80e6b6fce534ba16d9d26701154aa/libraries/jpegdec/jpeg.inl#L2131-L2133