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

Write JFIF header when saving JPEG #4639

Merged
merged 2 commits into from
Jun 20, 2020
Merged

Conversation

radarhere
Copy link
Member

Resolves #3328

Pillow has some C code that sets density_unit, X_density and Y_density. However, libjpeg states that

UINT8 density_unit
UINT16 X_density
UINT16 Y_density
The resolution information to be written into the JFIF marker;
not used otherwise.

So this code should tell libjpeg to write the JFIF marker.

@hugovk
Copy link
Member

hugovk commented May 23, 2020

Is this testable?

@radarhere
Copy link
Member Author

Yes, because it also turns out that this resolves #1676. So I've added a test to the commit.

assert test(300) == (300, 300)
assert test(100, 200) == (100, 200)
assert test(0) is None # square pixels
for test_image_path in [TEST_FILE, "Tests/images/pil_sample_cmyk.jpg"]:
Copy link
Contributor

@nulano nulano May 25, 2020

Choose a reason for hiding this comment

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

I would suggest using pytest.mark.parameterize in future tests, helps #4193. In my experience, it is a lot easier to both read and debug. WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, and I've pushed a commit for this. It'll just take me a while to internalise that this is the new way of doing things.

@hugovk hugovk merged commit bcb8cbb into python-pillow:master Jun 20, 2020
@radarhere radarhere deleted the jfif branch June 20, 2020 12:57
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.

Why is it not possible to set jfif data when saving a jpeg file?
3 participants