Skip to content

Fix Pillow examples using frombuffer#535

Merged
BoboTiG merged 3 commits into
BoboTiG:mainfrom
jholveck:pil-example-fix
Jun 2, 2026
Merged

Fix Pillow examples using frombuffer#535
BoboTiG merged 3 commits into
BoboTiG:mainfrom
jholveck:pil-example-fix

Conversation

@jholveck

@jholveck jholveck commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

The Pillow docs recommend always using the decoder arguments when using frombuffer; see
https://pillow.readthedocs.io/en/stable/reference/Image.html

Changes proposed in this PR

  • Tests added/updated - N/A
  • Documentation updated
  • Changelog entry added
  • ./check.sh passed

jholveck added 2 commits June 1, 2026 19:39
The Pillow docs recommend always using the decoder arguments when
using frombuffer; see
https://pillow.readthedocs.io/en/stable/reference/Image.html
@jholveck
jholveck marked this pull request as ready for review June 2, 2026 03:54
Copilot AI review requested due to automatic review settings June 2, 2026 03:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates Pillow usage across tests, docs examples, and demos to follow Pillow’s recommended Image.frombuffer raw decoder argument form, and adjusts v11.0.0 release notes around ScreenShot.raw.

Changes:

  • Add explicit raw decoder args (..., "BGRX", 0, 1) to PIL.Image.frombuffer usage in tests, docs examples, and demos.
  • Update v11.0.0 release notes to describe ScreenShot.raw as deprecated (not removed) and add a documentation highlights section.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/tests/third_party/test_pil.py Makes Image.frombuffer call explicit about raw decoder args in tests.
docs/source/release-history/v11.0.0.md Adjusts ScreenShot.raw lifecycle wording; adds documentation highlights entry about Pillow decoder args.
docs/source/examples/pil.py Updates documented Pillow example to pass explicit raw decoder args (incl. commented alternative).
demos/video-capture-simple.py Updates demo conversion from MSS BGRA to Pillow image with explicit raw decoder args.
demos/tinytv-stream.py Updates stream generator to create Pillow image with explicit raw decoder args.
demos/tinytv-stream-simple.py Updates simple TinyTV demo to create Pillow image with explicit raw decoder args.
demos/cat-detector.py Updates demo image conversion step to use explicit raw decoder args.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread docs/source/examples/pil.py
Comment thread docs/source/release-history/v11.0.0.md Outdated
@jholveck

jholveck commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Sorry, accidentally turned on the switch to automatically have Copilot check all my PRs.

jholveck added a commit to jholveck/python-mss that referenced this pull request Jun 2, 2026
In the release notes, one change was accidentally in PR BoboTiG#535 instead,
and one change was omitted entirely.  Fix.
@BoboTiG
BoboTiG merged commit c27a14e into BoboTiG:main Jun 2, 2026
18 checks passed
BoboTiG pushed a commit that referenced this pull request Jun 26, 2026
* Make the data views writable.

Previously (while working on 11.0), we changed the .bgra and .rgb
attributes to be read-only memoryviews.  We also removed the .raw
attribute.

The main reason we made the memoryviews read-only is so that we don't
have to worry about cached values of .pixels and .rgb being in-sync
with the underlying buffer, if users mutated the pixel data.

However, I now realize that this takes away a possible valuable use
case: ctypes.  ctypes can only create an array (or pointer) from a
buffer if it's writable; it doesn't support a read-only version.
Previously, users could make a ctypes pointer using the .raw
attribute.  The new API doesn't give them that ability, without
copying the data (or using ctypes' from_address, which is perilous).

Read-only buffers also trigger a warning from PyTorch, although it's
clearly-written and only is printed once.

This PR, as it stands, makes the `.bgra` and `.rgb` properties return
writable memoryviews.  (It also restores `.raw`, this time as a
deprecated alias of bgra, since there's no pressing need to remove the
name entirely.)

It also documents that, while these memoryviews are writable, actually
modifying the data may cause undefined behavior.

There's alternatives, of course.  Instead of what I've got here,
ChatGPT instead suggests that we leave .bgra and .rgb read-only (as
they were in 10.2, since they were `bytes` objects), and to add a
property like `.writable_bgra` or something.  Its argument is as
follows:

> The problem with making .bgra writable and saying “mutating is UB
> [undefined behavior]” is that Python users will absolutely see
> writable buffer and think “cool, in-place image editing.”  That’s
> not UB in the fun C sense; it’s more like “congratulations, you have
> a weird cache-invalidation footgun.”  The API shape would be lying a
> little.

I'm making this PR for discussion and consensus on the best way to go.

* Update release notes

In the release notes, one change was accidentally in PR #535 instead,
and one change was omitted entirely.  Fix.

* Apply suggestions from code review

Co-authored-by: Halldor Fannar <halldorfannar@users.noreply.github.com>

---------

Co-authored-by: Halldor Fannar <halldorfannar@users.noreply.github.com>
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.

3 participants