MarekKowalski / LiveScan3D

LiveScan3D is a system designed for real time 3D reconstruction using multiple Azure Kinect or Kinect v2 depth sensors simultaneously at real time speed.
MIT License
749 stars 202 forks source link

Changed Color Image Mapping to be more accurate #48

Closed ChristopherRemde closed 3 years ago

ChristopherRemde commented 4 years ago

Hi Marek,

while using Livescan, we noticed that it uses the color_image_to_depth_camera Transformation, which doesn't handle occlusion correctly. I think this was done for performance reasons?

Our team wanted to have more accuracy, so I implemented the depth_image_to_color_camera transformation. Here are the results:

Current version, which is using the color_image_to_depth_camera transformation:

snapshot01

This Pull Request, which is using the depth_image_to_color_camera transformation:

snapshot00

There are of course downsides to this.

This mapping technique is quite a bit slower than the original transformation, mainly because it produces a larger pointcloud, I implemented a color image resizer, which scales the color image down to the same height as the depth image, while preserving the aspect ratio of the color image. This produce a much smaller pointcloud. I could also save some processing speed on the "StoreFrame" function by avoiding the use of "Vector.push_back".

But even using these tricks, this mapping has a higher performance cost, which could leed to a decrease in FPS on older systems. However I think this is in a reasonable range, as I tested the software on two mid-tier processors (Desktop Ryzen 2600 and Mobile 8750H) and they held a stable 30 FPS while recording.

That said, I'm also quite a newbie regarding C++, so I hope I didn't do anything catastrophically wrong. I tested the software quite a bit and didn't encounter any errors/Memory leaks.

ChristopherRemde commented 3 years ago

Hi Marek,

Thanks, yes I can do that! I'm on vacation in the next two weeks, so I'll provide the feature after that.

Best Regards!

MarekKowalski commented 3 years ago

Hi Christopher,

I think you answered to my previous comment about making this optional, which I since deleted - sorry. On second though I realized that it makes sense for this feature to be enabled in all cases, especially given it works at full framerate on a mid-tier CPUs. Also, when you checked the performance, how many Kinects did you run on the same machine?

Thanks,

Marek

ChristopherRemde commented 3 years ago

Hi Marek,

Thank you for looking through the code and the detailed comments! I'll try to sort them out after the vacation.

Regarding the performance:

Both of the PCs I used weren't able to run more than one Kinect at full frame rate, even with the standard Livescan( for Azure Kinect), but I don't know if this is due to processor or USB limitations. So I didn't test this pull request with two Kinects at the same machine.

I'll try to get a bit more detailed performance statistics, so that you can decide if this should be enabled by default or as a an option!

ChristopherRemde commented 3 years ago

Hi Marek,

So I did a Performance Test today of both the original Azure Kinect branch and this fix... And I could not really detect any significant difference between the two. But I think my testing method is flawed. Currently I set one breakpoint at the start of the update loop in liveScanClient.cpp and then read out the time it took to update one frame (Visual studio shows this per default at the right end of the line at the breakpoint). This happens in Debug mode, so the performance is obiously distorted, compared to Release Build performance.

Do you know any way to better measure the performance, maybe even for a Release Build?

MarekKowalski commented 3 years ago

Hi Christopher,

Visual Studio has a great profiler, which allows for measuring the performance of your code. Here is the documentation: https://docs.microsoft.com/en-us/visualstudio/profiling/?view=vs-2019

Marek

ChristopherRemde commented 3 years ago

Hi Marek,

thanks for your tip! I used the tool and the results are kinda interesting for me. So here they are:

I expected the results to be the other way around, so that this pull request is slower. I tried to to find the reason for this, and it basically comes down to the "mfmjpegdec.dll" not being used in this pull request. I don't really know the reason for this, maybe this has something to do with the K4A-SDK? I tested the Azure Kinect Branch and this pull request both with SDK Version 1.4.1

Here are the detailed results if you are interested:

Azure Kinect Branch:

Performance_Original_Azure_Kinect_Branch

This pull request:

Performance_Original_Mapping_Fix_Branch

Regarding the performance of "INTER_LINEAR" and INTER_NEAREST": I couldn't really detect any difference between the two, so I changed the interpolation method to inter_linear.

Otherwise I fixed all the changes you requested with the latest commit.

MarekKowalski commented 3 years ago

Hi Christopher,

Thanks a lot for looking into this. Did you profile the app in Debug or Release mode? The times looks to be quite long which makes me think it might have been Debug. Would you be able to profile again in Release? Sorry I'm not able to help, but I do not have access to a Kinect at the moment.

If that looks good I am happy to accept the PR.

Marek

ChristopherRemde commented 3 years ago

Hello Marek,

yes you were right, I did profile in Debug mode, my bad! I didn't realize you could also profile in Release mode.

I did profile in Release mode again, and here the numbers look a lot more coherent with what I'd expect:

So there is a performance penalty while using this pull request. There seems to be some performance overhead while profiling though, because running the build without it, the FPS-Counter in the Client Window never drops under 29.72 FPS while recording (for this pull request) . So I'm not really sure which measure is to trust.

MarekKowalski commented 3 years ago

Thanks for running the test Christopher. I think these numbers are fine, I'll now accept the PR.

Best regards,

Marek

ChristopherRemde commented 3 years ago

Thanks a lot Marek for taking the time to review this PR!