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

feat(server): xmp sidecar metadata #2466

Merged
merged 37 commits into from May 25, 2023

Conversation

alex-phillips
Copy link
Contributor

@alex-phillips alex-phillips commented May 17, 2023

This PR adds support for ingesting metadata from XMP sidecar files.

  • If an XMP sidecar exists with a media file, it is assumed that the XMP file takes priority and all metadata is read from the file.
  • XMP sidecars can now be uploaded with the initial media files via the API.
  • "All" metadata scans check for the presence of new sidecar files if the media file does not have a sidecar file associated with it. This allows the creation of metadata files from third party software such as Digikam or Adobe Lightroom.
  • "Missing" metadata scans include media assets that do not have a sidecar file associated with them. It attempts to find the sidecar in the filesystem and associates that sidecar file with the asset in the future.
  • The XMP sidecar path is stored along with the media asset with a reference to its filepath.

image
image
image

@vercel
Copy link

vercel bot commented May 17, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
immich ⬜️ Ignored (Inspect) Visit Preview May 24, 2023 9:32pm

Copy link
Contributor

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

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

Looking good! Have you run any of the linting checks? You should look at server/package.json or the contributing docs and run all the check commands to lint and format, and test the code (both web and UI).

server/libs/domain/src/job/job.repository.ts Outdated Show resolved Hide resolved
Comment on lines +344 to +349

@Processor(QueueName.SIDECAR)
export class SidecarProcessor {
private logger = new Logger(SidecarProcessor.name);
private assetCore: AssetCore;

Copy link
Contributor

Choose a reason for hiding this comment

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

The code in domain/ is basically refactored code, this code here hasn't been updated and doesn't have any tests for it yet. Eventually the thought is to move all the logic out of this file into a domain/media/media.service.ts file. If you're up for that, it would be the place to put it. Otherwise, I can move it after it's merged (refactored processors are in processors.ts and a new service would need to be exported in domain.module.ts). If not, no worries.

@jrasm91
Copy link
Contributor

jrasm91 commented May 20, 2023

Can you attach a few screenshots? One of the files in library/ (with xmp) and one of the new job? Thanks.

@alex-phillips
Copy link
Contributor Author

Screenshot of sidecars in place after library upload:
image

Screenshot of the new jobs section:
image

image

@vercel
Copy link

vercel bot commented May 21, 2023

@alex-phillips is attempting to deploy a commit to the immich Team on Vercel.

A member of the Team first needs to authorize it.

@alextran1502
Copy link
Contributor

alextran1502 commented May 21, 2023

Can you help update the documentation on how we can use this feature?

@alex-phillips
Copy link
Contributor Author

Can you help update the documentation on how we can use this feature?

Added documentation and included some of the images in it as well. Let me know if I should elaborate on anything or include more details.

@alextran1502
Copy link
Contributor

@alex-phillips Thank you. There are some typo in the documentation, you can copy and paste the paragraph below to fix

There are 2 administrator jobs associated with sidecar files: `SYNC` and `DISCOVER`. The sync job will re-scan all media with existing sidecar files and queue them for a metadata refresh. This is a great use case when third-party applications are used to modify the metadata of media. The discover job will attempt to scan the filesystem for new sidecar files for all media that does not currently have a sidecar file associated with it.

Can you also help resolve the conflicts?

Thank you

@alextran1502
Copy link
Contributor

Is there something wrong with the merge conflict? Looks like there are problems after the conflicts are resolved.

Copy link
Contributor

@alextran1502 alextran1502 left a comment

Choose a reason for hiding this comment

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

Thank you for the high-quality PR! And sorry for the constant new changes to the main branch :P

Copy link
Contributor

@alextran1502 alextran1502 left a comment

Choose a reason for hiding this comment

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

Thank you for the high-quality PR! And sorry for the constant new changes to the main branch :P

@alextran1502 alextran1502 merged commit 7c1dae9 into immich-app:main May 25, 2023
18 checks passed
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.

None yet

3 participants