search

IntelRealSense / realsense_sdk_zr300

Toolkit built on top of the Intel® RealSense™ Cross Platform API (librealsense)
Apache License 2.0
55 stars 39 forks source link

Confusing type re-definition #2

Closed ddiakopoulos closed 7 years ago

ddiakopoulos commented 8 years ago

rs::core::pixel_format - as found in places like this https://github.com/IntelRealSense/realsense_sdk/blob/master/sdk/include/rs/utils/librealsense_conversion_utils.h

There is a 1:1 mapping between realsense_sdk and librealsense. Since librealsense is a dependency of this repo, why not use those types directly instead of creating a translation layer?

oordan commented 8 years ago

We are not happy with the translation of some definitions at all. However, we need to provide a camera independent API to the MWs. So any API, which is used by MWs libraries, is defined by the SDK. librealsense is a dependency of this repo, but not of the MWs libs.

leonidk commented 8 years ago

LibRealSense is a camera independent API.

This is a one to one mapping and you're not providing any abstraction. The include file from librealsense (#include "rs/core/types.h") is already being included so we're simply polluting people's namespaces.

Especially if you're not happy with this, it's still an open issue.

oordan commented 8 years ago

librealsense is depth camera independent API, which is suitable for streaming video (preview), but doesn't cover full platform camera capabilities, and it doesn't make sense to extend it to be.

We don't abstract depth camera for applications which use the SDK (we are strongly against it), but we compromised on abstraction for MWs - we do one to one mapping, but not using the lrs types, so MWs don't have to include rs.hpp. "rs/core/types.h" is an SDK include file. The application indeed depends on librealsense API (by design), and unfortunately has to translate to independent types for the MW.

Did I get your comments right, or am I missing anything?

oordan commented 7 years ago

Fixed some issues in SDK components consumed by MWs, to become independent of librealsense. We also cleaned unused definitions, which were redefined but not used.