Conversation
Two XMP sidecars that the reader gets wrong: - a digiKam:TagsList with more than one rdf:li: none of the tags is read (a single one is); - all properties in one rdf:Description element: nothing but the GPS coordinates is read (description, date, rating and tags are dropped).
…decars
The reader walks the parsed document and matches property paths such as
"TagsList/Seq/li". walk appends an index to the elements of a list
("Description[2]", "li[1]"), and filter stripped only the index of the
Description element, so
- a tag list with several rdf:li items matched nothing: every item's
path ended in "li[N]";
- a sidecar with a single rdf:Description matched nothing but GPS: its
properties' paths kept the "/xmpmeta/RDF/Description/" prefix that
filter only stripped when indexed, while the GPS cases were written
for that unindexed form and missed the indexed one.
Strip every list index and the Description prefix before matching, and
match GPS on the normalised path like the other properties.
walk also reassigned its path variable when it met a list, so the keys
following the list in the same element were looked up under a wrong
prefix; use a local variable.
Before the fix, GPS coordinates were matched on the unindexed path only, so they were read solely from sidecars with a single rdf:Description. The multi-Description layout now works too, but no fixture exercised it; add GPS to the multi-tags fixture to cover it.
Stripping the list indices made every rdf:li of an rdf:Alt match, so a description repeated in several languages ended up as whichever entry came last. The XMP specification puts the x-default entry first in an Alt array, so keep the first match instead.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fix XMP sidecar layouts that immich-go reads wrongly:
digiKam:TagsListwith more than one item applied no tags at all (a single item worked);rdf:Altwith more than onerdf:li) was dropped;rdf:Descriptionelement yielded nothing but the GPS coordinates (description, date, rating and tags dropped).Fixes #1430
Cause
walkappends an index to the elements of a list (Description[2],li[1]) andfilterstripped only the index of theDescriptionelement, so a multi-item list's paths gained ali[N]segment and a single-Description sidecar's paths kept the/xmpmeta/RDF/Description/prefix; neither matched the property namesfilterlooks for. The GPS cases were written for the unindexed prefix, which is why only they worked in that layout (and they missed the indexed one).walkalso reassigned itspathvariable on meeting a list, so the keys after the list in the same element got a wrong prefix.Fix
filterstrips every[N]index and theDescriptionprefix before matching, and GPS is matched on the normalised path like the other properties. A multi-language description takes its first entry, which the XMP specification requires to be the x-default one.walkkeeps the list path in a local variable.Three sidecars are added as test data, one per layout, and the multi-Description one also covers GPS spread over several
rdf:Descriptionelements; the tests fail before the fix (expected 3 tags, got 0; the single-Description one loses description, date, rating and tags) and pass after.Notes
Based on
mainrather thandevelop, because the fix was tested against Immich v3, whose support is onmainonly.