Bug 158377 - digikam duplicates downloaded images while overwriting existing ones
Summary: digikam duplicates downloaded images while overwriting existing ones
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: Import-MainView (show other bugs)
Version: 0.9.3
Platform: Debian testing Linux
: VHI critical
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-02-25 02:59 UTC by Thiago Jung Bauermann
Modified: 2012-08-22 05:21 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 0.9.4


Attachments
digikam window showing duplicated images (131.59 KB, image/png)
2008-02-25 03:22 UTC, Thiago Jung Bauermann
Details
download dialog with images for retry selected (242.84 KB, image/png)
2008-02-25 03:24 UTC, Thiago Jung Bauermann
Details
file name rename dialog during download from camera to prevent overwritting (533.47 KB, image/png)
2008-03-03 09:45 UTC, caulier.gilles
Details
Patch to include the filename in the source (605 bytes, patch)
2008-03-03 16:22 UTC, David Fraser
Details
digiKam 0.9.4 try to reproduce the dysfunction into camera gui (665.95 KB, image/png)
2008-03-07 09:16 UTC, caulier.gilles
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Thiago Jung Bauermann 2008-02-25 02:59:22 UTC
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.
Comment 1 Thiago Jung Bauermann 2008-02-25 03:22:52 UTC
Created attachment 23704 [details]
digikam window showing duplicated images
Comment 2 Thiago Jung Bauermann 2008-02-25 03:24:25 UTC
Created attachment 23705 [details]
download dialog with images for retry selected
Comment 3 Thiago Jung Bauermann 2008-02-25 03:29:53 UTC
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.
Comment 4 Daniel Zuberbühler 2008-02-25 11:23:20 UTC
this is the same bug as 157681
it is a serious problem, I had to downgrade to 0.9.2
Comment 5 Klaus Weidenbach 2008-02-25 16:07:40 UTC
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.
Comment 6 David Fraser 2008-03-01 05:46:24 UTC
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...
Comment 7 Arnd Baecker 2008-03-01 07:36:14 UTC
Bumping up priority and severity - data loss should not happen
(However I can't help, because I don't see the problem ..)
Comment 8 Yip Wai Peng 2008-03-01 16:40:35 UTC
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.
Comment 9 Loïc Brarda 2008-03-02 23:15:27 UTC
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
Comment 10 caulier.gilles 2008-03-03 08:09:00 UTC
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
Comment 11 caulier.gilles 2008-03-03 09:41:23 UTC
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

Comment 12 caulier.gilles 2008-03-03 09:45:33 UTC
Created attachment 23769 [details]
file name rename dialog during download from camera to prevent overwritting
Comment 13 Yip Wai Peng 2008-03-03 15:32:09 UTC
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
Comment 14 caulier.gilles 2008-03-03 15:41:20 UTC
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 
Comment 15 caulier.gilles 2008-03-03 15:43:08 UTC
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
Comment 16 David Fraser 2008-03-03 16:05:56 UTC
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...
Comment 17 David Fraser 2008-03-03 16:22:36 UTC
Created attachment 23770 [details]
Patch to include the filename in the source

Added patch that tries to include the filename in the source. Haven't tested
yet :-)
Comment 18 David Fraser 2008-03-03 16:59:15 UTC
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 :-)
Comment 19 David Fraser 2008-03-03 17:16:32 UTC
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
Comment 20 caulier.gilles 2008-03-03 18:19:44 UTC
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


Comment 21 Arnd Baecker 2008-03-03 19:26:30 UTC
*** Bug 157681 has been marked as a duplicate of this bug. ***
Comment 22 caulier.gilles 2008-03-07 09:16:34 UTC
Created attachment 23818 [details]
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
Comment 23 caulier.gilles 2008-03-07 09:49:59 UTC
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
Comment 24 David Fraser 2008-03-11 17:27:18 UTC
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.
Comment 25 Julien Narboux 2008-03-11 17:32:35 UTC
Hi all,

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

Best,

Julien
Comment 26 caulier.gilles 2008-03-11 19:01:07 UTC
Thanks for the reports.

I close this file now...

Gilles Caulier
Comment 27 Thiago Jung Bauermann 2008-03-13 03:35:57 UTC
Cool, thanks for working on this!
Comment 28 caulier.gilles 2008-03-14 11:58:05 UTC
Fixed in svn 

Gilles Caulier
Comment 29 Thiago Jung Bauermann 2008-04-14 07:07:33 UTC
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?
Comment 30 David Fraser 2008-04-22 16:15:47 UTC
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.
Comment 31 Thiago Jung Bauermann 2008-04-22 20:01:42 UTC
Great! Thanks for the explanation (and for the fix, of course!). I am
completely happy then. :-)

2008/4/22 David Fraser <davidf@sjsoft.com>:

[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:davidf@sjsoft.com">davidf@sjsoft.com</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
Comment 32 Thiago Jung Bauermann 2008-04-22 21:42:20 UTC
oh, man. what a mess. sorry about the html in the post above.
Comment 33 David Fraser 2008-07-22 16:22:31 UTC
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)