Skip to content

Commit 3f79832

Browse files
author
Evgeni Raikhel
committed
Remove public per-feature struct header; caller defines its own struct matching the documented composite-option layout
The SDK must not ship any feature-specific typed struct for composite options - only the generic rs2_composite_option_id enum and the rs2_set/get_composite_option pair are public. Deleted include/librealsense2/h/rs_hkr_temporal_filter_dpp.h and its include from rs.h; the HKR Temporal Filter DPP wire layout (4 fields, 16 bytes) is now documented as a comment on RS2_COMPOSITE_OPTION_HKR_TEMPORAL_FILTER_DPP in rs_composite_option.h instead of as a type. Each caller of the generic API independently defines its own local struct matching that documented layout: d500_depth_sensor uses a private hkr_temporal_filter_dpp_wire_layout (file-scope, d500-device.cpp) only to compute the control's registered wire size; the realsense-viewer panel and the fake-transport mock example each define their own separate local copies to cast the raw bytes. None of these three structs are shared types - they just happen to agree on layout by convention, exactly like a real app would have to. Rebuilt realsense2, realsense-viewer, and the mock example (0 errors) and reconfirmed round-trip correctness and the exactly-one-set_xu/one-get_xu atomicity guarantee through the caller-owned-struct contract.
1 parent 63bb293 commit 3f79832

9 files changed

Lines changed: 119 additions & 89 deletions

File tree

common/device-model.cpp

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,25 @@ using namespace rs400;
2929
using rsutils::json;
3030
using namespace rs2::sw_update;
3131

32+
namespace {
33+
34+
// PROTOTYPE / DEMO: this viewer's OWN local struct matching the wire layout documented as a
35+
// comment on RS2_COMPOSITE_OPTION_HKR_TEMPORAL_FILTER_DPP in
36+
// include/librealsense2/h/rs_composite_option.h. The SDK ships no public type for composite
37+
// options - every caller (this viewer, the mock example under examples/) independently
38+
// defines its own copy of this struct matching the documented byte layout.
39+
#pragma pack( push, 1 )
40+
struct hkr_temporal_filter_dpp_layout
41+
{
42+
int32_t enabled; // 0 = Off, 1 = On
43+
float smooth_alpha; // range [0,1], default 0.4, step 0.01
44+
int32_t smooth_delta; // range [1,100], default 20, step 1
45+
int32_t persistency_index; // range [0,8], default 3, step 1
46+
};
47+
#pragma pack( pop )
48+
49+
} // namespace
50+
3251
namespace rs2
3352
{
3453
// RAII guard pairing BeginDisabled/EndDisabled: keeps them balanced even if an exception is
@@ -2776,10 +2795,12 @@ namespace rs2
27762795
// same way as other per-sensor extension features in this loop, e.g.
27772796
// depth_sensor above). Goes through the GENERIC rs2::composite_option_sensor
27782797
// get_composite_option/set_composite_option entry points, keyed by
2779-
// RS2_COMPOSITE_OPTION_HKR_TEMPORAL_FILTER_DPP - this viewer code is the only
2780-
// place that knows to cast the raw bytes to/from rs2_temporal_filter_dpp_config.
2781-
// All fields are sent together in ONE atomic UVC transaction when "Apply" is
2782-
// clicked - never as separate per-field option writes.
2798+
// RS2_COMPOSITE_OPTION_HKR_TEMPORAL_FILTER_DPP. The SDK ships no struct for
2799+
// this - this viewer casts the raw bytes to/from its own local
2800+
// hkr_temporal_filter_dpp_layout (defined above), matching the layout
2801+
// documented as a comment on the enumerator. All fields are sent together in
2802+
// ONE atomic UVC transaction when "Apply" is clicked - never as separate
2803+
// per-field option writes.
27832804
if (sub->s->is<rs2::composite_option_sensor>())
27842805
{
27852806
auto composite_sensor = sub->s->as<rs2::composite_option_sensor>();
@@ -2792,10 +2813,10 @@ namespace rs2
27922813
if (!sub->temporal_filter_dpp_populated)
27932814
{
27942815
auto bytes = composite_sensor.get_composite_option(RS2_COMPOSITE_OPTION_HKR_TEMPORAL_FILTER_DPP);
2795-
if (bytes.size() != sizeof(rs2_temporal_filter_dpp_config))
2816+
if (bytes.size() != sizeof(hkr_temporal_filter_dpp_layout))
27962817
throw std::runtime_error("HKR Temporal Filter DPP: unexpected payload size from get_composite_option");
27972818

2798-
rs2_temporal_filter_dpp_config cfg{};
2819+
hkr_temporal_filter_dpp_layout cfg{};
27992820
memcpy(&cfg, bytes.data(), sizeof(cfg));
28002821
sub->temporal_filter_dpp_enabled = cfg.enabled;
28012822
sub->temporal_filter_dpp_smooth_alpha = cfg.smooth_alpha;
@@ -2821,7 +2842,7 @@ namespace rs2
28212842
label = rsutils::string::from() << "Send##temporal_filter_dpp_send" << id;
28222843
if (ImGui::Button(label.c_str()))
28232844
{
2824-
rs2_temporal_filter_dpp_config cfg{};
2845+
hkr_temporal_filter_dpp_layout cfg{};
28252846
cfg.enabled = sub->temporal_filter_dpp_enabled;
28262847
cfg.smooth_alpha = sub->temporal_filter_dpp_smooth_alpha;
28272848
cfg.smooth_delta = sub->temporal_filter_dpp_smooth_delta;

examples/hkr-temporal-filter-dpp-mock/rs-hkr-temporal-filter-dpp-mock.cpp

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,14 @@
1313
// What this proves:
1414
// 1. Round-trip correctness: a payload sent via set_raw(void*, size) comes back
1515
// byte-identical in the SDK-allocated vector returned by get_raw(), once memcpy'd into
16-
// the caller's typed struct (rs2_temporal_filter_dpp_config) - exactly the contract of
17-
// the real rs2_set_composite_option(data, data_size) / rs2_get_composite_option()
18-
// (which returns an rs2_raw_data_buffer, since the caller has no generic way to know an
19-
// arbitrary option_id's wire size in advance - mirrors rs2_get_safety_preset).
16+
// THIS EXAMPLE'S OWN local struct (hkr_temporal_filter_dpp_layout, below) - exactly the
17+
// contract of the real rs2_set_composite_option(data, data_size) /
18+
// rs2_get_composite_option() (which returns an rs2_raw_data_buffer, since the caller has
19+
// no generic way to know an arbitrary option_id's wire size in advance - mirrors
20+
// rs2_get_safety_preset). The SDK ships NO typed struct for composite options: this
21+
// example defines its own struct matching the byte layout documented as a comment on
22+
// RS2_COMPOSITE_OPTION_HKR_TEMPORAL_FILTER_DPP in
23+
// include/librealsense2/h/rs_composite_option.h.
2024
// 2. Atomicity: set_raw() performs EXACTLY ONE set_xu() call, and get_raw() performs
2125
// EXACTLY ONE get_xu() call - i.e. the whole payload always travels as a single UVC
2226
// transaction, never as separate per-field writes/reads. This is the non-negotiable
@@ -28,9 +32,9 @@
2832
// - only the transport (platform::uvc_device) is faked.
2933

3034
#include <src/ds/structured-xu-control.h>
31-
#include <librealsense2/h/rs_hkr_temporal_filter_dpp.h>
3235

3336
#include <cassert>
37+
#include <cstdint>
3438
#include <cstring>
3539
#include <iostream>
3640
#include <stdexcept>
@@ -39,6 +43,20 @@ using namespace librealsense;
3943

4044
namespace {
4145

46+
// This example's OWN local struct - NOT an SDK type - matching the wire layout documented as
47+
// a comment on RS2_COMPOSITE_OPTION_HKR_TEMPORAL_FILTER_DPP in rs_composite_option.h. Any
48+
// real application would define its own copy of this exact struct the same way; the SDK does
49+
// not provide one.
50+
#pragma pack( push, 1 )
51+
struct hkr_temporal_filter_dpp_layout
52+
{
53+
int32_t enabled; // 0 = Off, 1 = On
54+
float smooth_alpha; // range [0,1], default 0.4, step 0.01
55+
int32_t smooth_delta; // range [1,100], default 20, step 1
56+
int32_t persistency_index; // range [0,8], default 3, step 1
57+
};
58+
#pragma pack( pop )
59+
4260
// Minimal fake transport standing in for the real UVC backend. Implements just enough of
4361
// platform::uvc_device to exercise xu_structured_control::get_raw/set_raw. Everything else is
4462
// unused by this test and throws loudly if ever invoked - this class is test scaffolding
@@ -114,9 +132,9 @@ try
114132

115133
// Generic control, registered for exactly this one composite option's wire size - mirrors
116134
// how d500_depth_sensor populates its _structured_controls registry.
117-
xu_structured_control control( depth_xu, DS5_HKR_TEMPORAL_FILTER_DPP, sizeof( rs2_temporal_filter_dpp_config ) );
135+
xu_structured_control control( depth_xu, DS5_HKR_TEMPORAL_FILTER_DPP, sizeof( hkr_temporal_filter_dpp_layout ) );
118136

119-
rs2_temporal_filter_dpp_config sent{};
137+
hkr_temporal_filter_dpp_layout sent{};
120138
sent.enabled = 1;
121139
sent.smooth_alpha = 0.4f;
122140
sent.smooth_delta = 20;
@@ -130,10 +148,10 @@ try
130148
control.set_raw( dev, &sent, sizeof( sent ) );
131149

132150
std::vector< uint8_t > bytes = control.get_raw( dev );
133-
if( bytes.size() != sizeof( rs2_temporal_filter_dpp_config ) )
151+
if( bytes.size() != sizeof( hkr_temporal_filter_dpp_layout ) )
134152
throw std::runtime_error( "get_raw returned an unexpected payload size" );
135153

136-
rs2_temporal_filter_dpp_config received{};
154+
hkr_temporal_filter_dpp_layout received{};
137155
std::memcpy( &received, bytes.data(), sizeof( received ) );
138156

139157
// 1) Round-trip correctness.

include/librealsense2/h/rs_composite_option.h

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,15 @@
99
* rs2_get_composite_option) shared by every composite (multi-field, atomically-exchanged) XU
1010
* control, keyed by an rs2_composite_option_id. This mirrors Orbbec OrbbecSDK's
1111
* setStructuredData(OBPropertyID, const void*, uint32_t) / getStructuredData(...) pattern:
12-
* there is no bespoke named function per feature - adding a new composite option means adding
13-
* one enumerator here (plus its typed payload struct, e.g. rs2_temporal_filter_dpp_config in
14-
* rs_hkr_temporal_filter_dpp.h) and registering it internally, NOT adding new public functions.
12+
* there is no bespoke named function per feature, and - importantly - the SDK does NOT ship a
13+
* public typed struct per feature either. Adding a new composite option means adding one
14+
* enumerator here (with its wire layout DOCUMENTED as a comment, not as a type) and
15+
* registering it internally, NOT adding new public functions or public structs. Callers define
16+
* their own local struct matching the documented layout for whichever option_id they use.
1517
*
1618
* Each call performs EXACTLY ONE UVC control transaction (one set_xu/get_xu round trip): all
17-
* fields of the option's payload struct travel together, atomically - never as separate
18-
* per-field writes/reads.
19+
* fields of the option's payload travel together, atomically - never as separate per-field
20+
* writes/reads.
1921
*/
2022

2123
#ifndef LIBREALSENSE_RS2_COMPOSITE_OPTION_H
@@ -28,27 +30,40 @@ extern "C" {
2830
#include "rs_types.h"
2931

3032
/**
31-
* Identifies which composite option (and therefore which typed payload struct) a
32-
* rs2_set_composite_option/rs2_get_composite_option call refers to. This enum is the ONLY
33+
* Identifies which composite option a rs2_set_composite_option/rs2_get_composite_option call
34+
* refers to. This enum (and the documented byte layout on each enumerator below) is the ONLY
3335
* per-feature footprint of the generic mechanism - the entry point functions themselves never
34-
* change.
36+
* change, and the SDK ships no typed struct for any of these. The caller/app is responsible
37+
* for defining its own local struct matching the documented layout and casting the raw bytes
38+
* to/from it.
3539
*/
3640
typedef enum rs2_composite_option_id
3741
{
38-
RS2_COMPOSITE_OPTION_HKR_TEMPORAL_FILTER_DPP, /**< Prototype: HKR/D555 Depth Post-Processing Temporal Filter. Payload: rs2_temporal_filter_dpp_config (see rs_hkr_temporal_filter_dpp.h) */
42+
/** Prototype: HKR/D555 Depth Post-Processing Temporal Filter.
43+
* Documented wire layout (tightly packed, no padding - all fields are naturally
44+
* 4-byte-aligned so no explicit packing pragma is required in the caller's struct),
45+
* 16 bytes total, in this exact field order:
46+
* int32_t enabled; 0 = Off, 1 = On
47+
* float smooth_alpha; range [0,1], default 0.4, step 0.01
48+
* int32_t smooth_delta; range [1,100], default 20, step 1
49+
* int32_t persistency_index; range [0,8], default 3, step 1
50+
* The SDK does not define a type for this - define your own local struct matching this
51+
* layout and pass &your_struct / sizeof(your_struct) to rs2_set/get_composite_option. */
52+
RS2_COMPOSITE_OPTION_HKR_TEMPORAL_FILTER_DPP,
3953
RS2_COMPOSITE_OPTION_COUNT
4054
} rs2_composite_option_id;
4155
const char* rs2_composite_option_id_to_string(rs2_composite_option_id id);
4256

4357
/**
4458
* rs2_set_composite_option - generic composite-option setter.
4559
* Writes data_size bytes from data to the device in ONE atomic UVC control transaction (one
46-
* set_xu call). The caller is responsible for knowing which typed struct corresponds to
47-
* option_id and passing its address + sizeof(...) as data/data_size.
60+
* set_xu call). The caller is responsible for knowing the documented wire layout for option_id
61+
* (see rs2_composite_option_id above) and passing a pointer to its own matching struct +
62+
* sizeof(...) as data/data_size. The SDK does not ship a typed struct for this.
4863
* \param[in] sensor Sensor that exposes the requested composite option
4964
* \param[in] option_id Which composite option to write
50-
* \param[in] data Pointer to the caller's typed payload struct
51-
* \param[in] data_size sizeof(...) of the caller's typed payload struct
65+
* \param[in] data Pointer to the caller's own struct matching the documented layout
66+
* \param[in] data_size sizeof(...) of the caller's struct
5267
* \param[out] error If non-null, receives any error that occurs during this call, otherwise, errors are ignored
5368
*/
5469
void rs2_set_composite_option(const rs2_sensor* sensor, rs2_composite_option_id option_id, const void* data, unsigned int data_size, rs2_error** error);
@@ -64,8 +79,9 @@ void rs2_set_composite_option(const rs2_sensor* sensor, rs2_composite_option_id
6479
* \param[in] option_id Which composite option to read
6580
* \param[out] error If non-null, receives any error that occurs during this call, otherwise, errors are ignored
6681
* \return SDK-allocated buffer holding the option's raw payload bytes; the
67-
* caller casts them to the typed struct that corresponds to option_id
68-
* (e.g. rs2_temporal_filter_dpp_config). Free with rs2_delete_raw_data.
82+
* caller casts them into its own local struct matching the
83+
* documented layout for option_id (see rs2_composite_option_id
84+
* above). Free with rs2_delete_raw_data.
6985
*/
7086
const rs2_raw_data_buffer* rs2_get_composite_option(const rs2_sensor* sensor, rs2_composite_option_id option_id, rs2_error** error);
7187

include/librealsense2/h/rs_hkr_temporal_filter_dpp.h

Lines changed: 0 additions & 50 deletions
This file was deleted.

include/librealsense2/hpp/rs_composite_option.hpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,10 @@ namespace rs2
4444
// Single atomic UVC transaction (one get_xu round trip). The SDK allocates the result
4545
// (the caller has no generic way to know option_id's wire size in advance) and this
4646
// wrapper hides the raw rs2_raw_data_buffer/manual-free entirely, returning a plain
47-
// std::vector<uint8_t> of the option's raw payload bytes - cast/memcpy them into the
48-
// typed struct that corresponds to option_id (e.g. rs2_temporal_filter_dpp_config).
47+
// std::vector<uint8_t> of the option's raw payload bytes. The SDK ships no typed
48+
// struct for this - cast/memcpy the bytes into your own local struct matching the
49+
// layout documented on the corresponding rs2_composite_option_id enumerator (see
50+
// rs_composite_option.h).
4951
std::vector<uint8_t> get_composite_option(rs2_composite_option_id option_id) const
5052
{
5153
rs2_error* e = nullptr;

include/librealsense2/rs.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ extern "C" {
2424
#include "h/rs_sensor.h"
2525
#include "h/rs_safety_sensor.h"
2626
#include "h/rs_eth_config.h"
27-
#include "h/rs_hkr_temporal_filter_dpp.h"
2827
#include "h/rs_composite_option.h"
2928

3029
#define RS2_API_MAJOR_VERSION 2

src/composite-option-interface.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,11 @@ class composite_option_interface
2323
virtual ~composite_option_interface() = default;
2424

2525
// Generic dispatch by option id. Each call performs EXACTLY ONE UVC control transaction
26-
// (one set_xu/get_xu). The bytes are an opaque blob whose layout is defined by whichever
27-
// typed struct corresponds to option_id (e.g. rs2_temporal_filter_dpp_config for
28-
// RS2_COMPOSITE_OPTION_HKR_TEMPORAL_FILTER_DPP) - the caller is responsible for casting.
26+
// (one set_xu/get_xu). The bytes are an opaque blob whose layout is documented as a
27+
// comment on the corresponding rs2_composite_option_id enumerator (see
28+
// rs_composite_option.h) - the SDK ships no typed struct for it. The caller is
29+
// responsible for defining its own local struct matching that documented layout and
30+
// casting to/from it.
2931
//
3032
// get_composite_option returns a vector sized exactly to that option's known wire size -
3133
// the caller has no generic way to know that size in advance, so the SDK owns the

src/ds/d500/d500-device.cpp

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,29 @@ constexpr bool hw_mon_over_xu = true;
4040
constexpr bool hw_mon_over_xu = false;
4141
#endif
4242

43+
namespace {
44+
45+
// PROTOTYPE / DEMO: internal-only wire layout for RS2_COMPOSITE_OPTION_HKR_TEMPORAL_FILTER_DPP,
46+
// used ONLY here to compute this control's wire size when registering it below. This is a
47+
// private implementation detail, NOT a public SDK type - the public API ships no struct for
48+
// composite options at all (see include/librealsense2/h/rs_composite_option.h, which
49+
// documents this exact layout as a comment on the enumerator). Callers (realsense-viewer,
50+
// the mock example) each independently define their own local struct matching that documented
51+
// layout - this one is not shared with them.
52+
#pragma pack( push, 1 )
53+
struct hkr_temporal_filter_dpp_wire_layout
54+
{
55+
int32_t enabled;
56+
float smooth_alpha;
57+
int32_t smooth_delta;
58+
int32_t persistency_index;
59+
};
60+
#pragma pack( pop )
61+
static_assert( sizeof( hkr_temporal_filter_dpp_wire_layout ) == 16,
62+
"must match the wire layout documented on RS2_COMPOSITE_OPTION_HKR_TEMPORAL_FILTER_DPP in rs_composite_option.h" );
63+
64+
} // namespace
65+
4366
namespace librealsense
4467
{
4568
std::map<uint32_t, rs2_format> d500_depth_fourcc_to_rs2_format = {
@@ -134,7 +157,7 @@ namespace librealsense
134157
// are fully generic and never change.
135158
_structured_controls.emplace(
136159
RS2_COMPOSITE_OPTION_HKR_TEMPORAL_FILTER_DPP,
137-
xu_structured_control( ds::depth_xu, ds::DS5_HKR_TEMPORAL_FILTER_DPP, sizeof( rs2_temporal_filter_dpp_config ) ) );
160+
xu_structured_control( ds::depth_xu, ds::DS5_HKR_TEMPORAL_FILTER_DPP, sizeof( hkr_temporal_filter_dpp_wire_layout ) ) );
138161
}
139162

140163
// PROTOTYPE / DEMO: generic composite-option dispatch - single atomic UVC transaction (one

src/ds/d500/d500-device.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
#include <src/embedded-filter-interface.h>
2424
#include <src/composite-option-interface.h>
2525
#include <src/ds/structured-xu-control.h>
26-
#include <librealsense2/h/rs_hkr_temporal_filter_dpp.h>
2726

2827
#include <map>
2928

0 commit comments

Comments
 (0)