Bug 346405 - Problem importing pick labels [patch]
Summary: Problem importing pick labels [patch]
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: Metadata-Sidecar (show other bugs)
Version: 4.4.0
Platform: Debian stable Linux
: NOR normal
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-20 17:36 UTC by mau
Modified: 2015-05-09 12:36 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.10.0
Sentry Crash Report:


Attachments
Photo as a test case (245.30 KB, image/jpeg)
2015-04-20 17:40 UTC, mau
Details
Sidecar file for the test case image (3.39 KB, application/xml)
2015-04-20 17:40 UTC, mau
Details
removeImageTags.patch (491 bytes, patch)
2015-04-21 20:05 UTC, Maik Qualmann
Details
removeImageTags2.patch (1.62 KB, patch)
2015-04-22 18:35 UTC, Maik Qualmann
Details
removeImageTags3.patch (2.23 KB, patch)
2015-04-23 20:31 UTC, Maik Qualmann
Details
removeImageTags4.patch (2.17 KB, patch)
2015-05-05 18:40 UTC, Maik Qualmann
Details
signature.asc (818 bytes, application/pgp-signature)
2015-05-09 12:36 UTC, mau
Details

Note You need to log in before you can comment on or make changes to this bug.
Description mau 2015-04-20 17:36:59 UTC
I use two computers with digikam installed, one of them as my main machine with all my photos, and a laptop for on the way. Both of them are configured to read and write metadata only from/to xmp sidecar files.

After editing metadata of photos in one folder on the laptop I copy the sidecar files into the same folder on my main machine (images are already there), replacing the sidecar files there. Then I launch digikam and let it Re-Read metadata from files (or maintenance -> sync metadata...). Until a few weeks or months ago this worked as expected, but now ratings are imported but pick labels are not.

Both digikam installations are configured identically and correctly; Gilles could already confirm the bug.

To test the issue one can also edit the pick label entry in the xmp file with a text editor and force digikam to re-read metadata, it doesn't work.

Versions:
digikam 4.4.0
libkexiv2: 2.3.2
Exiv2: 0.24
System is Debian Jessie (becoming stable in a few weeks!)

Reproducible: Always

Steps to Reproduce:
1. Edit pick label in xmp sidecar file
2.  Re-Read metadata


Actual Results:  
Modified pick label does not show up correctly in digikam.

Expected Results:  
Modified pick label should show up.
Comment 1 mau 2015-04-20 17:40:08 UTC
Created attachment 92132 [details]
Photo as a test case
Comment 2 mau 2015-04-20 17:40:46 UTC
Created attachment 92133 [details]
Sidecar file for the test case image
Comment 3 Maik Qualmann 2015-04-21 20:05:26 UTC
Created attachment 92148 [details]
removeImageTags.patch

The problem was that the old tags not removed from the DB. I think the patch is fine, what do you think Gilles?

Maik
Comment 4 caulier.gilles 2015-04-22 04:21:41 UTC
Maik, 

Good question... 

In fact we must be sure if it will work in all cases.

Marcel, 

The patch is very simple. It's just a missing line in source code here ?

Gilles
Comment 5 Maik Qualmann 2015-04-22 05:56:17 UTC
I've already thought to only remove all kinds of ColorLabel (also incorrect) and PickLabel tags.

Maik
Comment 6 Maik Qualmann 2015-04-22 06:03:23 UTC
Maybe we need to first see if any ColorLabels and PickLabels are included in the new set of tags.

Maik
Comment 7 mau 2015-04-22 07:54:03 UTC
Thanks for looking into this issue so fast!!!

The question is if any kind of conflict management is necessary - I don't think so because Re-Read metadata from Image means (at least for me) basically replace old (db) data by the new one.

Example:
Let rating = 1 in db and no pick label and one tag.
Let new xmp contain no rating, pick label = 1 and no tag.
I'd expect to "loose" the rating (so new rating = 0) and the tag.

Other opinions? Is there a difference between "no rating" and "rating = 0"?
Comment 8 caulier.gilles 2015-04-22 08:14:14 UTC
no rating == rating 0 in DB
Comment 9 Maik Qualmann 2015-04-22 18:35:37 UTC
Created attachment 92166 [details]
removeImageTags2.patch

The rating of an image is stored in a different area of the DB. Here are the "special" tags affected such as keywords, color and pick-label.

My solution with this patch::
If a valid color or pick-label present in XMP, must be removed the old tags from the DB. If no valid color or pick-label available, we leave the old tags in the DB. The behavior of the keywords is not changed.

Maik
Comment 10 caulier.gilles 2015-04-22 19:32:22 UTC
Maik,

I'm not sure if this kind of rules must go to ImageScanner class.

Look MetadataHub class. perhaps this code already exist, in another form, and perhaps it do not work due to an existing bug...

Gilles
Comment 11 Maik Qualmann 2015-04-23 20:31:42 UTC
Created attachment 92189 [details]
removeImageTags3.patch

Optimized patch again, am sure the patch at this place is correct. I have checked it with an DB editor before and after.

Maik
Comment 12 Maik Qualmann 2015-05-05 18:40:39 UTC
Created attachment 92439 [details]
removeImageTags4.patch

Only code optimization.

Maik
Comment 13 caulier.gilles 2015-05-05 21:13:59 UTC
Maik,

As Marcel said in private mail (CC with you), patch is fine to commit.

Gilles
Comment 14 Maik Qualmann 2015-05-06 16:58:46 UTC
Git commit 6f48b8643b4263e3a73a2b7cc97fbaa73d20ca3e by Maik Qualmann.
Committed on 06/05/2015 at 16:46.
Pushed by mqualmann into branch 'master'.

apply patch #92439 to remove old pick and color tags from the DB if new tags added
FIXED-IN: 4.10.0

M  +2    -1    NEWS
M  +26   -1    libs/database/imagescanner.cpp

http://commits.kde.org/digikam/6f48b8643b4263e3a73a2b7cc97fbaa73d20ca3e
Comment 15 caulier.gilles 2015-05-06 19:19:57 UTC
Git commit 6a7b6cd9f661057903f2b9aaf0fb10d552bf67ab by Gilles Caulier.
Committed on 06/05/2015 at 19:19.
Pushed by cgilles into branch 'frameworks'.

backport commit #6f48b8643b4263e3a73a2b7cc97fbaa73d20ca3e from git/master to frameworks branch

M  +26   -1    libs/database/item/imagescanner.cpp

http://commits.kde.org/digikam/6a7b6cd9f661057903f2b9aaf0fb10d552bf67ab
Comment 16 mau 2015-05-09 12:36:14 UTC
Created attachment 92508 [details]
signature.asc

Thank you so much for the fix; would it be difficult to get it into Debian 
Jessie (stable)? What would be the way to get it into it, should I report a 
bug against digikam in the Debian bug tracker referring to the KDE bug 
tracker's bug number?