-
-
Notifications
You must be signed in to change notification settings - Fork 167
Fix/xmp image tags overwrite fix #1235
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
base: develop
Are you sure you want to change the base?
Fix/xmp image tags overwrite fix #1235
Conversation
Adds a flag to defer applying tags until after metadata extraction is complete. This improves performance by preventing unnecessary tag processing before the metadata has been extracted. It also introduces a mechanism to wait for the metadata extraction job to complete. Increases tag cache size when deferring tags to prevent eviction.
This ensures metadata extraction finishes as fast as possible without competing with other heavy jobs. This only applies when the jobs were paused to begin with.
Introduces the `--defer-tags` option to postpone tag application until after metadata extraction. This prevents overwriting embedded file tags/keywords during the upload process, addressing the issue of XMP image tags being overwritten.
Changes the default value of the `defer-tags` flag to true. This ensures that tags are applied after metadata extraction, preserving embedded file tags and keywords. Updates documentation and configuration examples to reflect the new default value.
Changes the default value of the `defer-tags` flag to `false`. Too many potential issues with large libraries This ensures that tags are applied immediately after upload, unless explicitly disabled by the user. This is necessary to ensure XMP image tags overwrite existing tags instead of being overwritten by them.
|
/run-e2e |
|
Will review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have seen this problem before
- Immich-go uploads the asset
- immich accepts the asset and add task to its runner to read exif and XMP
- immich-go sets tags and adds to albums
- defered task at step 2 overrides the changes done by step 3
This was the motivation for batch tagging and stopping immich's background jobs while uploading.
I had the impression this have fixed the problem.
I don't see why re-starting the metadataExtraction job before other paused jobs could fix the issue
What has changed since then to explain why this problem is back?
Can you add an end to end tests in the folder internal/e2e/client to validate the fix?
| uc.app.Log().Info("Resuming metadata extraction...") | ||
| // Use a background context to ensure the command is sent even if the main context is cancelling | ||
| bgCtx := context.Background() | ||
| _, err := uc.client.AdminImmich.SendJobCommand(bgCtx, "metadataExtraction", "resume", true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The linter doesn't like it
Error: app/upload/run.go:116:50: Non-inherited new context, use function like context.WithXXX instead (contextcheck)
_, err := uc.client.AdminImmich.SendJobCommand(bgCtx, "metadataExtraction", "resume", true)
anyway, I don't know how to do something else
| _, err := uc.client.AdminImmich.SendJobCommand(bgCtx, "metadataExtraction", "resume", true) | |
| _, err := uc.client.AdminImmich.SendJobCommand(bgCtx, "metadataExtraction", "resume", true) //nolint:contextcheck |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this is different than the original problem you are describing.
I can confirm task 2 does not override the values of task 3. I believe task 2 will simply not add tags read from the metadata if tags have already been applied to the asset.
Running the metadata task first was my way of trying to speed up the process so the tags get applied in immich and immich-go can than apply its tags as soon as possible so the user can than exit the program.
I have now tested this solution a couple times with my large library and it does work but in the event that the import gets stuck or immich tasks deadlock you end up having to reimport the whole library so writing the tags to a file first so it can always pick up would make this implementation more reliable..
That all being said I think I may investigate the immich Code base first to see if the process of applying tags could be updated there as this is turning into a large work around for what may just be a simple PR on that end.
I have also never written in Go until now so this is not my area of expertise and I don't like committing spaghetti or AI slop so I will get back to this in a bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't forgot about this but also haven't looked into it yet. If I forget about this entirely feel free to close but its the holidays so just a little busy
Defers applying tags until after the Immich metadata extraction job is complete. Immich will not apply tags found in an assets metadata if a tag was applied externally first.
Please see #1233