DDS using generic format types #11946
Conversation
…erting commit c34c956, because we will have a unified topic for IMU and that will force same frequency in DDS stream)
…t than USB debaults), DDS device tags correctly
src/device.h
Outdated
|
|
||
| bool device_changed_notifications_on() const { return _device_changed_notifications; } | ||
|
|
||
| bool should_use_basic_formats() const; |
There was a problem hiding this comment.
A few things: :)
- Not sure about the name - if we stuck to the existing convention, it would be
use_basic_formats_on()? - Should we really expose a function from the device? I'd bet the sensor pretty much knows how to get the context? If so, why not query the context directly and no go through the device? It's really not a device function, is it?
There was a problem hiding this comment.
The sensor uses it's device "owner" to access the context, no direct access.
Conceptually we can make this a per sensor decision, but I don't really see a case where it is not an application wide resolution. It is whether we convert at server side or host side and the CPU/bandwidth resources needed.
There was a problem hiding this comment.
I would argue it's not actually per device, though. It's per context.
I think the sensor should query the context.
There was a problem hiding this comment.
Updated sensor to parse this parameter
src/sensor.cpp
Outdated
|
|
||
| // Invoke processing blocks callback | ||
| const auto&& process_cb = make_callback([&, this](frame_holder f) { | ||
| const auto & process_cb = make_callback( [&, this]( frame_holder f ) { |
|
|
||
| void override_default_profiles( const std::map< std::string, realdds::dds_stream_profiles > & stream_name_to_profiles, | ||
| std::map< std::string, size_t > & stream_name_to_default_profile ) const; | ||
| size_t get_index_of_profile( const realdds::dds_stream_profiles & profiles, |
There was a problem hiding this comment.
These two look like they don't belong in the class and can be static in the cpp
There was a problem hiding this comment.
I don't like creating static functions just to hide the signature. This is why they are private functions of the class. All the public interface is on the first lines of the class so users don't need to scroll down and "get to know" the private parts if it is not important for them. I considered making the parameters members of the class but decided not to, currently.
There was a problem hiding this comment.
But then it dirties the header and pollutes the build space when changes are made. It's implementation. If it doesn't need to be in class, it shouldn't. My opinion. Do what you think is right.
| "}" ); | ||
| nlohmann::json j = { | ||
| { "dds-discovery", false }, | ||
| { "use-basic-formats", true } |
unit-tests/dds/d405.py
Outdated
| # dds.video_stream_profile( 30, dds.stream_format("RGBA"), 1280, 720 ), | ||
| # dds.video_stream_profile( 30, dds.stream_format("RGB2"), 1280, 720 ), | ||
| # dds.video_stream_profile( 30, dds.stream_format("rgb8"), 1280, 720 ), | ||
| dds.video_stream_profile( 30, dds.stream_format("rgb8"), 1280, 720 ), |
There was a problem hiding this comment.
I don't know if your question is for the colored infrared stream or general.
For the D405 color stream and also for the other modules I re-enabled rgb8 because that is what the users will see after LibRS will convert the received streams.
For the colored infrared it was more of inertia, doing the same all over the files. The stream is not broadcasted anyway.
There was a problem hiding this comment.
Fixed, as previously discussed
2bcc566 to
edff921
Compare
rs-dds-adaptersends only profiles of "generic" formats: YUYV/UYVY for color, no interleaved formats etc...The adapter also sets default profiles that are different from the USB defaults. D400 default color format is RGB8 for USB, here using YUYV, Infrared default resolution matches depth default resolution.
Context is saving the json params it receives during construction, device is using relevant parameters without the need for context to parse the parameters