bbenligiray / stag

STag: A Stable Fiducial Marker System
MIT License
187 stars 52 forks source link

Changes to make this more useful as an external library #10

Open slensgra opened 4 years ago

slensgra commented 4 years ago

I'd like to get the ROS node I have running put into a more finished state by separating out the STag code and the ROS wrapper. To get that node running I had to make a couple of changes to this code:

Are you interested in having these changes contributed back into your code? If so, I can do the separation work and improve the style of what I have then make a PR into this code. I could also add an alternate implementation for the windows specific code which would work on linux.

Here is a diff for your reference (messy but can be cleaned if you want the changes). https://github.com/bbenligiray/stag/compare/master...dartmouthrobotics:master

Norfo commented 4 years ago

Hello, @slensgra! Can you explain where you added destructor, please. I'm also faced with this problem.

bbenligiray commented 4 years ago

Hey @slensgra , I'd be happy to merge your PR!

slensgra commented 4 years ago

Great, let me get one ready.

@Norfo See destructor in EDInterface at line 27 of EDInterface.cpp in the diff above

Norfo commented 4 years ago

@slensgra, thanks! By the way, @bbenligiray, do you have any plans to rewrite ED classes? As far as I understand, they are quite old. I faced with a problem that they cannot be used together with OpenCV4. In addition, there is a possibility to rewrite them with cuda support.

bbenligiray commented 4 years ago

The ED code was my coauthors' work and I barely wrapped it for this project. Cihan Topal had a student refactor it with C++ STL (https://github.com/CihanTopal/ED_Lib). As far as I know it's slightly slower, but I would prefer to use that for production. I think there is some ongoing work to integrate this version to OpenCV by third parties. https://github.com/opencv/opencv_contrib/pull/2313

Especially the edge segment detection part of the algorithm is considerably sequential, so it requires some modifications for a CUDA implementation. There is a proof of concept below from 2012, but I didn't see the code being released. https://ieeexplore.ieee.org/document/6152450