Skip to content

add convenience method & sample code for registration #226

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

Merged
merged 10 commits into from
May 12, 2015

Conversation

floe
Copy link
Contributor

@floe floe commented May 7, 2015

Thanks again @larshg and @xlz for your sample code. I've rewritten it to not use OpenCV, as we probably want to remove that dependency at some point (see also the PR by @christiankerl). Also, I have not yet implemented the depth buffer from @xlz's example, will do that soon.

@xlz
Copy link
Member

xlz commented May 8, 2015

is rounding the right way to go here - no subpixel precision?

RGB images are much oversampled than depth images. More sampling precision on RGB would have little improvement.

 r_off = d_off*3;

3 is just hardcoded bytes_per_pixel value. VA-API and Tegra hardware jpeg decoders return in BGRA format. They would need 4 for bytes_per_pixel.

@floe
Copy link
Contributor Author

floe commented May 8, 2015

Good point about bytes_per_pixel, I'll fix that.

@floe floe mentioned this pull request May 8, 2015
@floe
Copy link
Contributor Author

floe commented May 10, 2015

Any test reports from Windows or MacOS? I'm reluctant to merge this otherwise.

@gaborpapp
Copy link
Contributor

Thanks for your work on this. I tested on MacOS 10.9.5. Registration seems to work, although I get some noise at the bottom of the image.

screen shot 2015-05-10 at 20 43 06

Also I get a crash on exit, which might not be related, but I don't get it with the latest 585d4c4 upstream/master.
Crash report:

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0 libusb-1.0.0.dylib 0x00000001091a0886 usbi_log_v + 70 (core.c:2290)
1 libusb-1.0.0.dylib 0x000000010919eb5f usbi_log + 127 (core.c:2378)
2 libusb-1.0.0.dylib 0x00000001091a9e76 darwin_cancel_transfer + 438 (darwin_usb.c:1746)
3 libusb-1.0.0.dylib 0x00000001091a3163 libusb_cancel_transfer + 99 (io.c:1500)
4 libfreenect2.dylib 0x0000000108f8128a libfreenect2::usb::TransferPool::cancel() + 138 (deque:299)
5 libfreenect2.dylib 0x0000000108f902cb libfreenect2::Freenect2DeviceImpl::stop() + 411 (libfreenect2.cpp:567)
6 Protonect 0x0000000108f6eb99 main + 3033 (Protonect.cpp:106)
7 libdyld.dylib 0x00007fff876a15fd start + 1

@xlz
Copy link
Member

xlz commented May 10, 2015

The noise:

    uint8_t registered[depth->height*depth->width*3];
    registration.apply(rgb,depth,registered);
    cv::imshow("registered", cv::Mat(depth->height, depth->width, CV_8UC3, registered));

Need a memset here, and probably it's not a good idea to define a huge array on stack. registered = cv::Mat::zeros() should work.

The crash is a race condition, discussed in #212.

@gaborpapp
Copy link
Contributor

Thanks! memset and the xlz@c6f6e57 commit mentioned in #212 indeed fixed both issues.

@floe
Copy link
Contributor Author

floe commented May 10, 2015

Thanks for testing - instead of memset, I've modified the inner loop to also set skipped pixels to zero, so we don't need to memset everything to zeroes which will get overwritten anyway.

@gaborpapp
Copy link
Contributor

Thanks. This also solves the issue.

@xlz
Copy link
Member

xlz commented May 10, 2015

FTBFS with MSVC 2013 here:

uint8_t registered[depth->height*depth->width*rgb->bytes_per_pixel];

Error C2057: expected constant expression

@floe
Copy link
Contributor Author

floe commented May 10, 2015

Ooops, OK (a bit surprising, but as you said, it's probably a better idea to move the buffer to the heap anyway). I'll fix that ASAP.

@xlz
Copy link
Member

xlz commented May 10, 2015

Also, this line causes stack overflow with MSVC:

libfreenect2::Registration registration(dev->getIrCameraParams(), dev->getColorCameraParams());

After replacing this and registered with heap allocation, it works correctly on Windows.

@floe
Copy link
Contributor Author

floe commented May 11, 2015

Also, this line causes stack overflow with MSVC:

Now that is somewhat surprising, why should this line be an issue? IMHO it should be valid C++, can you post a stacktrace?

@xlz
Copy link
Member

xlz commented May 11, 2015

The stack overflow happens upon entering main().

Probably because libfreenect2::Registration is quite large on main()'s stack: 3392KB.

@floe
Copy link
Contributor Author

floe commented May 11, 2015

Ah, OK. I would have assumed the default stack is a lot bigger, but it's fixed now.

@larshg
Copy link
Contributor

larshg commented May 11, 2015

I had to add #include <stdint.h> to make it work for VS2013 Update 4 else uint8_t is unknown.

And it seems that default stack is 1 MB :)

@floe
Copy link
Contributor Author

floe commented May 11, 2015

Also fixed, now using unsigned char* which is probably the most portable solution.

@larshg
Copy link
Contributor

larshg commented May 11, 2015

Works great 👍

@@ -87,6 +91,10 @@ int main(int argc, char *argv[])
cv::imshow("ir", cv::Mat(ir->height, ir->width, CV_32FC1, ir->data) / 20000.0f);
cv::imshow("depth", cv::Mat(depth->height, depth->width, CV_32FC1, depth->data) / 4500.0f);

if (!registered) registered = new unsigned char[depth->height*depth->width*rgb->bytes_per_pixel];

Choose a reason for hiding this comment

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

Given that the depth frame size is already known to be 512x424 if I recall correctly, is there any reason to not to allocate outside of the loop?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Depending on the color frame decoder, rgb->bits_per_pixel is either 3 or 4, so I'd like to keep it consistent.

@floe
Copy link
Contributor Author

floe commented May 11, 2015

OK, since everything works as expected across all major platforms, I'll merge this tomorrow; additional features will be added via separate PRs. Thanks for contributing, everyone!

@JuantAldea
Copy link

This are the results I am getting from the registration:

i) noticeable radial distortion in the closet. Shouldn't the depth data be undistorted first?

ii) pixel duplication, you can see my hand and the pen appear two times. Is this normal? Shouldn't registration be a bijection?

Camera calibration parameters used are those returned by the lib.

pixel_duplication_radial_distortion

I'm quite newbie on this subject so I might be missing something.

@floe
Copy link
Contributor Author

floe commented May 12, 2015

@JuantAldea

  1. the depth data is already being undistorted, but the built-in parameters are probably just not as good as those you would get from an individual chessboard calibration. In any case, if you compare it to the raw depth data, I think the distortion is already somewhat better.
  2. this is a fundamental geometric limitation, from the POV of the color camera, the pixels on the right are directly behind the pen, too. This will be solved in a second PR by using a depth buffer.

@JuantAldea
Copy link

@floe Thanks.

I'd say it looks like the depth isn't undistorted but the color is "distorted" so that it matches the depth.

I've superimposed the three images (registration, depth, and IR) as layers on this [1] Gimp file. You can play a little with the transparency and the layer order.

[1] https://www.dropbox.com/s/t2nnf737jvt4b3y/IR_DEPTH_REGISTERED.xcf?dl=0

@floe
Copy link
Contributor Author

floe commented May 12, 2015

@JuantAldea good point, got it backwards myself. Nice example, thanks!

floe added a commit that referenced this pull request May 12, 2015
add convenience method & sample code for registration
@floe floe merged commit cda9ea3 into OpenKinect:master May 12, 2015
@JuantAldea
Copy link

So is this working as intended? Shouldn't the depth be undistorted?

@floe
Copy link
Contributor Author

floe commented May 12, 2015

As far as we know, the parameters stored in the device are specific for this operation only, i.e. mapping color onto depth. Unless @sh0 knows about some more internals (such as the tables at the end of DepthCameraParamsResponse), this is all we can currently support without external calibration.

@JuantAldea
Copy link

Got it, thanks.

@floe
Copy link
Contributor Author

floe commented May 12, 2015

Actually, maybe I was posting too soon: in theory, the parameters for standalone depth undistortion should be there, too. I'll re-read the old discussion in #41 tomorrow.

@floe
Copy link
Contributor Author

floe commented May 13, 2015

I was obviously posting this too late at night yesterday evening, of course it's possible to undistort the depth image separately - that's exactly what I added undistort_map for in the first place :-)

@robotsorcerer
Copy link

I'm trying to do a face detection with the Protonect.cpp code. I need to apply a haarclassifier on the registered image. I tried with opencv haarcascade class

//-- Read the registration stream
registration.open( -1 );
if( registration.isOpened() )
{
for(;;)
{ registration >> frame;
//-- 3. Apply the classifier to the frame
if( !frame.empty() )
{ detectAndDisplay( frame ); }
else
{ printf(" --(!) No captured frame -- Break!"); break; }
int c = waitKey(10);
if( (char)c == 'c' ) { break; }
}
}
}

dev->stop();
dev->close();

but I kept getting a member error error: request for member ‘open’ in ‘registration’, which is of pointer type ‘libfreenect2::Registration*’ (maybe you meant to use ‘->’ ?) registration.open( -1 );
What is the correct way to access the registered/depth/rgb/undistorted frames?

@floe
Copy link
Contributor Author

floe commented Jul 1, 2015

@robotsorcerer
Copy link

Thanks! I seem to have some preliminary results on a face detection algorithm using haarcascades
myface

my code here.
Will appreciate contributions~!

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

Successfully merging this pull request may close these issues.

6 participants