Skip to content

Conversation

RetiredWizard
Copy link

I think this is the second time I have found myself tracing back the code to lib/tjpgd/src/tjpgd.c:

		case 0xD9:	/* EOI */
			return JDR_FMT3;	/* Unsuppoted JPEG standard (may be progressive JPEG) */

before realizing that I needed to uncheck the progressive option when creating my JPEG file for CircuitPython.

This PR adds the progressive hint from the core comment to the reported gifio error. It does also increase the message size a little so I considered catching the error in the adafruit_imageload library decoder.open call, adding the "progressive" hint and then re-raising the error but I think that would add the hint to more unrelated errors than the core update does.

I'm happy to go the library route if the additional string storage is problematic.

@RetiredWizard
Copy link
Author

Either pre-commit updated circuitpython.pot or I messed something else up, I didn't mean to make changes to that file.

@dhalbert
Copy link
Collaborator

Either pre-commit updated circuitpython.pot or I messed something else up, I didn't mean to make changes to that file.

circuitpython.pot is updated when you change or add messages. Locally, make sure your branch is up to date, so it includes the latest messages. Then you can run make translate at the top level, which will update circuitpython.pot`. However, things might be OK now.

@RetiredWizard
Copy link
Author

This was the first time I've submitted a PR against an ESP32 change in a while and I made a couple of mis-steps during the branch creation. I could just give up on this one and create a new PR from a fresh branch.

Assuming this is a change that we want, let me know if you want me to do anything to clean this up.

@dhalbert
Copy link
Collaborator

I think this is a worthwhile change. You could decide if you want it against 10.0.x (so it will go in the next stable bugfix release or main (will go to 10.1.0-beta.x series). Just bring your repo up to date, then apply the single message change, then do make translate, and then push the commit and make the PR.

@RetiredWizard
Copy link
Author

You could decide if you want it against 10.0.x (so it will go in the next stable bugfix release or main (will go to 10.1.0-beta.x series).

This isn't really a "bug" that needs fixing right away so main is probably fine. I'm thinking that if it goes in main, it's a little easier because you don't have to manually re-merge it later. If I have that backwards let me know...

@dhalbert
Copy link
Collaborator

I'm thinking that if it goes in main, it's a little easier because you don't have to manually re-merge it later. If I have that backwards let me know...

We would regularly merge from 10.0.x to main before any 10.1.x release, which is usually trivial. So 10.0.x is actually a little easier.

@RetiredWizard
Copy link
Author

I'm going to close this because I don't know git well enough to repair my local branch. It will be far easier to recreate a clean local branch against 10.0.x and resubmit the change with that.

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

Successfully merging this pull request may close these issues.

2 participants