Bug 158377

Created attachment 23704 [details]
digikam window showing duplicated images

Created attachment 23705 [details]
download dialog with images for retry selected

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.

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

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.

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...

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

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.

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

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

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

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

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

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 

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

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...

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 :-)

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 :-)

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

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

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

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

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

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.

Hi all,

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

Best,

Julien

Thanks for the reports.

I close this file now...

Gilles Caulier

Cool, thanks for working on this!

Fixed in svn 

Gilles Caulier

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?

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.

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

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

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)