Bug 161783 - exif autorotation on import should be backgrounded / pipelined
Summary: exif autorotation on import should be backgrounded / pipelined
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: Import-PostProcessing (show other bugs)
Version: 3.0.0
Platform: Debian testing Linux
: NOR wishlist
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-05-08 11:16 UTC by Adrian von Bidder
Modified: 2017-08-16 05:55 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 3.0.0
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Adrian von Bidder 2008-05-08 11:16:06 UTC
Version:           0.9.3 (using KDE 3.5.9)
Installed from:    Debian testing/unstable Packages
OS:                Linux

Subject says it pretty much all.

Camera sits idle while autorotation is done, and CPU is idle while image is downloaded from camery.  1+1 = 2 :-)  Logical conclusion: batch the image downloads and immediately start up a background process doing the exif autorotation on all pictures already downloaded.

(Yet another think I should be writing code for when I only had the time ... :-)

(Yes, I know I can just switch off autorotation and batch it after via kipi.  But that's just ugly from a UI/Usability view, and I really like having the autoroation feature...  Autoration could even finish in the background, getting back to the GUI immediately after image import.  With an 8G CF card, image download takes a while, especially since on my machine EXIF rotation is even longer...)
Comment 1 caulier.gilles 2008-05-27 14:44:39 UTC
Adrian,

Autorotation is done in background using multithreading...

Gilles Caulier
Comment 2 Adrian von Bidder 2008-05-27 16:04:01 UTC
Cool.  In which version?
Comment 3 caulier.gilles 2008-05-27 16:11:03 UTC
Since 0.8.0 (:=)))

Gilles Caulier
Comment 4 Adrian von Bidder 2008-05-27 16:15:23 UTC
Not here.

Connect PTP camera.  Import pictures.  The following happens:  image is downloaded (activitiy led on camera flashes), then image is autorotated (while nothing happens on camera), next image is downloaded, next image is autorotated. etc.  takes ages for loads of images, and 50% CPU is idle and 50% USB link is idle.

No multithreading/backgrounding of any work is being done.
Comment 5 caulier.gilles 2008-12-16 15:06:01 UTC
No. Downloading + autorotation is performed in a separated thread against application gui tread.

If you have a double core cpu, downloading is done on one cpu and digiKam still available to work. Just iconize camera gui windows and go back to album gui to continue to work. camera operation are done in background. gui still suitable.

For me this entry is invalid...

Gilles Caulier
Comment 6 Adrian von Bidder 2009-02-03 12:50:30 UTC
Gilles,

download and autorotate is done in a separate thread from the GUI, yes, maybe.  BUT: download *and* autorotate are in the same thread.  If autorotate where in a different thread, the application could already start downloading the next image while rotating the last one.  This would speed up the image downloading process by up to 50% on my machine (where rotating an image and downloading an image is about equally fast, if all images are rotated)

cheers
-- vbi
Comment 7 Adam Porter 2011-03-18 17:29:23 UTC
I was going to file this bug myself, but I see it's already been reported.  :)

Please, do fix this.  It makes downloading photos take far, far longer than necessary.  Even if the EXIF rotation was just put off until all the photos had been downloaded, and done all at once before the import operation finished, it'd still be much faster.

Having the USB connection sit idle while Digikam rotates photos is just a waste of time. :/
Comment 8 bubukind 2011-05-01 13:49:28 UTC
+1, I have a fast cf card (16G) and 4 cores but still have to wait a long time for importing to finish.
Doing the download/rotation separately in a task-based manner would probably speed the whole thing up a _lot_.
Comment 9 Islam Wazery 2012-06-12 16:03:15 UTC
Git commit 1b9359549f2e234fd21bf8e595a983fbebe8f134 by Islam Wazery.
Committed on 12/06/2012 at 17:57.
Pushed by wazery into branch 'development/3.0.0'.

Pipelined exif autorotation on import, needs review

M  +40   -10   utilities/cameragui/controller/cameracontroller.cpp
M  +5    -1    utilities/cameragui/controller/cameracontroller.h
M  +30   -10   utilities/cameragui/main/cameraui.cpp
M  +2    -1    utilities/cameragui/main/cameraui.h
M  +3    -0    utilities/cameragui/main/cameraui_p.h

http://commits.kde.org/digikam/1b9359549f2e234fd21bf8e595a983fbebe8f134
Comment 10 caulier.gilles 2012-06-12 19:37:28 UTC
Islam,

In your code, you have used QtConcurrentRun, QFuture, and QFutureWatcher.

KDE provide a more simplest way to manage parallelization of thread, through ThreadWeaver KDE API :

http://api.kde.org/4.8-api/kdelibs-apidocs/threadweaver/html/index.html

We already use it in kipi-plugins with this wrapper class :

https://projects.kde.org/projects/extragear/graphics/kipi-plugins/repository/revisions/master/show/common/libkipiplugins/tools/threads

We use it already in some plugins based on ActionThread Class as CameraControler :

https://projects.kde.org/projects/extragear/graphics/kipi-plugins/repository/revisions/master/entry/dngconverter/plugin/actionthread.h#L47

https://projects.kde.org/projects/extragear/graphics/kipi-plugins/repository/revisions/master/entry/jpeglossless/plugin/actionthread.h#L47

https://projects.kde.org/projects/extragear/graphics/kipi-plugins/repository/revisions/master/entry/timeadjust/actionthread.h#L49

https://projects.kde.org/projects/extragear/graphics/kipi-plugins/repository/revisions/master/entry/rawconverter/manager/actionthread.h#L55

The goal is to run more than one thread at the same time, according with CPU core available. For ex, if you use an i5 computer, 4 core are available, so 4 images are processed at the same time.

Look the test implementation done in kipi-plugins based on JPEGLossLess core tool :

https://projects.kde.org/projects/extragear/graphics/kipi-plugins/repository/revisions/master/show/tests/multithreading

... it's a stand alone test program...

I don't have checked/tested your code, but if you try to parallelize processing in digiKam import tool, i recommend highly to use kpactionthreadbase.cpp implementation in digiKam core. Personally, i plan to use it into BatchQueueManager in the future. i will share this class between kipi-plugins and digiKam through libkdcraw or libkexiv2 (both depend of this libraries).

What do you think about ?

Gilles Caulier
Comment 11 Islam Wazery 2012-06-18 22:06:08 UTC
I have some stashed work that uses the ThreadWeaver class, but I feel the autorotation process is not that big to implement a class for it, so I used QtConcurrent but the only missing thing in my code is that I need to separate the autorotation from the download, the current code I wrote makes a new thread for autorotation and continues the download process, but I need to modify it to rotate as much files as it cans while the download is running in a separate thread. 

Do you think I should use ThreadWeaver not QtConcurrent, or just modify this QtConcurrent code?

P.S. When I removed the exif autorotation the whole download process time remains the same, I don't know a good way to test the time that autorotation takes to finish.
Comment 12 Marcel Wiesweg 2012-06-19 16:53:54 UTC
IMO, which of the "modern" threading techniques you use is a matter of taste and of your personal experience. I, for example, never use ThreadWeaver because I've never used it and did not learn the API. Nor QtConcurrent btw. I'm a friend of signal-based techniques; doesn't mean you have to use them.
You need to check if you need the possibility to cancel a threaded operation (usually: yes) and provide progress info (usually: yes) and how this can be done with your chosen API, here they can differ. In my experience, multithreading the operations is not much of a problem, you'll need more work to provide control over the multithreaded process.
Comment 13 caulier.gilles 2012-06-22 13:04:48 UTC
Islam,

The real choice to parallel programming interface choice will depend of a technical issue, reported in the past in bugzilla.

The problem is simple : when user connect a DLSR to computer to upload item from device to digiKam, this can take a while. During this operation, device battery will be discharged, which can be a problem in pro photograph workflow. 

Note that not all photographer have a stand alone memory card reader or included with computer, especially during a trip. Photographer use camera as memory card reader as well...

Currently, in 2.6.0, digiKam download items and process process batch operation when item is in computer. This is done, step by step, for each item.

The ideal case, to limit camera battery time consuming, is to download all items in one time, and to process batch operation after downloading process. Like this we can said to user that all items are now stored in computer and it can disconnect device from computer. The batch operation do not require camera anymore...

As you can see, the way to separate downloading and batch operations using parallel programming interface is important... I don't know which API can offer you to do it in easy way...

Gilles Caulier
Comment 14 Islam Wazery 2012-06-22 22:09:18 UTC
Thanks Gilles for your explanation, now it makes sense for me what I should do. Also thanks Marcel.
Comment 15 caulier.gilles 2012-06-28 15:01:57 UTC
Islam,

the class based on ThreadWeaver API from kipi-plugins have been backported to digiKam core :

https://projects.kde.org/projects/extragear/graphics/digikam/repository/entry/libs/threads/dactionthreadbase.h?rev=development%2F3.0.0

... if you want to use it. Current, another student port core BQM to it...

Gilles Caulier
Comment 16 Islam Wazery 2012-08-30 23:54:45 UTC
Git commit 12c8601fe9f97617bf4d22cffacaeaa7f8fa5f9c by Islam Wazery.
Committed on 31/08/2012 at 01:53.
Pushed by wazery into branch 'development/3.0.0'.

hanged auto-rotation method (backgrounded) and renamed cameraui files to importui
Related: bug 305961

M  +2    -2    digikam/dragdrop/albumdragdrop.cpp
M  +2    -2    digikam/dragdrop/imagedragdrop.cpp
M  +25   -20   digikam/main/digikamapp.cpp
M  +2    -2    digikam/main/digikamapp_p.h
M  +2    -2    utilities/importui/CMakeLists.txt
M  +6    -6    utilities/importui/backend/cameracontroller.cpp
M  +1    -1    utilities/importui/backend/camerahistoryupdater.cpp
M  +1    -1    utilities/importui/backend/camerahistoryupdater.h
M  +0    -1    utilities/importui/items/importdelegate.cpp
M  +0    -1    utilities/importui/items/importoverlays.cpp
R  +182  -143  utilities/importui/main/importui.cpp [from: utilities/importui/main/cameraui.cpp - 090% similarity]
R  +6    -5    utilities/importui/main/importui.h [from: utilities/importui/main/cameraui.h - 097% similarity]
R  +0    -0    utilities/importui/main/importui.rc [from: utilities/importui/main/cameraui.rc - 100% similarity]
R  +7    -6    utilities/importui/main/importui_p.h [from: utilities/importui/main/cameraui_p.h - 095% similarity]
M  +10   -10   utilities/importui/views/importiconview.cpp
M  +3    -3    utilities/importui/views/importpreviewview.cpp
M  +7    -7    utilities/importui/views/importview.cpp
M  +3    -3    utilities/importui/views/importview.h
M  +2    -2    utilities/importui/widgets/importcontextmenu.cpp
M  +6    -6    utilities/setup/cameratype.cpp
M  +3    -3    utilities/setup/cameratype.h

http://commits.kde.org/digikam/12c8601fe9f97617bf4d22cffacaeaa7f8fa5f9c