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

WIP Copy EXIF data to resized images (fix #2204) #2207

Merged
merged 9 commits into from Feb 14, 2016
Merged

WIP Copy EXIF data to resized images (fix #2204) #2207

merged 9 commits into from Feb 14, 2016

Conversation

@ralsina
Copy link
Member

@ralsina ralsina commented Jan 4, 2016

This branch does a naïve copy of EXIF data from the source images to the resized targets. This means it preserves things like EXIF dates, but it also preserves size in pixels, which is now wrong.

This is hidden behind the PRESERVE_EXIF_DATA option, so set that if you want to test it.

@Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented Jan 4, 2016

We should only select date information IMO. Many people wouldn’t want to share all EXIF tags.

@ralsina
Copy link
Member Author

@ralsina ralsina commented Jan 4, 2016

We probably need to offer to scrub EXIF and copy the right bits if the user doesn't request it.

EXIF scrubbing is a good privacy measure.

It's also a lot more work, since we need to make sure ALL images get EXIF scrubbed, including those which are not resized.

@ralsina
Copy link
Member Author

@ralsina ralsina commented Jan 4, 2016

Looks like I could just remove the ImageWidth / ImageLength tags if they are present, and wrap this in a "PRESERVE_EXIF_DATA" option that defaults to False.

Of course I can't pop tags. There is nothing on PIL to support re-encoding the dict into the right format for saving into the image. So that would mean a new dependency.

Or adopting this code: https://github.com/zenwerk/Pyxif/blob/master/pyxif/_load_and_dump.py

ralsina added 4 commits Jan 4, 2016
@humitos
Copy link
Member

@humitos humitos commented Jan 5, 2016

I agree with @Kwpolska about only copy dates, at least for the first version.

In the future version, it would be great to have a config where you can write down all the EXIF fields you want to be shared in your gallery. If the person choose widht and size, we should put the right one in the resized image.

Regarding new dependency. I'm not sure, but I think it easier to maintain a new dependency that a bunch of code from "somewhere". In fact, this dependency could be optional as many other that nikola already has.

@ralsina
Copy link
Member Author

@ralsina ralsina commented Jan 5, 2016

@humitos can't be done. All or nothing. "Some" requires an extra dependency. In any case, for JPG at least, noone puts the size in the EXIF tags.

@ralsina
Copy link
Member Author

@ralsina ralsina commented Jan 27, 2016

This code seems to more or less do what the user would expect. My initial concern about preserving EXIF data with wrong image size seems unfounded. While theoretically possible, no images I have seen actually HAVE that data, and even if they had it, viewers happily ignore it and use the real image sizes from the image format native metadata.

ralsina added 2 commits Jan 29, 2016
@ralsina
Copy link
Member Author

@ralsina ralsina commented Feb 14, 2016

WTH let's merge this, if it breaks anything let me know.

ralsina added a commit that referenced this pull request Feb 14, 2016
WIP Copy EXIF data to resized images (fix #2204)
@ralsina ralsina merged commit e509a52 into master Feb 14, 2016
4 checks passed
4 checks passed
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@ralsina ralsina deleted the preserve-exif branch Feb 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants