Skip to content

Conversation

@smason
Copy link
Contributor

@smason smason commented Dec 2, 2022

Fixes #6773

Changes proposed in this pull request:

  • a JPEG COM segment can now be written during Image.save
  • opening a JPEG file and saving to another file will now copy any existing comment across
  • user can now do image.save(filename, format='JPEG', comment='comment to be written') and the comment will be written to a COM segment

@radarhere radarhere changed the title support round-tripping JPEG comments Support round-tripping JPEG comments Dec 2, 2022
@radarhere radarhere added the JPEG label Dec 2, 2022
@smason
Copy link
Contributor Author

smason commented Dec 3, 2022

not sure what's going on with that MSYS2 MinGW 32-bit test failure, but it looks unrelated

@radarhere
Copy link
Member

There are some intermittent failures in our CIs. I've rerun the job, and it has passed now.

@radarhere radarhere changed the title Support round-tripping JPEG comments Support saving JPEG comments Dec 4, 2022
Use jpeg_write_marker to write comment
* means `comment=None` can be passed directly
* no need to conditionally run `str.encode()`
* clean up checking of whether a comment is passed
case 4:

if (context->comment_size > 0) {
if (context->comment) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the rest of the code is conditional on comment != NULL rather than comment_size > 0

they should be the same, but keeping things consistent feels nicer to me

Copy link
Member

Choose a reason for hiding this comment

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

When you say "the rest of the code", what are you referring to?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, that wasn't explained well

PyImaging_JpegEncoderNew seems to imply that comment_size is only valid when comment is non-NULL. e.g. comment is initialised to NULL, but comment_size isn't initialised to zero, and later in function (line 1109) it only sets comment = NULL

Free comment when returning early
@radarhere radarhere merged commit 378adeb into python-pillow:main Dec 7, 2022
mergify bot added a commit that referenced this pull request Dec 7, 2022
@smason
Copy link
Contributor Author

smason commented Dec 8, 2022

@radarhere thanks for all the help getting that merged!

sorry for the silence the past couple of days. I've been a bit ill, but recovering now

@smason smason deleted the write-jpeg-com branch December 12, 2022 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PIL doesn't write JPEG comment

2 participants