After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 633972 - Cannot import some CR2 files any more
Cannot import some CR2 files any more
Status: RESOLVED WONTFIX
Product: f-spot
Classification: Other
Component: Import
0.8.0
Other Linux
: Normal blocker
: 0.8.1
Assigned To: Tim Howard
F-spot maintainers
gnome[unmaintained]
: 630860 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2010-11-04 07:41 UTC by Heiner
Modified: 2018-07-01 09:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
output (22.55 KB, text/plain)
2010-11-04 19:51 UTC, Heiner
  Details
f-spot import window (69.69 KB, image/jpeg)
2010-11-04 19:51 UTC, Heiner
  Details
f-spot --debug (2.00 KB, text/plain)
2010-11-12 19:12 UTC, Pat Suwalski
  Details
Make sure the amount of data we are intended to read for an IFD entry is sane (933 bytes, patch)
2010-11-17 05:19 UTC, Tim Howard
needs-work Details | Review
revised patch to check SubIFD length (966 bytes, patch)
2010-11-17 17:59 UTC, Tim Howard
committed Details | Review
Sanity check for entry sizes (1.08 KB, patch)
2010-12-12 03:25 UTC, Tim Howard
reviewed Details | Review
Make sure the IFD entry size is even possible before trying to create it (1.25 KB, patch)
2010-12-12 17:25 UTC, Tim Howard
committed Details | Review

Description Heiner 2010-11-04 07:41:21 UTC
When I connect my camera (Canon EOS 1000D) to my computer,
f-spot will start and show the import window asking me to select an
import source (directory or camera). When I select my camera, f-spot
will try to show the preview images for the cr2 files, which will take a
lot of time, although there are just a few photos on the camera. After a
while, the thumbnails all show up with date 01.01.0001 (german format).

When I press the import button now, f-spot will try to import the files
and crash after a while. Afterwards, the database will be corrupted and
f-spot will crash again shortly after each start.

I could import the images from my camera without any problems using
shotwell. Afterwards, I could import the cr2 files from the directory,
where shotwell had saved them, into f-spot via drag'n drop. This way,
everything was imported correctly (including metadata).

This issue appeared after upgrading from Ubuntu lucid to maverick.
Comment 1 Fabio Durán Verdugo 2010-11-04 14:58:40 UTC
please can you open fspot from console and paste here all result?
Comment 2 Heiner 2010-11-04 19:51:01 UTC
Created attachment 173847 [details]
output
Comment 3 Heiner 2010-11-04 19:51:42 UTC
Created attachment 173848 [details]
f-spot import window
Comment 4 Heiner 2010-11-04 19:53:05 UTC
Here is the output of "f-spot --debug" and a screenshot of the imnport window.
Comment 5 Pat Suwalski 2010-11-12 19:04:29 UTC
I have the exact same problem. It happened on the upgrade to Maverick.

I decided to create a more specific test case. I created two bootable USB sticks with Ubuntu 10.10: i386 and amd64. I also removed the camera communication layer by placing one of the candidate CR2 files in its own folder before importing.

So, with a perfectly clean database and no chance of communication error, here are my results:

 - i386 works fine
 - amd64 chokes on parsing the file

As soon as I choose the folder for import, f-spot wants to regenerate the thumbnail, and hangs. The "f-spot --debug" output confirms this. The only relevant lines in the debug output are:

[1 Debug 18:53:51.284] Received controller event: SourceChanged
[1 Debug 18:53:51.288] Received controller event: PhotoScanStarted
[1 Debug 18:53:51.411] Invalid thumbnail, reloading: file:///home/ubuntu/Desktop/test/IMG_0857.CR2

Again, this a completely clean f-spot. As with the other user, Shotwell reads the file perfectly.

My instinct is that someone made a non-64-bit-safe change somewhere.
Comment 6 Pat Suwalski 2010-11-12 19:08:24 UTC
Right after I finished typing that, the thumbnail finally showed up, after 11 minutes and 20 seconds, and several gigs of RAM used!

Relevant lines in debug output:

[1 Debug 19:05:03.895] open uri = file:///home/ubuntu/Desktop/test/IMG_0857.CR2
[1 Debug 19:05:03.907] Received controller event: PhotoScanFinished
[1 Debug 19:05:04.125] Received controller event: ImportStarted
[1 Debug 19:05:12.306] Loading image took 00:11:20.8635560

I forgot to mention that attaching an strace to the process shows this kind of output in quick succession:

(...)
read(22, "", 4)                         = 0
read(22, "", 4)                         = 0
read(22, "", 4)                         = 0
read(22, "", 4)                         = 0
read(22, "", 4)                         = 0
read(22, "", 4)                         = 0
(...)
Comment 7 Pat Suwalski 2010-11-12 19:12:36 UTC
Created attachment 174349 [details]
f-spot --debug
Comment 8 Pat Suwalski 2010-11-12 19:15:43 UTC
Here is a CR2 file that exhibits the problem (I have many):

http://pat.suwalski.net/temp/IMG_0857.CR2

It comes from a Canon Digital Rebel XT.
Comment 9 Heiner 2010-11-13 20:14:11 UTC
"My instinct is that someone made a non-64-bit-safe change somewhere."

I don't think so, because mine is a 32-bit system. I've also tried with a clean database - same issue.
Comment 10 Tim Howard 2010-11-17 05:19:27 UTC
Created attachment 174647 [details] [review]
Make sure the amount of data we are intended to read for an IFD entry is sane

This seemed like an OK way to deal with the situation. If the size of data we calculated based on other information is bigger than the file (or what's left of the file based on the various offsets) then how can we possibly read the data in?
Comment 11 Ruben Vermeersch 2010-11-17 09:50:15 UTC
Comment on attachment 174647 [details] [review]
Make sure the amount of data we are intended to read for an IFD entry is sane

Good catch! Now, when we detect something like this, we don't just silently ignore it. The number may seem bogus, but we're handling undocumented file formats here, it may be crucial.

When we detect stuff that's not understood, the right thing to do is to call MarkAsCorrupt which prevents the file from being written. Search for usages, there are plenty of examples.

Again, this is data-safety conservatism, but at least we know what is going on.
Comment 12 Tim Howard 2010-11-17 15:05:34 UTC
Thanks Ruben. I considered that might be the right behavior after I attached the patch. I've seen it in use before. I will update the patch ASAP.
Comment 13 Tim Howard 2010-11-17 17:59:19 UTC
Created attachment 174704 [details] [review]
revised patch to check SubIFD length
Comment 14 Heiner 2010-11-18 09:04:19 UTC
I have applied the patch, but it doesn't seem to fix this issue, at least not completely. It still takes a lot of time until the previews are displayed, when I try to import images from my camera. After a while, they show up - and now they all have the date 01.01.1970 (1970/01/01). At least I can import them now without crashing f-spot. But metadata are still rubbish.

I still don't think that the problem is the file format. Once the files are out of my camera, I don't have any problems to import them, even without applying the patch. I can import any cr2 file into f-spot unless it comes directly from my camera. If the files are on any other storage device (harddisk, usb stick, memory card), f-spot imports them even with correct metadata. It just doesn't work if I connect my camera to my computer and try to import any images directly from it. I could do this with any older f-spot version.

So I still think that this bug happens in the communication between
f-spot and the camera.
Comment 15 Pat Suwalski 2010-11-21 21:55:22 UTC
Heiner, I wonder if we can track this down to the camera mode. My Canon has two options: "PC Connection" and "PTP". Which one are you using for your camera, if you can set it? I find PTP has always worked more slowly, but that the Canon-proprietary mode has never been too stable for me.
Comment 16 Tim Howard 2010-11-21 21:57:42 UTC
Pat, sorry, someone should probably have made a comment on here. It would seem that most likely it's due to PTP. I don't personally know much about it at the moment but in general it seems to be regarded as rubbish.
Comment 17 Heiner 2010-11-22 06:35:49 UTC
Thanks Pat, this seems to be an important hint. My EOS 1000D offers no possibility to change the transfer mode, and I think it's PTP. This would explain, why I can transfer photos using a cardreader, while using the camera doesn't work.

I guess, using a card reader is the better way to transfer files anyway, but I still wonder, why this worked in older f-spot versions.
Comment 18 Ruben Vermeersch 2010-11-29 09:48:43 UTC
Review of attachment 174704 [details] [review]:

Merged this patch with small cosmetic changes and attached a test case. Definitely solves an issue with the provided sample file.

::: src/TagLib/IFD/IFDReader.cs
@@ +935,3 @@
+                    // This is impossible right?
+                    if (base_offset + offset > file.Length)
+                    {

Opening brace goes on the same line in Taglib#. Also, they use tabs in the code.
Comment 19 Tim Howard 2010-11-30 02:04:04 UTC
OK, thanks.
Comment 20 Pat Suwalski 2010-12-01 05:36:49 UTC
Gentlemen, getting close. I checked out the latest TagLib and applied it to f-spot 0.8. I can confirm that IMG_0857.CR2 from comment #8 works.

I started re-importing my library and found another image that causes f-spot to choke in the same way. I have uploaded it here:

  http://pat.suwalski.net/temp/img_6165.cr2

Hopefully the problem is related and not difficult to correct.
Comment 21 Tim Howard 2010-12-10 03:22:39 UTC
No problem here importing when I build from master.
Comment 22 Pat Suwalski 2010-12-11 00:57:23 UTC
Using a fresh checkout, the images from before import correctly. I decided to try to re-import my entire library into a clean f-spot. These images seem problematic still:

http://pat.suwalski.net/temp/img_6665.cr2
http://pat.suwalski.net/temp/img_7595.cr2
http://pat.suwalski.net/temp/img_7931.cr2

What happens is that 460-or-so images import correctly, then it hits the first one, and does the usual 10-13 minute high-cpu high-memory pause, then it keeps going, but very slowly (one image per minute rather than 2 per second).

Eventually it all shows up, but when it tries to generate the thumbnail, there is another 10 minute pause, and if I click on the thumbnail, another 10 minute pause. So, there is something still not right.
Comment 23 Tim Howard 2010-12-12 03:25:07 UTC
Created attachment 176265 [details] [review]
Sanity check for entry sizes
Comment 24 Pat Suwalski 2010-12-12 06:20:48 UTC
The patch in comment #23 seemed to work very well. It allowed me to import my entire collection without a hitch.
Comment 25 Ruben Vermeersch 2010-12-12 15:41:07 UTC
Review of attachment 176265 [details] [review]:

::: src/TagLib/IFD/IFDReader.cs
@@ +300,3 @@
+                // ignore this entry and keep going
+                if (value_count + base_offset > max_offset)
+                    continue;

Shouldn't we mark the file as possibly corrupt here? Since we have no idea what's going on. Might be that canon does things that are not entirely up to spec. Better to play it safe here.
Comment 26 Tim Howard 2010-12-12 17:14:02 UTC
I was thinking that because when entry is null from CreateIFDEntry we just keep going that it would be a similar situation. However, I see that in CreateIFDEntry that the cases where that should happen we've already marked the file as possibly corrupt. I'll get a new patch attached.
Comment 27 Tim Howard 2010-12-12 17:25:04 UTC
Created attachment 176300 [details] [review]
Make sure the IFD entry size is even possible before trying to create it

Marking the file as possibly corrupt now before continuing.
Comment 28 Pat Suwalski 2010-12-14 18:30:34 UTC
Going back to the original issue Heiner reported in comment #1, when using the camera as the source, and it is set to be in PTP mode, the dates on the CR2 thumbnails are definitely wrong. Rather than 0001, I get the year 1969.

I haven't been able to try it with the camera in "PC Connection" mode, since that only works once in a blue moon (this is not a new issue).
Comment 29 Tim Howard 2010-12-14 18:32:15 UTC
This is against master with this patch?
Comment 30 Pat Suwalski 2010-12-14 18:36:36 UTC
(In reply to comment #29)
> This is against master with this patch?

It is. That said, I was unable to import before from the camera because the camera would go to sleep in the 13 minutes it took to time out. So, I don't know that it's a new issue.
Comment 31 Nuno Ferreira 2010-12-15 01:49:11 UTC
*** Bug 630860 has been marked as a duplicate of this bug. ***
Comment 32 Nuno Ferreira 2010-12-15 01:57:06 UTC
(repeating  information I added to duplicate bug #630860)

Just tested this latest patch, with it applied I was able to import ~2000 CR2 photos with no significant increase in memory usage.

Without the patch, the import (of a subset) of those photos always failed.
Comment 33 Ruben Vermeersch 2010-12-19 11:32:49 UTC
Comment on attachment 176300 [details] [review]
Make sure the IFD entry size is even possible before trying to create it

Committed! Can this be closed now?
Comment 34 Pat Suwalski 2010-12-19 17:06:17 UTC
(In reply to comment #33)
> (From update of attachment 176300 [details] [review])
> Committed! Can this be closed now?

Have you seen comments #28-30? It harkens back to the original bug report.
Comment 35 André Klapper 2018-07-01 09:02:17 UTC
f-spot is not under active development anymore, has not seen code changes for five years, and saw its last tarball release in the year 2010.
Its codebase has been archived: https://gitlab.gnome.org/Archive/f-spot/commits/master

Closing this report as WONTFIX as part of Bugzilla Housekeeping to reflect reality. Please feel free to reopen this ticket (or rather transfer the project to GNOME Gitlab, as GNOME Bugzilla is deprecated) if anyone takes the responsibility for active development again.