| Summary: | Comments lost when converting from png to jpeg | ||
|---|---|---|---|
| Product: | [Applications] digikam | Reporter: | Paweł Marciniak <pave> |
| Component: | Plugin-Bqm-Convert | Assignee: | Digikam Developers <digikam-bugs-null> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | caulier.gilles, marcel.wiesweg |
| Priority: | NOR | ||
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | unspecified | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | 1.5.0 | |
| Sentry Crash Report: | |||
|
Description
Paweł Marciniak
2007-11-12 18:03:58 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. 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. 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. Marcel, Are you seen the race condition found by Pawel ? Gilles Caulier 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. Marcel, There is no signal like this in likipi, but it's trivial to add somthing like that (KDE4 only) Gilles Caulier 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? 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
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) (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. :) 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 |