Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allows structured_light pipeline to be run from Python #2452

Merged
merged 1 commit into from Mar 14, 2020

Conversation

r2d3
Copy link
Contributor

@r2d3 r2d3 commented Mar 8, 2020

SinusoidalPattern::unwrapPhaseMap now takes an InputArray instead of InputArrayOfArrays to correct a Python binding problem

present a scriptable HistogramPhaseUnwrapping::create

This patch solves issue #1704 and allow to write a whole pipeline including reliability computation.

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under OpenCV (BSD) License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

@r2d3 r2d3 changed the title WIP: Allows structured_light pipeline to be run from Python Allows structured_light pipeline to be run from Python Mar 8, 2020
Copy link
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for contribution!

It would be nice if you could add simple test for Python API (no need to perform complex accuracy check).

See example: https://github.com/opencv/opencv_contrib/blob/4.2.0/modules/rgbd/misc/python/test/test_rgbd.py
or similar Python tests from main opencv repository.

Test data is stored in opencv_extra repository.

// alias for scripting
CV_WRAP
static Ptr<HistogramPhaseUnwrapping> create( int width, int height, float histThresh = static_cast<float>(3 * CV_PI * CV_PI),
int nbrOfSmallBins = 10, int nbrOfLargeBins = 5);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please follow the existed practice and avoid unpacking of "Params" structure.
You can check this code as an example:

https://github.com/opencv/opencv/blob/850414a501d5e6dba111c92d73fdd05794c7061d/modules/features2d/include/opencv2/features2d.hpp#L653-L654

Copy link
Contributor Author

@r2d3 r2d3 Mar 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing practice is not clear as the structured_light itself is using 2 practices:

  • Params unpacking in GrayCodePattern::create
  • smart pointer for SinusoidalPattern::create

Some codes pass a smart pointer like this is done for SinusoidalPattern::create

https://github.com/opencv/opencv_contrib/blob/master/modules/structured_light/include/opencv2/structured_light/sinusoidalpattern.hpp#L100

but I followed the solution used by GrayCodePattern::create

https://github.com/opencv/opencv_contrib/blob/master/modules/structured_light/include/opencv2/structured_light/graycodepattern.hpp#L89

Because changing the create API (instead of overloading it) will break current codes using structured_light.

The code you point out is not using Params struct to pass parameters.

If you just pass a const Params&, the Python binding generator will not generate the needed code. Am I right ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Params" class should be marked for bindings wrapping too (use CV_EXPORTS_W_SIMPLE)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I managed to bind the HistogramPhaseUnwrapping::Params; What I was missing is the typedef in pyopencv_phase_unwrapping.hpp as it is done in pyopencv_features2d.hpp

I added the Python test for structured_light replicating what is done in C++. However, this shown a "bug" in HistogramPhaseUnwrapping::unwrapPhaseMap where the unwrappedPhaseMap were uninitialized in places outside of the mask.
It is not clear if this were intentional or not.

@r2d3 r2d3 changed the title Allows structured_light pipeline to be run from Python WIP: Allows structured_light pipeline to be run from Python Mar 12, 2020
SinusoidalPattern::unwrapPhaseMap now takes an InputArray instead of InputArrayOfArrays to correct a Python binding problem
present a scriptable HistogramPhaseUnwrapping::create

replicate C++ structured_light test in Python

PhaseUnwrapping now init unwrappedPhase so pixel outside the mask area are set to 0

python binding for HistogramPhaseUnwrapping::Params to use HistogramPhaseUnwrapping::create
@r2d3 r2d3 changed the title WIP: Allows structured_light pipeline to be run from Python Allows structured_light pipeline to be run from Python Mar 12, 2020
Copy link
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done! Thank you for contribution 👍

@opencv-pushbot opencv-pushbot merged commit 2a32a5a into opencv:3.4 Mar 14, 2020
@alalek alalek mentioned this pull request Mar 17, 2020
@hitmanman
Copy link

hitmanman commented Apr 26, 2020

Hi the following codings make my python kernel die.
The problem is
'unwrapped_phase_map = sinus.unwrapPhaseMap(wrapped_phase_map, cam_size, shadow_mask)'
Can you tell me how to correct it?
thank you

import cv2
import numpy as np
import math
import sys

cam_size = [-1,-1]

params = cv2.structured_light_SinusoidalPattern_Params()
params.width = 1024 #pixel width
params.height = 1024 #pixel height
params.nbrOfPeriods = 20 # amount of waves
params.setMarkers = False # patterns /w or /wo markers
params.horizontal = False
params.methodId = cv2.structured_light.FTP
#params.methodId = cv2.structured_light.PSP
#params.methodId = cv2.structured_light.FAPS
params.shiftValue = 2.0 * math.pi / 3.0
params.nbrOfPixelsBetweenMarkers = 70

sinus = cv2.structured_light_SinusoidalPattern.create(params)
patterns = sinus.generate()
count = 0

img = []
while count < len(patterns[1]) :
img.append(patterns[1][count])
count += 1

captures = []
captures.append(img[0])
captures.append(img[1])
captures.append(img[2])

wrapped_phase_map, shadow_mask = sinus.computePhaseMap(captures)
cam_size = (img[0].shape[0], img[0].shape[1])
print("before")
unwrapped_phase_map=np.zeros([img[0].shape[0], img[0].shape[1]],np.uint8)
unwrapped_phase_map = sinus.unwrapPhaseMap(wrapped_phase_map, cam_size, shadow_mask)
print("after")

@r2d3
Copy link
Contributor Author

r2d3 commented Apr 26, 2020

Hi the following codings make my python kernel die.

The problem is
'unwrapped_phase_map = sinus.unwrapPhaseMap(wrapped_phase_map, cam_size, shadow_mask)'
Can you tell me how to correct it?

Which version of OpenCV are you using ?

import cv2
print(cv2.__version__)

You should have a recent 3.4 release (ideally 3.4.10) to get the correction

Regards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants