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

Added reading of earlier ImageMagick PNG EXIF data #4471

Merged
merged 2 commits into from
Mar 26, 2020

Conversation

radarhere
Copy link
Member

Resolves #4460

Before 'eXIf' chunks became part of the PNG specification, ImageMagick stored EXIF data in a zTXt chunk. As a zTXt chunk, it is read into Pillow's info dictionary as "Raw profile type exif".

So this PR suggests reading that data as a fallback, if an eXIf chunk is not present.

Installing ImageMagick-7.0.6-0, I ran convert over Tests/images/exif.png and created an image with the zTXt chunk.

When reading, I skip the first two lines - see the corresponding ImageMagick code https://github.com/ImageMagick/ImageMagick/blob/9caa8b0c4269163def2ce0fa6ff1754ec0c234d2/coders/png.c#L7911-L7921

@radarhere radarhere added the Exif label Mar 13, 2020
@radarhere radarhere changed the title Added reading of earlier ImageMagick EXIF data Added reading of earlier ImageMagick PNG EXIF data Mar 13, 2020
@hugovk
Copy link
Member

hugovk commented Mar 25, 2020

We could use pytest's parametrize on this test.

Before

    def test_exif(self):
        for test_file in (
            "Tests/images/exif.png",  # With an EXIF chunk
            "Tests/images/exif_imagemagick.png",  # With an ImageMagick zTXt chunk
        ):
            with Image.open(test_file) as im:
                exif = im._getexif()
            assert exif[274] == 1

After

    @pytest.mark.parametrize(
        "test_file",
        [
            "Tests/images/exif.png",  # With an EXIF chunk
            "Tests/images/exif_imagemagick.png",  # With an ImageMagick zTXt chunk
        ],
    )
    def test_exif(self, test_file):
        with Image.open(test_file) as im:
            exif = im._getexif()
        assert exif[274] == 1

One benefit is it (eg. pytest Tests/test_file_png.py::TestFilePng::test_exif) is run as two tests instead of one, and separates each test case so that one doesn't inadvertently depend on the other. And if the first fails, the second will still be run.

@python-pillow python-pillow deleted a comment from codecov bot Mar 26, 2020
@radarhere
Copy link
Member Author

Sounds reasonable. Done.

@hugovk hugovk merged commit 3970db0 into python-pillow:master Mar 26, 2020
@hugovk
Copy link
Member

hugovk commented Mar 26, 2020

Thanks!

This was referenced Mar 17, 2021
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.

PngStream does not decode EXIF data from "Raw profile" tags
2 participants