Skip to content
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

Fix cli crash during screenshot generation #884

Merged
merged 2 commits into from
Jan 9, 2024

Conversation

double-thinker
Copy link
Contributor

Describe the changes you have made:

Description of changes:

Implemented a fix that addresses an RGBA handling issue during screenshot generation when using IPython with auto-display.

The issue happens because show=True is set in Display.snapshot, and the screenshot generated on a Mac has an alpha channel (RGBA instead of RGB). When IPython attempts to display this image and convert it to JPEG, the process crashes (the exact cause is unclear).

This PR introduces a simple fix that enforces RGB mode in all cases.

I chose not to alter the default value of show because there may be use cases that depend on the current behavior, and this change is less likely to introduce new issues.

In the following commit, I performed a minor refactor related to the change I made: instead of saving/loading the image into a file that is neither used nor exposed, we now directly use the memory buffer. I separated this into another commit in case it causes any issues or if there are anticipated future uses for the file-based approach, making it easier to roll back or opt not to merge.

Thank you for the amazing project. ;)

Reference any relevant issues (e.g. "Fixes #000"):

Fixes #878

Pre-Submission Checklist (optional but appreciated):

  • I have included relevant documentation updates (stored in /docs)
  • I have read docs/CONTRIBUTING.md
  • I have read docs/ROADMAP.md

OS Tests (optional but appreciated):

  • Tested on Windows
  • Tested on MacOS
  • Tested on Linux

@KillianLucas
Copy link
Collaborator

Great fixes @double-thinker. I'm not sure why I went with the tempfile approach! This is much smarter, and the conversion sounds like a solid precaution/solution. Merging.

@KillianLucas KillianLucas merged commit cddae65 into OpenInterpreter:main Jan 9, 2024
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.

OSError: cannot write mode RGBA as JPEG
2 participants