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

Support searching for Subject and Keywords EXIF fields #1958

Closed
wants to merge 4 commits into from
Closed

Support searching for Subject and Keywords EXIF fields #1958

wants to merge 4 commits into from

Conversation

cycneuramus
Copy link
Contributor

@cycneuramus cycneuramus commented Mar 6, 2023

As per this discussion, I have added the ability to search for a couple of additional EXIF fields.

This will prove useful for those people who maintain their own photo libraries and have spent considerable time adding tags, faces, etc. to the EXIF metadata. See, e.g., this comment.

@vercel
Copy link

vercel bot commented Mar 6, 2023

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

1 Ignored Deployment
Name Status Preview Comments Updated
immich ⬜️ Ignored (Inspect) Mar 7, 2023 at 3:57PM (UTC)

@michelheusschen
Copy link
Contributor

Existing migration files shouldn't be edited. Instead, generate a new migration file using the database migrations guide. If needed, you can edit this new migration file manually.

@cycneuramus
Copy link
Contributor Author

Existing migration files shouldn't be edited. Instead, generate a new migration file using the database migrations guide. If needed, you can edit this new migration file manually.

Whoops! It seems I got a bit carried away with the ripgrepping. Should be fixed now.

Författare: cycneuramus <56681631+cycneuramus@users.noreply.github.com>
Författare: cycneuramus <56681631+cycneuramus@users.noreply.github.com>
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.

Can you post some sample values for these two fields? Also, you probably will want to add those to asset.schema.ts.

@cycneuramus
Copy link
Contributor Author

Added.

Sample values:

$ exiftool example1.jpg | grep Subject
Subject: John Doe, Jane Doe

$ exiftool example2.jpg | grep Keywords
Keywords: Monument, Notre-Dame, apple, anything

@jrasm91
Copy link
Contributor

jrasm91 commented Mar 7, 2023

It looks like the fields are comma separated items. Do we want to store these as plain strings or should we transform them to string arrays? If we are only interested in them being searchable then it probably does not matter very much, but if they will be used for anything else they should probably be arrays.

@cycneuramus
Copy link
Contributor Author

They are stored as plain strings right now since I was really only interested in making them searchable. I also did not include them in the photo details sheet—or anywhere in the UI, really—because I thought that might be too much of an intrusive change in a direction I'm not sure this project is going. Hence I was mostly going for a small QoL improvement for the photo library power users, so to speak.

You're probably right, though, that making them arrays would be a bit more future-proof. I'd welcome some more thoughts on this.

@jrasm91
Copy link
Contributor

jrasm91 commented Mar 7, 2023

Yeah, not 100% what approach we want to take here. They might be nice to add and add as string arrays, eventually could make their way into the UI, not sure if that would be a welcomed change or not. Alternatively we could just have a "catch all" text field that we just dump (concatenate) all unused exif tags into just to be able to search them. IMO some basic support for common, well-known tags that other photo management solutions use is not a bad idea to add some limited support for here. My personal vote would be to make them string arrays and extract them as such.

As far as making them searchable, we're migrating search operations to typesense, which is a dedicated search component, and we will probably only use the postgres-based search in the future for low-end devices. You've successfully added the fields to the postgres search, but to actually search on them in the typesense implementation you'll need to add them here:

https://github.com/immich-app/immich/blob/main/server/libs/infra/src/search/typesense.repository.ts#L268-L274

@jrasm91
Copy link
Contributor

jrasm91 commented Mar 7, 2023

What if we made some of this "user specified"? We're considering adding an administration screen for "custom tags" which could let you list all the extra tags you wanted to be able to search on.

@cycneuramus
Copy link
Contributor Author

Sounds good to me. Ingesting the additional EXIF fields and letting the user specify which ones to use, though perhaps a bit convoluted from an UX perspective, would also save us the trouble of manually implementing each desired field like I have done here.

@jrasm91
Copy link
Contributor

jrasm91 commented Mar 7, 2023

This is what we have come up with: #1965

@cycneuramus
Copy link
Contributor Author

Looks good! Closing this PR in favor of #1965.

@cycneuramus cycneuramus closed this Mar 7, 2023
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.

3 participants