freesig / nuitrack-rs

A Rust crate giving access to the Nuitrack SDK
2 stars 3 forks source link

Data ownership #18

Open freesig opened 5 years ago

freesig commented 5 years ago

I'm assuming that all data given to the callbacks is given as a pointer that will out last the callback. I'm not sure this is the case as it is a pointer to a vector that may be destructed.

freesig commented 5 years ago

Im pretty sure for the skeleton data I should be stopping c from freeing it (might not be possible because of how it's allocated, may need to store a shared pointer on the c side) using this method to free it

freesig commented 5 years ago

The RGBFrame and DepthFrame are a bit trickier because all we are given is a pointer and a length. There is no way to tell how long it lives.

freesig commented 5 years ago

Ok as of right now I'm pretty sure all we can saftley do is assume the pointers live as long as the callback is called for. This means to use the pointers you either need to make a copy of the data on the rust side or use them only within the callback

freesig commented 5 years ago

Summary

It is most likely that the data you are given in any of the call backs lives as long as the shared_ptr is alive. The signatures are all like:

viod callback(shared_ptr<Data> data);

So it would make sense that when noone holds that shared_ptr then the data is destroyed. This creates a bit of a problem for use because we can't use `shared_ptr. The only valid way I know of is to keep a static copy of the ptr on the c++ side then destroy the static copy when you are done using the raw pointer in rust. This is exactly what I'm doing each handler but they are only created and deleted once.

The problem is that the data ptrs will be created by nuitrack and some rate A while they will be destroyed by the user at some rate B. These two rates will be out of sync because I have no way of knowing what the user does with the pointer once it's in their callback. (ie. they may reasonably want the data to outlive the callback because it's being processed on some other thread)

With the rates out of sync there will potentially be some pool of static shared_ptr<Data> building up. All which have to be thread safe. I feel like this is going to be really hard to do with out it getting messy, slow and potentially creating deadlocks.

Alternative

The alternative is to hold a single static shared_ptr<Data> that is always free'd at the end of the callback. The data pointers lifetime is as long as the callbacks call Meaning that you either need to use the data in the callback (ie. draw it to the frame) or make a owned copy of it. Might need your feedback on this @mitchmindtree as the alternative is pretty limiting for the user but the original solution might be slow and unsafe.

mitchmindtree commented 5 years ago

Yeah I agree that it might be simplest/safest to go with your Alternative option and clone the underlying data - that way we can at least guarantee that the lifetime of the data on our end and don't have to deal with any uncertainty introduced by the nuitrack API.

This won't be as efficient as using the buffers given to us by nuitrack, but we should time the clone first to see if it's significant enough to cause any issues. If it takes less than a few milliseconds then I'd imagine we should be fine as we still have the majority of the frame to run update and view stuff in the nannou side.

freesig commented 5 years ago

Yeh no probs. I mean I can always do the first option later if we are experiencing latency issues. You could avoid the need to clone if you could provide the closure with the thing that the data is ending up in ie. for a RGBFrame from nui you are probably sending it straight to the GPU but I'm not sure if it's possible to do a GPU / vulkan call from the callback as it will be out of sync with you're main draw etc.

freesig commented 5 years ago

Actually I just realized theres no need to make a static for the alternative option because the ptr is alive for the whole callback function anyway:

// arg is the shared_ptr
const auto wrapper = [=](const auto arg){
    // Convert to simple structs and raw ptrs 
    auto sd = to_simple(arg);
    // Call the users callback
    cb(user_data, sd);
    // arg is destroyed here after the callback returns
};

I might do some timing of the current implementation and report the results here

freesig commented 5 years ago

Seems to be only using 3% of the calls with copying the data. And this is without even reusing the buffer. The bottleneck seems to actually be in the orbtec SDK

I'll wait for the frame buffer before testing this further.

When testing nuitrack-rs FPS it's capped at 31fps. With just RGBFrame on it's and no data processing it's 31fps. With RGBFrame, SkeletonData, DepthFrame it's between 23 - 31fps with no data processing.