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

Add ability to open multiple devices from different processes (libtm and t265) #4261

Closed
orshefi opened this issue Jun 21, 2019 · 11 comments
Closed

Comments

@orshefi
Copy link

orshefi commented Jun 21, 2019

Hi guys,

As a followup to this issue #4163 on which I shared my application (and I believe many other users need) to open up multiple cameras from different processes (f.e. two different docker containers)
I would like to offer my and fellow workers help as contributors to accomplish the needed changes to support this ability with libtm.

#4163 (comment)
this comment in specific explains the issue I encountered.

I believe the issue occurs for the library registers a libusb event listener to identify usb attachments, I think somehow it performs system call futex (lock) on the usb bus which prevents other devices to be discovered.

Please guide us through this feature

@MartyG-RealSense
Copy link
Collaborator

Hi @orshefi - Librealsense already has some ability to stream with multiple cameras / processes, though with some rules to follow. I hope the information in the link below will be a useful reference for your project goal.

https://github.com/IntelRealSense/librealsense/blob/master/doc/rs400_support.md#multi-streaming-model

@orshefi
Copy link
Author

orshefi commented Jun 22, 2019

@MartyG-RealSense - Please read carefully.

I want to stream two different t265 devices from two differet processes.

As you can see from the ticket I attached this feature isn't supported in the current version.

I want to add this feature.

@MartyG-RealSense
Copy link
Collaborator

I am not certain why the systems described in the documentation do not meet your needs. It says that it allows you to "Stream from camera A in one process and from camera B in another process". Can you provide more details please about how this is different from streaming different T265 from two different processes, please.

@dorodnic
Copy link
Contributor

Hi @orshefi
Appreciate your offer to help. Here is IMHO the source of the problem:
third-party/libtm/libtm/src/Manager.h:152 - libtm, the original T265 "driver", is detecting all T265 devices and preemptively opening libusb handles to all of them and storing these handles in a singleton.

The way this should be solved, is by removing Manager class all together and passing librealsense backend object directly to Device. librealsense already has a mechanism for device discovery, it is thread-safe, lazy and well-tested with depth-cameras. However, based on my gut-feeling it can take developer roughly 1-2 weeks to get this done, and I'm still looking for a time slot for it.

However, it's very likely a work-around can be implemented in less time, without changing the architecture. My thinking was to replace this code with a "lazy" version of itself. So basically the manager would identify a device, download firmware, but instead of creating actual Device object would create a new DevicePlaceholder object and store it in a map. This DevicePlaceholder object would only create Device and lock the handle upon first access. It's not perfect, and I'm not 100% sure it will work, but we would appreciate any assistance on this, since we also want to enable this use-case as soon as possible.

@orshefi
Copy link
Author

orshefi commented Jun 23, 2019

Hi @dorodnic,

About the second solution (DevicePlaceholder), I could not understand so far where does the locking happen and why does it effects the different processes, does it locks all the devices with the t265 vendor:product ID?

About the first (and right solution from my point of view) solution (removing Manager):

  1. I understand we will have to implemet a class that uses libtm device class and implements an inherent of device abstract class
  2. The next step is to change the create_devices inside context.cpp to handle the new device class we created upon t265 vid:pid detection and register them as devices to the context object.

As long as I port the libtm device class to implement the device abstract methods the architecture should support this new device type and the query mechanism should handle the new camera?
What steps am I missing?

@dorodnic
Copy link
Contributor

does it locks all the devices with the t265 vendor:product ID

Yes. IMHO libusb_ref_device(m.device); is locking the device. Now it does this for every T265 device, as soon as it is detected, and libuvc does not allow multiple open handles (from same or different processes)

What steps am I missing?

You understand the problem correctly. From my experience with the code base I can still advise that this is a sizable amount of work. Firmware download flow must be handled somewhere, also getting to libusb handle from context can require some hacking. Finally, getting two processes to play nice with each other without getting occasional resource busy errors (let's say when both are doing query_devices), requires extra work. There is refactoring already in progress to address some of these limitations.

@orshefi
Copy link
Author

orshefi commented Jun 24, 2019

Hi @dorodnic

First thing first, I want to start sharing my code changes in order to receive guidance and review from you guys, please send me a reference on how to do so.

Here's what I have so far:

  1. Removed all lines that initialize manager in the code base
  2. Changed pick_tm2_devices to find the USB device info that corresponds to the product id of the t265

Next thing I'd like to do is to change the device creation method, but there is something I can't understand.

What the difference between the TrackingDevice.h and the Device.h, here are my thoughts:

  1. TrackingDevice seems to me as a class that contains callback functions that are called by a listener instance, which means I need to start a listener that usually is initialized by the manager (?)
  2. Also I need to initialize a Device.h instance and somehow connect the listener to it in order to enable the functions that been implemented in the tm-device class to remain usefull, do I need to address the Device.h file directly (now its not part of the project include_directories).

In another matter, I think that firmware update shouldn't be a part of the project build because it sets a constraint that forces the project to be built on an internet-enabled station and on top of that it couples the process of build and fw update which is decoupled by nature (f.e. the process of fw update in D415).
Another issue it imposes is that one cannot easily freeze fw version of the t265 module - a fact that create a difficulty with reproducibility of a system behavior

@dorodnic
Copy link
Contributor

The issue with persistent update is that Movidius does not store firmware image on-board, and was designed to receive new firmware upon every power-up.

You can open pull-request to share and collaborate on this.

I'm not entirely sure about Device.h. The listener in librealsense is generic, abstracted via device_watcher interface, plugging into the context. From there, the new detected device_info should trigger creation of a tm2_device / sensor that would get libusb handle from info and pass it to the constructor of TrackingDevice

@orshefi
Copy link
Author

orshefi commented Jun 25, 2019

@dorodnic now I understand the complexity involved with the right solution, it is very complicated and demands a deep understanding of the different libraries.

The other solution you suggested seems reasonable so before working my way through it I'd like to clarify some things I may not fully understand:

The way I understood the flow (device_hub, pipeline, sensors, etc..) eventually it all comes down to the create function in tm-info.h before the actual query of the data from the camera.
Hence I believe the locking (in the form of libusb_ref_device() ? ) should be done inside the create function.
If you agree with me there is no use of the DeviceHolder Class, rather implement a locking function inside TrackingDevice.h and use it only in the create function.

Do you agree? Do the libusb_ref_device is the only function that locks the device and blocking the query_device mechanism from discovering the devices from two processes?

@dorodnic
Copy link
Contributor

dorodnic commented Jun 25, 2019

As far as I understand, yes, libusb_ref_device is the one that locks, but I'm not expert in this specific API (@matkatz ?)
The problem with locking in TrackingDevice ctor is that the TrackingManager will still preemptively create such devices regardless if they are needed or not. This is why I was thinking to keep Manager's map, but instead of it mapping TrackingDevice* for it to map some other object that only creates TrackingDevice upon first access.
In addition, I hope we will be able to get the proper solution of the ground soon enough. It's a follow-up of #4267 where @matkatz gave Firmware-Update USB devices very similar treatment.

@RealSenseSupport
Copy link
Collaborator

Thank you for highlighting the issue of not being able to open multiple devices with the T265. We have moved our focus to our next generation of products and consequently, we will not be addressing this issue in the T265.

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

No branches or pull requests

4 participants