Bug 303980

Created attachment 72712 [details]
Screenshot out of Digikam 2.7.0 after trying to convert multiple raw(srw) files to dng

Smit,

Another problem with parallelization of code. It sound like it do not manage properly the source items list to process. It miss a lock somewhere or an id is not incremented for some conditions ? Can you reproduce on your computer ?

Robert,

If you put only 2 items to process, it work fine ?
Which type of cpu do you use ? i3, i5, i7 ?

Can you run kdebugdialog and turn on kipiplugins, kdcraw, kipi, kexiv2 debug spaces, and from a console to run again digiKam and DNG converter. Post console trace here for investiguations.

Thanks in advance

Gilles Caulier

Hi

I confirm the bug. I tried 7 images. But I get 5 distinct outputs only. Other two are repeated. I will look into it.

Smit

Git commit d41ac0f984cf1f1d81e1c843f6712e2dbbd80298 by Smit Mehta.
Committed on 08/08/2012 at 13:43.
Pushed by smitmehta into branch 'development/3.0.0'.

Temporary fix : Added a mutex lock.
Not a problem with actiontread.cpp and batchdialog.cpp
TODO : figure out why multi-threading does unexpected stuff in dngprocessor.

M  +2    -1    dngconverter/plugin/actionthread.cpp

http://commits.kde.org/kipi-plugins/d41ac0f984cf1f1d81e1c843f6712e2dbbd80298

Smit,

1/ You have commited into 3.0.0 branch where master is for production. Please backport your commits to 2.x.
Generaly, i sync 3.x with 2.x through git merge command. It's safe to patch production fixes to master as it will be backported to 3.x branch later.

About merge stuff, look here : 

http://community.kde.org/Digikam/GSoC2012#Branches_Creation_and_Maintenance

2/ I think that re-entrency problem with DNGProcessor is due to temp file created in background. Look at destPath string : it's created using current timestamp. Time granularity is seconds, through this method :

http://qt-project.org/doc/qt-4.8/qdatetime.html#toTime_t

If more than one conversion is started during same second, there is a non re-entrancy issue...

Gilles Caulier

Hi Gilles

I am sorry, i will revert back the commit here and make a commit to 2.x now. You can backport it later.

Also, the problem is not due to timestamp. It used to be earlier. But then, i appended it with the filename itself, so it gave the uniqueness to the temporary file created. There must be some other problem.

Smit

Git commit 0800a58ef73180785dfbadde3f51a107e6d6bc05 by Smit Mehta.
Committed on 08/08/2012 at 14:22.
Pushed by smitmehta into branch 'master'.

Temporary fix : Added a mutex lock.
Not a problem with actiontread.cpp and batchdialog.cpp
TODO : figure out why multi-threading does unexpected stuff in dngprocessor.

M  +2    -1    dngconverter/plugin/actionthread.cpp

http://commits.kde.org/kipi-plugins/0800a58ef73180785dfbadde3f51a107e6d6bc05

Hi Gilles

Has anyone been able to figure out yet the problem in re-entrancy in dngconverter? The file name is not a problem as I said before, i had appended it with the filename also when you had similar problems on the photos of your london trip. It looks like a problem with some core dng stuff, where something is not thread safe. I have added a mutex lock to temporary avoid it, so atleast it works fine currently. Should I close this bug?

Smit

Smit,

Right. the problem is the re-entancy with DNGConverter core implementation.

In fact, it's wrong to share DNGWriter instance between ActionThread private container with all Tasks started in separated threads.

To be clear, nothing must be shared between ActionThread and Task. For each Task instance, a DNGWriter instance must be created.

Also, there is another point where implementation is wrong : the temporary file created by each Task instance. The temporary file name is based on timestamp where granularity is the seconds. This is not enough. We must not use timestamp to create unique temporary file. I already explain the problem with my comment #5.

You can take look how i manage Task and ActionThread clasess in digiKam Batch Queue Manager that i recently parallelized. I see the same problem during port. Look here :

https://projects.kde.org/projects/extragear/graphics/digikam/repository/revisions/master/entry/utilities/queuemanager/manager/actionthread.cpp

https://projects.kde.org/projects/extragear/graphics/digikam/repository/revisions/master/entry/utilities/queuemanager/manager/task.cpp

Note : RAW converter kipi-plugins has exactly the same implementation and must be fixed in the same way.

Do you will able to manage these problem before 3.0.0 final release planed around 25 January 2013 ?

Gilles Caulier

Git commit 791ce58f862d2907a2f9aee62a69082e7bf30ca7 by Gilles Caulier.
Committed on 21/12/2012 at 09:43.
Pushed by cgilles into branch 'master'.

To preserve re-entrancy with RawDecodingIface, move all private items from ActionThread to Task. Do not share ActionThread::Private with Task.
Code is very similar than digiKam BQM core, and same fixes must be applied to DNGConverter.
CCMAIL: smit.meh@gmail.com

M  +1    -1    rawconverter/dialogs/batchdialog.cpp
M  +2    -2    rawconverter/dialogs/singledialog.cpp
M  +74   -52   rawconverter/manager/actionthread.cpp
M  +14   -6    rawconverter/manager/actionthread.h

http://commits.kde.org/kipi-plugins/791ce58f862d2907a2f9aee62a69082e7bf30ca7

Smit,

I need to know if you will manage DNGConverter fixes, based on my last commit CC in comment #10, which patch RAWConverter implementation.

Fixes must be done before final release 3.0.0. Do you have time to work on it in a near future ?

Gilles

Git commit 631b4b76d637e6a344f3b642d992ff602d199860 by Gilles Caulier.
Committed on 21/12/2012 at 10:00.
Pushed by cgilles into branch 'master'.

for a better visibilty, separate ActionThread and Task class iomplementation
CCMAIL: smit.meh@gmail.com

M  +1    -0    rawconverter/CMakeLists.txt
M  +1    -218  rawconverter/manager/actionthread.cpp
M  +1    -38   rawconverter/manager/actionthread.h
A  +250  -0    rawconverter/manager/task.cpp     [License: GPL (v2+)]
A  +82   -0    rawconverter/manager/task.h     [License: GPL (v2+)]

http://commits.kde.org/kipi-plugins/631b4b76d637e6a344f3b642d992ff602d199860

Git commit d3dccabce94c4f8cbb783b384b591cef359ddd1a by Smit Mehta.
Committed on 24/12/2012 at 19:34.
Pushed by smitmehta into branch 'master'.

Moved all private items from ActionThread to Task and hence stopped sharing ActionThread::Private with Task.
Separated ActionThread and Task for better visibility

M  +1    -0    dngconverter/CMakeLists.txt
M  +22   -125  dngconverter/plugin/actionthread.cpp
M  +5    -34   dngconverter/plugin/actionthread.h
A  +212  -0    dngconverter/plugin/task.cpp     [License: GPL (v2+)]
C  +13   -49   dngconverter/plugin/task.h [from: dngconverter/plugin/actionthread.h - 062% similarity]     [License: UNKNOWN]  *

The files marked with a * at the end have a non valid license. Please read: http://techbase.kde.org/Policies/Licensing_Policy and use the headers which are listed at that page.


http://commits.kde.org/kipi-plugins/d3dccabce94c4f8cbb783b384b591cef359ddd1a