[Hardy] digikam duplicates downloaded images while overwriting existing ones

[Olivier]

Binary package hint: digikam

A threading issue leads to loss of data on import. See http://bugs.kde.org/show_bug.cgi?id=158377 for details. A working patch has been comitted to svn and is attached to this bug report.

Please include this critical patch in digikam for Hardy.

[Thiago Jung Bauermann (thiago-bauermann)]

Version: 0.9.3 (using KDE 3.5.8)
Installed from: Debian testing/unstable Packages
OS: Linux

When digikam is not able to download an image from the SD card at first (red X is shown in the image), I selected the failed images and used "download selected" to try to get them again. Digikam then says that it was able to download those, but in the process it also overwrote neighbour images with the downloaded ones!

I will upload some screenshots to illustrate the problem.

I was able to consistently reproduce the problem with the current images in my SD card, but at each time the set of images which digikam can't download is different. When started from the command line, no error is displayed by digikam, just this:

Found dcraw version: 8.81
Exif Orientation: 1
Exif Orientation: 1
Exif Orientation: 1
Exif Orientation: 1
Exif Orientation: 8
Exif orientation tag set to: 1
Exif Orientation: 1
Exif Orientation: 1
Exif Orientation: 1
Exif Orientation: 1
Exif Orientation: 1
Exif Orientation: 1
Exif Orientation: 1

For now, I will keep the SD card as it is to guarantee I can reproduce the problem later if needed. But if I need to use it for some reason, I'll have to erase the card and thus loose my ability to reproduce the problem.

[Thiago Jung Bauermann (thiago-bauermann)]

Created attachment 23704
digikam window showing duplicated images

[Thiago Jung Bauermann (thiago-bauermann)]

Created attachment 23705
download dialog with images for retry selected

[Thiago Jung Bauermann (thiago-bauermann)]

In the attached dialog screenshot, I had to retry downloading the selected images (img_3494.jpg and img_3497.jpg).

In the attached digikam screenshot, you can see that img_3492.jpg was overwritten with the contents of img_3494.jpg, and img_3496.jpg was overwritten with the contents of img_3497.jpg. You can see in the dialog screenshot that those images are originally very different.

Another weird thing is that what should be img_3496.jpg is called in digikam img_3495.jpg. And the real img_3495.jpg is missing. In the case of img_3492.jpg there was no such mess. It is img_3492.jpg itself that is missing.

I hope you realise how serious this bug is.

[Daniel Illi (dilli)]

this is the same bug as 157681
it is a serious problem, I had to downgrade to 0.9.2

[Klaus-weidenbach (klaus-weidenbach)]

I mentioned this problem some time ago in digikam-devel mailinglist. The red cross marks the wrong image as failed. In fact that picture got downloaded successfully, but it is stored with the image name right before it and that one is silenty overwritten, or maybe that failed downloading. So when you redownload the red cross image it looks as everything is all right, but it is not. Didn't had time to investigate more on this, but it is a really nasty bug that really results in image loss.

[David Fraser (davidf)]

I have lost photos on this bug and now it's happened again. I have kept Digikam open with the current situation to try and debug this...

[Arnd-baecker (arnd-baecker)]

Bumping up priority and severity - data loss should not happen
(However I can't help, because I don't see the problem ..)

[Jake Yip (waipengyip)]

It is happening to me too, since I upgraded. To replicate this, get a card of images (not need to be full, but at least >50).

1. Open up the media in Camera - Browse Media - <your media>.
2. In Settings - File rename options - Camera file name - Leave as is.
3. In On the Fly Operation - Auto-rotate / Flip image
4. Select Download All.
5. Wait for the download to finish.
6. Look for the Red Cross / X. for a file (e.g. 317.jpg)
7. Take note of the file name of the previous file. Look for it in the downloaded folder. (e.g. 316.jpg)
8. Open that. You will find that it is actually 317.jpg. 316.jpg is not downloaded at all.

I hope this helps. If you need anything, please let me know!

I am running Fedora 8, Digikam 0.9.3 from package.

[Iroxk-di-0o6vs (iroxk-di-0o6vs)]

I have the same problem here. The messages in the terminal window were (I added some debug messages in cameracontroler.cpp):
digikam: Exif autorotate: pict7096.jpg using (/home/loic/pictures/test/2008-02-25/.digikam-camera-tmp1-22374)
digikam: mimetype = JPEG
Minolta Makernote Orientation: 72
digikam: ExifRotate: no rotation to perform: /home/loic/pictures/test/2008-02-25/.digikam-camera-tmp1-22374
digikam: File downloaded: pict7096.jpg using (/home/loic/pictures/test/2008-02-25/.digikam-camera-tmp1-22374)
digikam: Downloading: pict7097.jpg using (/home/loic/pictures/test/2008-02-25/.digikam-camera-tmp1-22374)
digikam: Exif autorotate: pict7097.jpg using (/home/loic/pictures/test/2008-02-25/.digikam-camera-tmp1-22374)
digikam: ExifRotate: file do not exist: /home/loic/pictures/test/2008-02-25/.digikam-camera-tmp1-22374
digikam: File downloaded: pict7097.jpg using (/home/loic/pictures/test/2008-02-25/.digikam-camera-tmp1-22374)

pict7097.jpg was downloaded and replaced pict7096.jpg. pict7097.jpg had the wrong download icon. What is strange is that ExifRotate does not find the temporary file. I'll try to have a longer loog tomorrow if I have time.

  Loïc

[Caulier-gilles-9 (caulier-gilles-9)]

To be able to reproduce this problem, i need to see a screenshot of all pages from Camera GUI "Settings" right sidebar tab with all download configuration.

Also, let's me hear which camera driver is used in your case. Go to "Help" button and select "Camera Information".

Thanks in advance

Gilles Caulier

[Caulier-gilles-9 (caulier-gilles-9)]

I cannot reproduce this problem with current svn implementation. In all cases, a confirmation dialog is launch to ping users about a possible overwriting in target album.

If in "File Renaming Options", "Camera Filenames" is used to download items, no items are overwrited by differents pictures. The same file name is used in target album than camera file name.

If "Customize" option is used instead, digiKam use KDE rename dialog to ping user about a possible overwritting. digiKam detect properly than a file with the same name already exist in target album (a preview of existing image and camera image is displayed), and propose to rename target file to download...

Gilles Caulier

[Caulier-gilles-9 (caulier-gilles-9)]

Created attachment 23769
file name rename dialog during download from camera to prevent overwritting

[Jake Yip (waipengyip)]

I feel that the problem is less with the overwriting files then of files being silently failing and renamed.

To help with you reproducing the problem

1. In Settings - File rename options - Camera file name - Leave as is.
2. In On the Fly Operation - Auto-rotate / Flip image

As for "Camera Information":

Mounted Camera driver for USB/IEEE1394 mass storage cameras and Flash disk card readers.

Title: Images found in media:/sdb1
Model: directory browse
Port: Fixed
Path: /media/NIKON D50

[Caulier-gilles-9 (caulier-gilles-9)]

Yip,

I use the same config here, using a mounted point on my HDD, not a real UMS camera connected to my computer (it's more easy to test).

Of course, the problem still un-reproductible...

Gilles

[Caulier-gilles-9 (caulier-gilles-9)]

Yip,

When i said "un-reproductible... ", i want mean than pictures are never silently overwritten. I have always a dialog to rename pictures when it's necessary...

Gilles

[David Fraser (davidf)]

I think I have traced what the problem is, by inspecting the source code.
The actual sequence (the important parts):
 A) Download to temporary file (ALWAYS WITH THE SAME NAME, based on the pid)
 B) send event saying download complete
 C) When event is received, rename temporary file to destination name

A) and B) take place in the CameraThread::run method. C) is handled in the CameraController::customEvent method.

Since these happen in separate threads, if the event handler ever gets behind the downloader, what will happen is:
 img1 step A) Download to temporary file
 img1 step B) Send event
 img2 step A) Download to temporary file (OVERWRITING img1 in the temporary file
 img2 step B) Send event
 img1 step C) Rename temporary file to dest/img1, CONTAINING img2!!!
 img2 step C) Try rename temporary file to dest/img2, FAILING, since temporary file is missing

The reason that I am sure that this is the problem, is that we know that a GPItemInfo::DownloadFailed must have been issued for img2, since the icon shows as a cross. Of the places emitting this signal, one is when the CameraEvent::gp_downloadFailed is issued. But this creates a dialog (as Gilles said), and no dialog is displayed. The other is if the file renaming fails (which it will if the temporary file has disappeared). In this case, no dialog is displayed - the signal is simply sent.

Simplest fix would be to generate a unique temporary file name for each file. Patch to follow...

[David Fraser (davidf)]

Created attachment 23770
Patch to include the filename in the source

Added patch that tries to include the filename in the source. Haven't tested
yet :-)

[David Fraser (davidf)]

Tested and it at least works... I would recommend that this be used as a band-aid patch though.
The problem with the current system is that if the final rename fails, the picture has not been copied correctly, but there is no good way to recover this.
It seems that the rename was moved into the event handling section so that there could be GUI interaction to change the name.
Things that would improve this:
 * If the rename fails, a dialog should be created just like if the copy fails.(It is not expected to fail because the target file is checked, but as this error shows it can fail in other ways)
 * If the rename fails, then using "Download/Delete" should NOT delete images that weren't copied successfully... so the deleteAfter in slotDownload should ignore failed images
 * There should be options to "Select Failed" images and to "Delete successfully copied images"
 * There should be at least a sanity check when deleting to ensure that the target deleted image is the same file size etc as the source image, and big error dialogs should pop up if this is not the case :-)

[David Fraser (davidf)]

OK I've tested a successful download of 362 photos and movie files - on the previous run only 323 files copied successfully, replicating the behaviour above.

Spec file, Source RPM and FC8 i386 RPM for Fedora 8 i386 users at:
http://davidf.sjsoft.com/files/digikam.spec
http://davidf.sjsoft.com/files/digikam-0.9.3-2.fc8.src.rpm
http://davidf.sjsoft.com/files/digikam-0.9.3-2.fc8.i386.rpm

[Caulier-gilles-9 (caulier-gilles-9)]

To David, #18,

First, thanks to hack this problem and to provide a patch.

I'm agree with gui interaction when downloading failed.
Are you interested to patch again camera gui in this way ? It's not very difficult to implement and there are few existing similar code in camera gui to get inspiration...

Gilles Caulier

[Arnd-baecker (arnd-baecker)]

*** Bug 157681 has been marked as a duplicate of this bug. ***

[Caulier-gilles-9 (caulier-gilles-9)]

Created attachment 23818
digiKam 0.9.4 try to reproduce the dysfunction into camera gui

David,

Look my fresh screenshot. I have tried to reproduce the problem here using a
local path on my computer to simule a camera folder.

On the left, album icon view with donwloaded image from camera.
On the middle, camera interface, with the current selected items which have
been downloaded.
On the right, my camera interface settings.

This is how i have processed to reproduce the dysfunction :

1/ i start camera interface using location /mnt/data/camera.
2/ all icon item are displayed properlly in camera interface.
3/ I make a selection to prepare downloading.
4/ i open konqueror in /mnt/data/camera and i remove 3 files that i would to
download (currently "car.jpg", "cicada.jpg", and "flower.jpg")
5/ now i return to Camera inteface and i use "Download Selection".
6/ For each items previously deleted, digiKam open a dialog to ask than item
cannot be donwloaded and propose to continue or to cancel operation.
7/ When downloading is complete all item are properlly annoted with the right
icons on the top/right conner. Look like nothing is overwritted in target album
after downloading.

This test have been done without to apply your patch.

Of course, applying your patch is not a problem for me...

Let's me hear if i have forget something here in my test.

Best

Gilles Caulier

[Caulier-gilles-9 (caulier-gilles-9)]

SVN commit 783150 by cgilles:

digiKam from KDE3 branch: Cameragui fixes:
- prepend filename to temp files generated during camera download.
- remove properlly temp files if convert to lossless format is used.
CCBUGS: 158377

 M +7 -2 cameracontroller.cpp
 M +5 -5 cameracontroller.h

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

[David Fraser (davidf)]

Hi Gilles,

Thanks for the detailed testing of this. Sorry I haven't had time to look into this more and only got to look at the bug again today!

Testing this exact problem (before the patch) is difficult as it's a threading problem. Basically you would need to replicate more than one download completing in the background thread before the foreground thread does the rename. You could try and overload the machine during the transfer but that may still not replicate it - I think it's very clear from the code that this can happen, and that my patch fixes the problem, so I'm glad you committed it - thanks!

I'll see when I next get a chance if I can do any more on this.

In the mean time if anyone else can test (especially on Fedora 8 i386 as I've provided RPMs - Yip Wai Peng could you run these?) then that would be great.

[Julien (julien-narboux)]

Hi all,

It seems to me that the patch solved the problem. I can not reproduce it anymore with the svn version.

Best,

Julien

[Caulier-gilles-9 (caulier-gilles-9)]

Thanks for the reports.

I close this file now...

Gilles Caulier

[Achim Bohnet (allee)]

Thx Olivier for the report. Gilles final commit contains two more one line changes.
I've used this to create an debdiff for hardy:

  * Fix: [Hardy] digikam duplicates downloaded images while overwriting
    existing ones. Added: patches/10_lp201053_fix_download_rename_race.diff
    This is a backport of KDE SVN commit 783150. (Closes LP: #201053).
    See also http://bugs.kde.org/show_bug.cgi?id=158377 for more information.

We still need to test it, but I've never seen this race here, so I can't really test.

Achim

[Olivier (olivier-lacroix)]

Ok. I'll apply this patch and tell you if that solves the problem for me

[Olivier (olivier-lacroix)]

Transfering 66 images on build 2 leads to 4 missing pictures.

After applying the patch and importing the same pictures, eveything goes well.

So far so good !

[Thiago Jung Bauermann (thiago-bauermann)]

Cool, thanks for working on this!

[Caulier-gilles-9 (caulier-gilles-9)]

Fixed in svn

Gilles Caulier

[Olivier (olivier-lacroix)]

The bug is now marked as solved in the KDE bug tracking system. This is a CRITICAL bug. Please commit before beta release.

[Achim Bohnet (allee)]

Prepared pkgs for upload are at:

      https://edge.launchpad.net/~allee/+archive/

I ping on #kubuntu-devel about upload.

[Achim Bohnet (allee)]

Too late. I was told that there already in freeze due to the beta release.
After beta it will go in.

[Olivier (olivier-lacroix)]

Ok.

Thanks for your answer. I just felt a bit concerned that a bug leading to data loss would go into the beta. But if it is too late...

[Launchpad Janitor (janitor)]

This bug was fixed in the package digikam - 2:0.9.3-2

---------------
digikam (2:0.9.3-2) unstable; urgency=low

  [ Achim Bohnet]

  * Cherry pick fixes from KDEs SVN:
      40_kdesvn783161_fix_wrong_file-exits_check.diff
      40_kdesvn786622_honour_scrollbar.diff

  * Remove build-deps:
    + automake1.9: We take care that no rebuilding is necessary.
    + libexif-dev: replaced by libexiv2 upstream.

  * Remove Paul Telford <email address hidden> from uploaders. He's no longer
    involved in digikam maintenance. Thx for taking care of digikam as
    well as sponsoring me for a long time.

  * debian/copyright: replace Copyright (C) with ©

  * remove debian/TODO. Had only outdated or will not happen stuff.

  * Merge my changes from kubuntu:

  * Remove relibtoolization patches:
      02_autotools_update.diff
      03_libtool_update.diff
      04_am_maintainer_mode.diff
      05_pedantic-errors.diff
      06_disable_no_undefined.diff
      98_buildprep.diff
    let linker flag --as-needed in debian/rules do the job.

  * Add patch to fix FTBFS with new libkdcraw3 pkgs (0.1.4) required by
    kipi-plugins 0.1.5: 20_fix_ftbfs_with_libkdcraw3.diff .
    Fixes: https://bugs.kde.org/show_bug.cgi?id=146393
    Thx to the Mandriva Maintainer Angelo Naselli to point me to the patch.
    (Closes: #472547 digikam: FTBFS with current libkdcraw)

  * Bump build dependency of libkdcraw to >= 0.1.4 as libkdraw3 patch above
    requires it.

  * Fix: [Hardy] digikam duplicates downloaded images while overwriting
    existing ones. Added: patches/10_lp201053_fix_download_rename_race.diff
    This is a backport of KDE SVN commit 783150. (Closes LP: #201053).
    See also http://bugs.kde.org/show_bug.cgi?id=158377 for more information.

  * add missing final newline to showfoto.install.

  * Edited images saved with a new name wheree only read/writabblbe by it's
    owner. Now the umask is used. Add patches/30_respect_umask.diff. Fixes
    http://bugs.kde.org/show_bug.cgi?id=145252 .

  * Dropped kubuntu change:
    + build-dep bumps of libkexiv2-dev and libexiv2-dev. They were only
      needed for a library transition.

 -- Achim Bohnet <email address hidden> Wed, 26 Mar 2008 14:42:43 +0000

[Thiago Jung Bauermann (thiago-bauermann)]

Hi,

I just verified that version 0.9.4-beta2 doesn't have the problem anymore, the bug is fixed.

I'm sorry to take so long to test this. Having to download all build dependencies to compile digikam (especially in this case since I needed to get two libraries directly from subversion repo) is cumbersome.

By the way, maybe I'm being picky but: with the current fix, the problem can still occur if you have two files of the same name in different directories, right? Or is this too infrequent to be relevant?

[David Fraser (davidf)]

Re Comment #29:
> By the way, maybe I'm being picky but: with the current fix, the problem can still occur if you have two files of the same name in different directories, right? Or is this too infrequent to be relevant?

The current code creates the temporary file in the target directory, not in a temporary directory, and then renames it. So same filename in different directories should be fine.

[Thiago Jung Bauermann (thiago-bauermann)]

Great! Thanks for the explanation (and for the fix, of course!). I am
completely happy then. :-)

2008/4/22 David Fraser <email address hidden>:

[bugs.kde.org quoted mail]

Great! Thanks for the explanation (and for the fix, of course!). I am completely happy then. :-)<br><br><div class="gmail_quote">2008/4/22 David Fraser &lt;<a href="mailto:<email address hidden>"><email address hidden></a>&gt;:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="Ih2E3d">------- You are receiving this mail because: -------<br>
You reported the bug, or are watching the reporter.<br>
<br>
<a href="http://bugs.kde.org/show_bug.cgi?id=158377" target="_blank">http://bugs.kde.org/show_bug.cgi?id=158377</a><br>
<br>
<br>
<br>
<br>
</div>------- Additional Comments From davidf sjsoft com &nbsp;2008-04-22 16:15 -------<br>
Re Comment #29:<br>
<div class="Ih2E3d">&gt; By the way, maybe I&#39;m being picky but: with the current fix, the problem can still occur if you have two files of the same name in different directories, right? Or is this too infrequent to be relevant?<br>

<br>
</div>The current code creates the temporary file in the target directory, not in a temporary directory, and then renames it. So same filename in different directories should be fine.<br>
</blockquote></div><br><br clear="all"><br>-- <br>[]&#39;s<br>Thiago Jung Bauermann

[Thiago Jung Bauermann (thiago-bauermann)]

oh, man. what a mess. sorry about the html in the post above.

[David Fraser (davidf)]

For reference, this has been fixed on Fedora by backporting the patch to 0.9.3 - see https://bugzilla.redhat.com/show_bug.cgi?id=448235 (of course, the recently released 0.9.4 will replace that)