-
Notifications
You must be signed in to change notification settings - Fork 496
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
JPEGDEC: Sync with latest upstream #948
base: main
Are you sure you want to change the base?
Conversation
35a4222
to
f740f06
Compare
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. |
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. 🫠 |
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. |
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 😆 If you feel you need to hold back some shinies from the public domain for whatever reason- I totally understand! |
f740f06
to
67dd81f
Compare
Big ooof-
|
Looks like the change from: pimoroni-pico/libraries/jpegdec/jpeg.inl Lines 182 to 224 in f18f1ba
to: 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: pimoroni-pico/libraries/jpegdec/jpeg.inl Lines 2131 to 2133 in f18f1ba
|
Mostly in response to #941
@bitbank2 there are a few commits here that might be appropriate upstream. Notably:
JPEG_setFramebuffer
is forward-declared static, but not defined as staticucColMask
is defined but never usediTable
throws a sign compare- I looked over the code and figured it could be auint16_t
rather than anint
to fix this, but am not sure if I'm missing some case where it might need to go negative or VERY BIG.