Skip to content

Commit 63bb293

Browse files
author
Evgeni Raikhel
committed
Change rs2_get_composite_option to SDK-allocated rs2_raw_data_buffer ownership model, matching rs2_get_safety_preset
The caller has no generic way to know a given rs2_composite_option_id's wire size in advance (only the SDK-side control for that id knows it), so requiring a pre-sized caller buffer was unsafe. rs2_get_composite_option now returns an SDK-heap-allocated rs2_raw_data_buffer, read via rs2_get_raw_data_size/rs2_get_raw_data and freed via rs2_delete_raw_data - mirroring rs2_get_safety_preset exactly. rs2::composite_option_sensor:: get_composite_option hides this behind a std::vector<uint8_t> return, same as rs2::safety_sensor::get_safety_preset. rs2_set_composite_option is unchanged: the caller/producer already knows sizeof() of what it sends. xu_structured_control::get_raw and composite_option_interface:: get_composite_option now return std::vector<uint8_t> sized to the control's known wire_size instead of writing into a caller-supplied void*/size pair. Updated the realsense-viewer panel and the fake-transport mock example to the new contract; both still verify round-trip correctness and the exactly-one-set_xu/one-get_xu atomicity guarantee.
1 parent 18371bc commit 63bb293

10 files changed

Lines changed: 88 additions & 41 deletions

File tree

common/device-model.cpp

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
#include <librealsense2/rs.hpp>
66
#include <rs-config.h>
77

8+
#include <cstring>
9+
810
#include <third-party/filesystem/glob.h>
911

1012
#include <imgui.h>
@@ -2789,9 +2791,12 @@ namespace rs2
27892791
{
27902792
if (!sub->temporal_filter_dpp_populated)
27912793
{
2794+
auto bytes = composite_sensor.get_composite_option(RS2_COMPOSITE_OPTION_HKR_TEMPORAL_FILTER_DPP);
2795+
if (bytes.size() != sizeof(rs2_temporal_filter_dpp_config))
2796+
throw std::runtime_error("HKR Temporal Filter DPP: unexpected payload size from get_composite_option");
2797+
27922798
rs2_temporal_filter_dpp_config cfg{};
2793-
unsigned int data_size = sizeof(cfg);
2794-
composite_sensor.get_composite_option(RS2_COMPOSITE_OPTION_HKR_TEMPORAL_FILTER_DPP, &cfg, &data_size);
2799+
memcpy(&cfg, bytes.data(), sizeof(cfg));
27952800
sub->temporal_filter_dpp_enabled = cfg.enabled;
27962801
sub->temporal_filter_dpp_smooth_alpha = cfg.smooth_alpha;
27972802
sub->temporal_filter_dpp_smooth_delta = cfg.smooth_delta;
@@ -2828,6 +2833,10 @@ namespace rs2
28282833
{
28292834
error_message = error_to_string(e);
28302835
}
2836+
catch (const std::exception& e)
2837+
{
2838+
error_message = e.what();
2839+
}
28312840

28322841
ImGui::TreePop();
28332842
}

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

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,12 @@
1111
// hardware test.
1212
//
1313
// What this proves:
14-
// 1. Round-trip correctness: a payload sent via set_raw() comes back byte-identical via
15-
// get_raw(), when cast back to the caller's typed struct (rs2_temporal_filter_dpp_config)
16-
// - exactly the caller-casts-the-void* contract of the real rs2_set/get_composite_option
17-
// API.
14+
// 1. Round-trip correctness: a payload sent via set_raw(void*, size) comes back
15+
// 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).
1820
// 2. Atomicity: set_raw() performs EXACTLY ONE set_xu() call, and get_raw() performs
1921
// EXACTLY ONE get_xu() call - i.e. the whole payload always travels as a single UVC
2022
// transaction, never as separate per-field writes/reads. This is the non-negotiable
@@ -120,11 +122,19 @@ try
120122
sent.smooth_delta = 20;
121123
sent.persistency_index = 3;
122124

123-
// Caller casts to/from void* - exactly the contract of rs2_set/get_composite_option.
125+
// Caller casts to/from void* - exactly the contract of rs2_set/get_composite_option. SET
126+
// still takes a caller-owned buffer (the caller/producer already knows sizeof() of what
127+
// it's sending). GET returns an SDK-owned, correctly-sized vector - the caller has no
128+
// generic way to know a given option_id's wire size in advance - mirroring
129+
// rs2_get_safety_preset/rs2::safety_sensor::get_safety_preset.
124130
control.set_raw( dev, &sent, sizeof( sent ) );
125131

132+
std::vector< uint8_t > bytes = control.get_raw( dev );
133+
if( bytes.size() != sizeof( rs2_temporal_filter_dpp_config ) )
134+
throw std::runtime_error( "get_raw returned an unexpected payload size" );
135+
126136
rs2_temporal_filter_dpp_config received{};
127-
control.get_raw( dev, &received, sizeof( received ) );
137+
std::memcpy( &received, bytes.data(), sizeof( received ) );
128138

129139
// 1) Round-trip correctness.
130140
bool round_trip_ok = ( received.enabled == sent.enabled )

include/librealsense2/h/rs_composite_option.h

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,15 +56,18 @@ void rs2_set_composite_option(const rs2_sensor* sensor, rs2_composite_option_id
5656
/**
5757
* rs2_get_composite_option - generic composite-option getter.
5858
* Reads the current value from the device in ONE atomic UVC control transaction (one get_xu
59-
* call) into the caller-provided buffer.
60-
* \param[in] sensor Sensor that exposes the requested composite option
61-
* \param[in] option_id Which composite option to read
62-
* \param[out] data Pointer to the caller's typed payload struct (written on success)
63-
* \param[in,out] data_size On input: size of the caller's buffer (must be >= the option's
64-
* wire size). On output: the number of bytes actually written.
65-
* \param[out] error If non-null, receives any error that occurs during this call, otherwise, errors are ignored
59+
* call). The caller has no generic way to know the wire size of an arbitrary option_id in
60+
* advance (only the SDK-side control for that specific id knows it), so the SDK heap-allocates
61+
* and returns the result as an rs2_raw_data_buffer - read its bytes with rs2_get_raw_data_size/
62+
* rs2_get_raw_data and free it with rs2_delete_raw_data when done (mirrors rs2_get_safety_preset).
63+
* \param[in] sensor Sensor that exposes the requested composite option
64+
* \param[in] option_id Which composite option to read
65+
* \param[out] error If non-null, receives any error that occurs during this call, otherwise, errors are ignored
66+
* \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.
6669
*/
67-
void rs2_get_composite_option(const rs2_sensor* sensor, rs2_composite_option_id option_id, void* data, unsigned int* data_size, rs2_error** error);
70+
const rs2_raw_data_buffer* rs2_get_composite_option(const rs2_sensor* sensor, rs2_composite_option_id option_id, rs2_error** error);
6871

6972
#ifdef __cplusplus
7073
}

include/librealsense2/hpp/rs_composite_option.hpp

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,29 @@ namespace rs2
4141
error::handle(e);
4242
}
4343

44-
// Single atomic UVC transaction (one get_xu round trip).
45-
void get_composite_option(rs2_composite_option_id option_id, void* data, unsigned int* data_size) const
44+
// Single atomic UVC transaction (one get_xu round trip). The SDK allocates the result
45+
// (the caller has no generic way to know option_id's wire size in advance) and this
46+
// 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).
49+
std::vector<uint8_t> get_composite_option(rs2_composite_option_id option_id) const
4650
{
4751
rs2_error* e = nullptr;
48-
rs2_get_composite_option(_sensor.get(), option_id, data, data_size, &e);
52+
auto buffer = rs2_get_composite_option(_sensor.get(), option_id, &e);
53+
54+
std::shared_ptr<const rs2_raw_data_buffer> list(buffer, rs2_delete_raw_data);
55+
error::handle(e);
56+
57+
auto size = rs2_get_raw_data_size(list.get(), &e);
4958
error::handle(e);
59+
60+
auto start = rs2_get_raw_data(list.get(), &e);
61+
error::handle(e);
62+
63+
std::vector<uint8_t> result;
64+
result.insert(result.begin(), start, start + size);
65+
66+
return result;
5067
}
5168
};
5269
}

src/composite-option-interface.h

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@
1212
#include "core/extension.h"
1313
#include <librealsense2/h/rs_composite_option.h>
1414

15+
#include <vector>
16+
#include <cstdint>
17+
1518
namespace librealsense {
1619

1720
class composite_option_interface
@@ -20,11 +23,15 @@ class composite_option_interface
2023
virtual ~composite_option_interface() = default;
2124

2225
// Generic dispatch by option id. Each call performs EXACTLY ONE UVC control transaction
23-
// (one set_xu/get_xu). data/data_size are an opaque byte blob whose layout is defined by
24-
// whichever typed struct corresponds to option_id (e.g. rs2_temporal_filter_dpp_config for
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
2528
// RS2_COMPOSITE_OPTION_HKR_TEMPORAL_FILTER_DPP) - the caller is responsible for casting.
29+
//
30+
// get_composite_option returns a vector sized exactly to that option's known wire size -
31+
// the caller has no generic way to know that size in advance, so the SDK owns the
32+
// allocation (mirrors librealsense::safety_sensor::get_safety_preset).
2633
virtual void set_composite_option( rs2_composite_option_id option_id, const void * data, uint32_t data_size ) = 0;
27-
virtual void get_composite_option( rs2_composite_option_id option_id, void * data, uint32_t * data_size ) const = 0;
34+
virtual std::vector< uint8_t > get_composite_option( rs2_composite_option_id option_id ) const = 0;
2835
};
2936

3037
MAP_EXTENSION( RS2_EXTENSION_COMPOSITE_OPTIONS, librealsense::composite_option_interface );

src/ds/d500/d500-device.cpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ namespace librealsense
155155
[&control, data, data_size]( platform::uvc_device & dev ) { control.set_raw( dev, data, data_size ); } );
156156
}
157157

158-
void d500_depth_sensor::get_composite_option( rs2_composite_option_id option_id, void * data, uint32_t * data_size ) const
158+
std::vector< uint8_t > d500_depth_sensor::get_composite_option( rs2_composite_option_id option_id ) const
159159
{
160160
auto it = _structured_controls.find( option_id );
161161
if( it == _structured_controls.end() )
@@ -167,10 +167,8 @@ namespace librealsense
167167
throw wrong_api_call_sequence_exception( "composite option is not available: no raw depth sensor" );
168168

169169
auto & control = it->second;
170-
uint32_t wire_size = control.wire_size();
171-
raw_depth_sensor->invoke_powered(
172-
[&control, data, data_size]( platform::uvc_device & dev ) { control.get_raw( dev, data, *data_size ); } );
173-
*data_size = wire_size;
170+
return raw_depth_sensor->invoke_powered(
171+
[&control]( platform::uvc_device & dev ) { return control.get_raw( dev ); } );
174172
}
175173

176174
processing_blocks d500_depth_sensor::get_recommended_processing_blocks() const

src/ds/d500/d500-device.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ namespace librealsense
7070
// xu_structured_control handles the call is selected by option_id at runtime via
7171
// _structured_controls - there is no per-feature method here, only registration.
7272
void set_composite_option( rs2_composite_option_id option_id, const void * data, uint32_t data_size ) override;
73-
void get_composite_option( rs2_composite_option_id option_id, void * data, uint32_t * data_size ) const override;
73+
std::vector< uint8_t > get_composite_option( rs2_composite_option_id option_id ) const override;
7474

7575
protected:
7676
d500_device * _owner;

src/ds/structured-xu-control.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,15 @@
88

99
namespace librealsense {
1010

11-
void xu_structured_control::get_raw( platform::uvc_device & dev, void * data, uint32_t data_size ) const
11+
std::vector< uint8_t > xu_structured_control::get_raw( platform::uvc_device & dev ) const
1212
{
13-
if( data_size < _wire_size )
14-
throw std::runtime_error( "xu_structured_control::get_raw: caller buffer smaller than the control's wire size" );
13+
std::vector< uint8_t > data( _wire_size );
1514

1615
// Exactly one get_xu() call - the whole payload arrives atomically.
17-
if( ! dev.get_xu( _xu, _ctrl_id, reinterpret_cast< uint8_t * >( data ), (int)_wire_size ) )
16+
if( ! dev.get_xu( _xu, _ctrl_id, data.data(), (int)_wire_size ) )
1817
throw std::runtime_error( "get_xu() failed" );
18+
19+
return data;
1920
}
2021

2122
void xu_structured_control::set_raw( platform::uvc_device & dev, const void * data, uint32_t data_size ) const

src/ds/structured-xu-control.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@
2626

2727
#include <src/platform/uvc-device.h>
2828

29+
#include <vector>
30+
#include <cstdint>
31+
2932
namespace librealsense {
3033

3134
class xu_structured_control
@@ -40,8 +43,10 @@ class xu_structured_control
4043

4144
uint32_t wire_size() const { return _wire_size; }
4245

43-
// Exactly one get_xu() call. Throws if data_size is smaller than this control's wire size.
44-
void get_raw( platform::uvc_device & dev, void * data, uint32_t data_size ) const;
46+
// Exactly one get_xu() call. Returns a vector sized exactly wire_size() - this class is
47+
// the one place that knows the control's wire size, so it owns the allocation rather than
48+
// requiring the caller to guess a buffer size upfront.
49+
std::vector< uint8_t > get_raw( platform::uvc_device & dev ) const;
4550

4651
// Exactly one set_xu() call - the whole wire_size() payload sent together. Throws if
4752
// data_size does not exactly match this control's wire size.

src/rs.cpp

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4851,21 +4851,18 @@ void rs2_set_composite_option(
48514851
}
48524852
HANDLE_EXCEPTIONS_AND_RETURN(, sensor, option_id, data, data_size)
48534853

4854-
void rs2_get_composite_option(
4854+
const rs2_raw_data_buffer* rs2_get_composite_option(
48554855
rs2_sensor const* sensor,
48564856
rs2_composite_option_id option_id,
4857-
void* data,
4858-
unsigned int* data_size,
48594857
rs2_error** error) BEGIN_API_CALL
48604858
{
48614859
VALIDATE_NOT_NULL(sensor);
48624860
VALIDATE_ENUM(option_id);
4863-
VALIDATE_NOT_NULL(data);
4864-
VALIDATE_NOT_NULL(data_size);
48654861
auto composite = VALIDATE_INTERFACE(sensor->sensor, librealsense::composite_option_interface);
4866-
composite->get_composite_option(option_id, data, data_size);
4862+
std::vector<uint8_t> vec = composite->get_composite_option(option_id);
4863+
return new rs2_raw_data_buffer{ std::move(vec) };
48674864
}
4868-
HANDLE_EXCEPTIONS_AND_RETURN(, sensor, option_id, data, data_size)
4865+
HANDLE_EXCEPTIONS_AND_RETURN(nullptr, sensor, option_id)
48694866

48704867
void rs2_hw_monitor_get_opcode_string(int opcode, char* buffer, size_t buffer_size,
48714868
rs2_device* device,

0 commit comments

Comments
 (0)