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

Add captions and keywords as a photo field #915

Closed

Conversation

adambenali
Copy link

iOS 14 introduced captions to Photos. This PR adds the caption field to the list of requested fields.

iOS 14 introduced captions to Photos. This PR adds the caption field to the list of requested fields.
@adambenali adambenali changed the title Add captions as a photo field Add captions and keywords as a photo field Jul 19, 2024
@AndreyNikiforov
Copy link
Collaborator

iOS 14 introduced captions to Photos. This PR adds the caption field to the list of requested fields.

What problem does your change solve?

@adambenali
Copy link
Author

iOS 14 introduced captions to Photos. This PR adds the caption field to the list of requested fields.

What problem does your change solve?

@AndreyNikiforov This change makes it possible to read the keywords / caption attached to a picture from the photo._asset_record field.

One example use-case is that I can decide to embed this information into the EXIF of the image after I download it.

@AndreyNikiforov
Copy link
Collaborator

@AndreyNikiforov This change makes it possible to read the keywords / caption attached to a picture from the photo._asset_record field.

One example use-case is that I can decide to embed this information into the EXIF of the image after I download it.

Ok, so you would like user to be able to enter captions in iCloud.com and would like downloaded images to have these captions in EXIF. I assume that user can edit captions in iCloud.com, there is a standard field in EXIF for captions, and Apple does not add these captions into EXIF of the asset automatically (did you check all of these conditions?).

Then your PR should have functionality to request extra field from iCloud service and update EXIF in images with it. Functionality should work only when user requested it (==flag parameter). There should be tests confirming that you new behavior works, see tests for EXIF dates.

@adambenali
Copy link
Author

adambenali commented Jul 28, 2024

Then your PR should have functionality to request extra field from iCloud service

This is done.

and update EXIF in images with it

This is out of the scope of this PR. It's up to the app using icloudpd to make use of this information. The point of this PR is to have this caption information be accessible as part of the PhotoAsset object.

All in all, this PR is only updating the long list of requested fields, nothing else. I don't think it requires that many extra steps as you're suggesting.

@AndreyNikiforov
Copy link
Collaborator

and update EXIF in images with it

This is out of the scope of this PR. It's up to the app using icloudpd to make use of this information. The point of this PR is to have this caption information be accessible as part of the PhotoAsset object.

icloudpd is not a library that some other app is using, instead it is an application that provides functionality to end users. You need to provide all other functionality that you envisioned. I suggest posting an issue with what you are trying to achieve, so your idea can be discussed in detail before spend time coding.

@adambenali
Copy link
Author

and update EXIF in images with it

This is out of the scope of this PR. It's up to the app using icloudpd to make use of this information. The point of this PR is to have this caption information be accessible as part of the PhotoAsset object.

icloudpd is not a library that some other app is using, instead it is an application that provides functionality to end users. You need to provide all other functionality that you envisioned. I suggest posting an issue with what you are trying to achieve, so your idea can be discussed in detail before spend time coding.

Question: How is captionEnc (which is a requested field that's already there in the list) currently being used?

Now the exact same thing applies to extendedDescEnc and keywordsEnc.

@AndreyNikiforov
Copy link
Collaborator

Question: How is captionEnc (which is a requested field that's already there in the list) currently being used?

I don't know. It is in pyicloud code that was grandfathered

Now the exact same thing applies to extendedDescEnc and keywordsEnc.

if there is existing code that you do not know the use of, it is not a reason on itself to add more code that is not used. Provide code to icloudpd that will use these new fields and improve the app in a meaningful way

Copy link
Collaborator

@AndreyNikiforov AndreyNikiforov left a comment

Choose a reason for hiding this comment

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

Provide code to icloudpd that will use these new fields and improve the app in a way meaningful to the users.

@adambenali adambenali closed this Nov 27, 2024
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.

2 participants