Stuck in adding seeds

[LamboAventador]

Hi,
Thanks for our sharing. I'm trying to use Bundler+PMVS workflow, the console seem to be stuck in like below. How could I solve this problem?

pan@T830:/bundler_sfm/examples/ET$ ../../bin/genOption pmvs/
pan@T830:/bundler_sfm/examples/ET$ ../../bin/pmvs2 pmvs/ option-0000

../../bin/pmvs2
pmvs/
option-0000

--- Summary of specified options ---

of timages: 4 (enumeration)

of oimages: 0 (enumeration)

level: 1 csize: 2
threshold: 0.7 wsize: 7
minImageNum: 3 CPU: 8
useVisData: 1 sequence: -1

Reading images: ****
1 Harris running ...2 Harris running ...3 Harris running ...4 Harris running ...227 harris done
201 harris done
DoG running...DoG running...211 harris done
208 harris done
DoG running...DoG running...315 dog done
314 dog done
315 dog done
316 dog done
done
adding seeds

[sguyader]

Same problem here on Linux, pmvs2 stucks at "adding seeds".

[pmoulon]

Could be linked to the last updated #37 or #38. (@nh2 any idea?)

@LamboAventador & @sguyader Can you try with the repo checkout at e5663f1690c836a7f8e8300b1c988008b43c405f?

[sguyader]

I tried compiling against commit e5663f1, but it still gets stuck at adding seeds.

[pmoulon]

Can you try another dataset https://github.com/openMVG/ImageDataset_SceauxCastle?

[sguyader]

Still the same with the castle dataset:

--------------------------------------------------
--- Summary of specified options ---
# of timages: 5 (enumeration)
# of oimages: 0 (enumeration)
level: 1  csize: 2
threshold: 0.7  wsize: 7
minImageNum: 3  CPU: 4
useVisData: 1  sequence: -1
--------------------------------------------------
Reading images: *****
1 7 Harris running ...3 Harris running ...Harris running ...6 Harris running ...4273 harris done
DoG running...4092 harris done
DoG running...4357 harris done
DoG running...4409 harris done
DoG running...5855 dog done
10 Harris running ...4190 harris done
DoG running...5782 dog done
5867 dog done
5857 dog done
5869 dog done
done
adding seeds 

For information, I used VisualSFM, which uses the cmvs and pmvs2 binaries I just built. I also tried to use pmvs2 manually on the cmvs output files.

[pmoulon]

Can you try an older commit to see if there is everything wrong in the update?

[sguyader]

Still the same with commit a067274
I tried with a much older commit, but it failed to compile.

[pmoulon]

Which platform are you using (Mac, Linux, PC)?

[sguyader]

I'm on linux (Manjaro, up-to-date).

[pmoulon]

Can you try again with the last pushed version?

[edward81]

I'm stuck with the same bug Archlinux with the package cmvs-pmvs-git. So the bug is still here.

[pmoulon]

Can you try in an ubuntu docker to see if you have the same behavior?

[mostlyuseful]

Compiling and running pmvs2 in a docker ubuntu container (gcc) results in the same error.

Debugger tells me the main thread is hanging in Cseed::run in this block:

for (int i = 0; i < m_fm.m_CPU; ++i)
    thrd_join(threads[i], NULL);

While all eight worker threads are waiting in CpatchOrganizerS::addPatch, trying to acquire a write lock: m_fm.m_imageLocks[index].wrlock();

Seems like a matching unlock is missing.

[pmoulon]

Thank you for looking to the issue deeper. I did not write the main code, I just maintain it.
It could be cool if we could find the issue and fix it.
What is happening if you are running the code with 1 CPU m_fm.m_CPU = 1?

[mostlyuseful]

I think I fixed it 😃

Setting m_fm.m_CPU to 1 exposed the exact same behavior as with multiple threads. After inspecting the source code I've come to the conclusion that the code itself should work. No unmatched lock/unlock pairs as far as I can see. So I guess it's going wrong somewhere inside RWMutex.

I replaced the RWMutex-based m_fm.m_imageLocks and m_fm.m_countLocks with vectors of std::mutex, used std::lock_guard and updated C++ language level to C++17 (needed for std::optional to handle voluntary locking).

The program runs without hiccups and the produced model looks right.

I'll clean up the patch and create a pull request. You can decide whether you want to incorporate it or rather fix the underlying library issue. I am open for suggestions, right now just happy it works again 🎉

[pmoulon]

Happy to hear it we were able to reproduce the behavior in simpler settings!
I'm happy if we can use mutex.
I have a minor comment about the optional (see if we can stay c++11 compliant)...

[pmoulon]

@edward81 @sguyader Can you try the fix that is pushed in master?