Bug 152208 - Comments lost when converting from png to jpeg
Summary: Comments lost when converting from png to jpeg
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: Plugin-Bqm-Convert (show other bugs)
Version: unspecified
Platform: unspecified Linux
: NOR normal
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-11-12 18:03 UTC by Paweł Marciniak
Modified: 2017-07-01 15:24 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 1.5.0


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Paweł Marciniak 2007-11-12 18:03:58 UTC
Version:           svn (using KDE KDE 3.5.8)
Installed from:    Unlisted Binary Package
OS:                Linux

Steps to reproduce:
1. Copy one or more png files (with comments stored only in digiKam database [as Exiv2 does not support adding them to png files]) to a new album. The comments are still shown with the new images.
2. Convert one or more images with kipi batch convert to jpeg(s).
3. Comments are lost on the new jpegs.
4. Update the comment on any of the png files (adding any letter and then hitting backspace suffices, it is only to enable "apply" when you hit pgDown/pgUp).
5. Convert photos to jpegs again (not necessarily the updated one!) - comments are preserved.

Once I encountered the same issue with jpg->jpg recompression, but wasn't able to reproduce it.

I was not sure if this is not a bug in kipi plugins, but it looks like it's digiKam who confuses the plugin.
Comment 1 Paweł Marciniak 2007-11-13 14:31:27 UTC
Actually, the behavior is completely random and the above rule doesn't hold all the time.

I'm pretty sure it's some kind of a race condition in the digiKam database.
What I've found so far is that the reason for not storing comments in some of the newly generated jpegs is that the image is not yet in the database when digiKam tries to add a caption to it.

Details:
The AlbumDB::getImageId(int albumID, const QString& name) method returns -1 for the unlucky images. The albumID and name parameters are correct, so the only reason is that the required row is not yet in the database. When I rerun the query with sqlite3 command once the batch process is finished, the images in question are already in the database, but without any caption.
Comment 2 Paweł Marciniak 2007-11-13 17:11:42 UTC
Another observation is that the m_interface->refreshImages( urlList ) call in void BatchProcessImagesDialog::slotProcessDone(KProcess* proc) sometimes inserts the new image to the database, sometimes not. I'm unable to find the reason, so I guess my contribution to this bug ends here.
Comment 3 Paweł Marciniak 2007-11-18 13:20:50 UTC
OK, I spotted the beast. The problem is a race condition between cloning metadata and inserting the new image to the database.

The AlbumManager::slotDirty() method, fired by KDirWatcher, MAY or MAY NOT trigger and insert new picture to the database before the metadata is being cloned. This depends on the time it takes for the ImageMagick's convert to finish. Apparently my computer is too fast ;) so I wrapped convert with a script that adds "sleep 5" at the end, and now everything works correctly.

Not attempting to suggest a patch, as this probably requires deep knowledge of DigiKam architecture and perhaps some changes in the design.
Comment 4 caulier.gilles 2008-12-07 21:09:22 UTC
Marcel,

Are you seen the race condition found by Pawel ?

Gilles Caulier
Comment 5 Marcel Wiesweg 2008-12-07 21:24:08 UTC
Is there a way for a KIPI plugin to signal that metadata was changed? The metadata edit plugin will signal this, wont it?
I ask because the clean way would be to assume that the image is already added and signal metadata rescan.
Comment 6 caulier.gilles 2008-12-07 21:26:42 UTC
Marcel,

There is no signal like this in likipi, but it's trivial to add somthing like that (KDE4 only)

Gilles Caulier
Comment 7 Marcel Wiesweg 2008-12-13 22:17:19 UTC
We have refreshImages in kipiinterface().
We can always have the race condition if the new file is created, detected by the dir watch, scanned, and then the metadata is added. For 0.10 this can happen as well. The plugin shall call refreshImages() afterwards, but this will currently not result in a full metadata rescan, only in a partial rescan (because e.g. for the jpeglossless plugin, a full rescan would be inappropriate).

It's different if the plugin would call the addImage method. I can make it such that calling this method will always result in a full rescan. It seems that batchprocess is not available for KDE4? A plugin where a 100% metadata change can occur after file creation shall then call addImage when it has ensured that the file in finalized.

I would like to think about this solution. Any implications?
Comment 8 caulier.gilles 2008-12-15 12:12:31 UTC
Marcel,

>It seems that batchprocess is not available for KDE4?

yes, and i don't plan to port it. Instead Batch Queue Manager is in the way.

Using addImage() when a plugin rewrite an already existed file from collection can be fine for me (never tested). also, refreshImages() must be only used to refresh host thumbnails view.

also, unforget that we will add a new signal in libkipi to bring host about image metadata changes. This cannot be another alternative ?

Gilles

Comment 9 Marcel Wiesweg 2008-12-15 18:35:18 UTC
Yes we should add a method that signals metadata changes, changes like those done by the metadata edit plugin that require a full metadata rescan.
We should also think about adding a method signalling that the image data of a file changed.
With the batch plugin not being ported, there is also a chance of fixing the original bug here making sure that the race condition does not occur at all (like we do it in camera interface, using temp files. Perhaps with a defined extension like .kipi.tempfile or whatever that can be ignored by collection scanners)
Comment 10 Paweł Marciniak 2009-01-21 08:27:20 UTC
(In reply to comment #7)
Guys,

I'm not sure if I understand your discussion correctly, but it seems that what you are talking about doesn't completely solve the original problem I had.

Even when you force metadata rescan from the new file, the database entry for the new image will only have the information that is embedded in the file. What about information that is only stored in digikam database (such as comments for png files)?

For this to work, I think you would have to make sure the new file is picked up by dir watcher and added to digikam database prior to cloning metadata in the kipi plugin.

Sorry if my comment doesn't make sense, I'm not familiar with how exactly things work in there. :)
Comment 11 caulier.gilles 2010-10-05 15:43:32 UTC
SVN commit 1182729 by cgilles:

Use right order to clone data from one item to another one.
BUGS: 238823
BUGS: 152208
BUGS: 211558
BUGS: 199318


 M  +1 -1      batchprocessimages/batchprocessimagesdialog.cpp  
 M  +1 -1      dngconverter/plugin/batchdialog.cpp  
 M  +1 -1      rawconverter/batchdialog.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1182729