Skip to content

Added class md_attribute_parser_with_crc to dev #12424

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Dec 18, 2023
Merged

Added class md_attribute_parser_with_crc to dev #12424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
951138d
added class md_attribute_parser_with_crc
AviaAv Dec 17, 2023
653da4b
adding crc calculation to is_crc_valid
AviaAv Dec 17, 2023
dbe5432
added comment about function assumption
AviaAv Dec 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/ds/d400/d400-color.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ namespace librealsense
{
if (_separate_color)
{
auto md_prop_offset = offsetof(metadata_raw, mode) +
auto md_prop_offset = metadata_raw_mode_offset +
offsetof(md_rgb_mode, rgb_mode) +
offsetof(md_rgb_normal_mode, intel_rgb_control);

Expand All @@ -207,7 +207,7 @@ namespace librealsense
else
{
// attributes of md_rgb_control
auto md_prop_offset = offsetof(metadata_raw, mode) +
auto md_prop_offset = metadata_raw_mode_offset +
offsetof(md_rgb_mode, rgb_mode) +
offsetof(md_rgb_normal_mode, intel_rgb_control);

Expand Down
10 changes: 5 additions & 5 deletions src/ds/d400/d400-device.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -977,7 +977,7 @@ namespace librealsense
depth_sensor.register_metadata(RS2_FRAME_METADATA_FRAME_TIMESTAMP, make_uvc_header_parser(&platform::uvc_header::timestamp));

// attributes of md_capture_timing
auto md_prop_offset = offsetof(metadata_raw, mode) +
auto md_prop_offset = metadata_raw_mode_offset +
offsetof(md_depth_mode, depth_y_mode) +
offsetof(md_depth_y_normal_mode, intel_capture_timing);

Expand All @@ -986,14 +986,14 @@ namespace librealsense
make_attribute_parser(&md_capture_timing::sensor_timestamp, md_capture_timing_attributes::sensor_timestamp_attribute, md_prop_offset)));

// attributes of md_capture_stats
md_prop_offset = offsetof(metadata_raw, mode) +
md_prop_offset = metadata_raw_mode_offset +
offsetof(md_depth_mode, depth_y_mode) +
offsetof(md_depth_y_normal_mode, intel_capture_stats);

depth_sensor.register_metadata(RS2_FRAME_METADATA_WHITE_BALANCE, make_attribute_parser(&md_capture_stats::white_balance, md_capture_stat_attributes::white_balance_attribute, md_prop_offset));

// attributes of md_depth_control
md_prop_offset = offsetof(metadata_raw, mode) +
md_prop_offset = metadata_raw_mode_offset +
offsetof(md_depth_mode, depth_y_mode) +
offsetof(md_depth_y_normal_mode, intel_depth_control);

Expand All @@ -1014,7 +1014,7 @@ namespace librealsense
depth_sensor.register_metadata(RS2_FRAME_METADATA_FRAME_LED_POWER, make_attribute_parser(&md_depth_control::ledPower, md_depth_control_attributes::led_power_attribute, md_prop_offset));

// md_configuration - will be used for internal validation only
md_prop_offset = offsetof(metadata_raw, mode) + offsetof(md_depth_mode, depth_y_mode) + offsetof(md_depth_y_normal_mode, intel_configuration);
md_prop_offset = metadata_raw_mode_offset + offsetof(md_depth_mode, depth_y_mode) + offsetof(md_depth_y_normal_mode, intel_configuration);

depth_sensor.register_metadata((rs2_frame_metadata_value)RS2_FRAME_METADATA_HW_TYPE, make_attribute_parser(&md_configuration::hw_type, md_configuration_attributes::hw_type_attribute, md_prop_offset));
depth_sensor.register_metadata((rs2_frame_metadata_value)RS2_FRAME_METADATA_SKU_ID, make_attribute_parser(&md_configuration::sku_id, md_configuration_attributes::sku_id_attribute, md_prop_offset));
Expand All @@ -1031,7 +1031,7 @@ namespace librealsense
if (_fw_version >= hdr_firmware_version)
{
// attributes of md_capture_timing
auto md_prop_offset = offsetof(metadata_raw, mode) + offsetof(md_depth_mode, depth_y_mode) + offsetof(md_depth_y_normal_mode, intel_configuration);
auto md_prop_offset = metadata_raw_mode_offset + offsetof(md_depth_mode, depth_y_mode) + offsetof(md_depth_y_normal_mode, intel_configuration);

depth_sensor.register_metadata(RS2_FRAME_METADATA_SEQUENCE_SIZE,
make_attribute_parser(&md_configuration::sub_preset_info,
Expand Down
2 changes: 1 addition & 1 deletion src/ds/d500/d500-color.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ namespace librealsense
{
auto& color_ep = get_color_sensor();

auto md_prop_offset = offsetof(metadata_raw, mode) +
auto md_prop_offset = metadata_raw_mode_offset +
offsetof(md_rgb_mode, rgb_mode) +
offsetof(md_rgb_normal_mode, intel_rgb_control);

Expand Down
10 changes: 5 additions & 5 deletions src/ds/d500/d500-device.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,7 @@ namespace librealsense
}); //group_multiple_fw_calls

// attributes of md_capture_timing
auto md_prop_offset = offsetof(metadata_raw, mode) +
auto md_prop_offset = metadata_raw_mode_offset +
offsetof(md_depth_mode, depth_y_mode) +
offsetof(md_depth_y_normal_mode, intel_capture_timing);

Expand All @@ -634,14 +634,14 @@ namespace librealsense
make_attribute_parser(&md_capture_timing::sensor_timestamp, md_capture_timing_attributes::sensor_timestamp_attribute, md_prop_offset)));

// attributes of md_capture_stats
md_prop_offset = offsetof(metadata_raw, mode) +
md_prop_offset = metadata_raw_mode_offset +
offsetof(md_depth_mode, depth_y_mode) +
offsetof(md_depth_y_normal_mode, intel_capture_stats);

depth_sensor.register_metadata(RS2_FRAME_METADATA_WHITE_BALANCE, make_attribute_parser(&md_capture_stats::white_balance, md_capture_stat_attributes::white_balance_attribute, md_prop_offset));

// attributes of md_depth_control
md_prop_offset = offsetof(metadata_raw, mode) +
md_prop_offset = metadata_raw_mode_offset +
offsetof(md_depth_mode, depth_y_mode) +
offsetof(md_depth_y_normal_mode, intel_depth_control);

Expand All @@ -661,7 +661,7 @@ namespace librealsense
depth_sensor.register_metadata(RS2_FRAME_METADATA_FRAME_LED_POWER, make_attribute_parser(&md_depth_control::ledPower, md_depth_control_attributes::led_power_attribute, md_prop_offset));

// md_configuration - will be used for internal validation only
md_prop_offset = offsetof(metadata_raw, mode) + offsetof(md_depth_mode, depth_y_mode) + offsetof(md_depth_y_normal_mode, intel_configuration);
md_prop_offset = metadata_raw_mode_offset + offsetof(md_depth_mode, depth_y_mode) + offsetof(md_depth_y_normal_mode, intel_configuration);

depth_sensor.register_metadata((rs2_frame_metadata_value)RS2_FRAME_METADATA_HW_TYPE, make_attribute_parser(&md_configuration::hw_type, md_configuration_attributes::hw_type_attribute, md_prop_offset));
depth_sensor.register_metadata((rs2_frame_metadata_value)RS2_FRAME_METADATA_SKU_ID, make_attribute_parser(&md_configuration::sku_id, md_configuration_attributes::sku_id_attribute, md_prop_offset));
Expand All @@ -673,7 +673,7 @@ namespace librealsense
depth_sensor.register_metadata(RS2_FRAME_METADATA_GPIO_INPUT_DATA, make_attribute_parser(&md_configuration::gpioInputData, md_configuration_attributes::gpio_input_data_attribute, md_prop_offset));

// attributes of md_capture_timing
md_prop_offset = offsetof(metadata_raw, mode) + offsetof(md_depth_mode, depth_y_mode) + offsetof(md_depth_y_normal_mode, intel_configuration);
md_prop_offset = metadata_raw_mode_offset + offsetof(md_depth_mode, depth_y_mode) + offsetof(md_depth_y_normal_mode, intel_configuration);

depth_sensor.register_metadata(RS2_FRAME_METADATA_SEQUENCE_SIZE,
make_attribute_parser(&md_configuration::sub_preset_info,
Expand Down
8 changes: 4 additions & 4 deletions src/ds/ds-color-common.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ namespace librealsense
_color_ep.register_metadata(RS2_FRAME_METADATA_ACTUAL_FPS, std::make_shared<ds_md_attribute_actual_fps>());

// attributes of md_capture_timing
auto md_prop_offset = offsetof(metadata_raw, mode) +
auto md_prop_offset = metadata_raw_mode_offset +
offsetof(md_rgb_mode, rgb_mode) +
offsetof(md_rgb_normal_mode, intel_capture_timing);

Expand All @@ -83,22 +83,22 @@ namespace librealsense
make_attribute_parser(&md_capture_timing::sensor_timestamp, md_capture_timing_attributes::sensor_timestamp_attribute, md_prop_offset)));

// attributes of md_rgb_control
md_prop_offset = offsetof(metadata_raw, mode) +
md_prop_offset = metadata_raw_mode_offset +
offsetof(md_rgb_mode, rgb_mode) +
offsetof(md_rgb_normal_mode, intel_rgb_control);

_color_ep.register_metadata(RS2_FRAME_METADATA_GAIN_LEVEL, make_attribute_parser(&md_rgb_control::gain, md_rgb_control_attributes::gain_attribute, md_prop_offset));
_color_ep.register_metadata(RS2_FRAME_METADATA_ACTUAL_EXPOSURE, make_attribute_parser(&md_rgb_control::manual_exp, md_rgb_control_attributes::manual_exp_attribute, md_prop_offset));

// attributes of md_capture_stats
md_prop_offset = offsetof(metadata_raw, mode) +
md_prop_offset = metadata_raw_mode_offset +
offsetof(md_rgb_mode, rgb_mode) +
offsetof(md_rgb_normal_mode, intel_capture_stats);

_color_ep.register_metadata(RS2_FRAME_METADATA_WHITE_BALANCE, make_attribute_parser(&md_capture_stats::white_balance, md_capture_stat_attributes::white_balance_attribute, md_prop_offset));

// attributes of md_rgb_control
md_prop_offset = offsetof(metadata_raw, mode) +
md_prop_offset = metadata_raw_mode_offset +
offsetof(md_rgb_mode, rgb_mode) +
offsetof(md_rgb_normal_mode, intel_rgb_control);

Expand Down
8 changes: 4 additions & 4 deletions src/ds/ds-motion-common.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ namespace librealsense
_raw_fisheye_ep->register_metadata(RS2_FRAME_METADATA_AUTO_EXPOSURE, make_additional_data_parser(&frame_additional_data::fisheye_ae_mode));

// attributes of md_capture_timing
auto md_prop_offset = offsetof(metadata_raw, mode) +
auto md_prop_offset = metadata_raw_mode_offset +
offsetof(md_fisheye_mode, fisheye_mode) +
offsetof(md_fisheye_normal_mode, intel_capture_timing);

Expand All @@ -419,12 +419,12 @@ namespace librealsense
make_attribute_parser(&md_capture_timing::sensor_timestamp, md_capture_timing_attributes::sensor_timestamp_attribute, md_prop_offset)));

// attributes of md_capture_stats
md_prop_offset = offsetof(metadata_raw, mode) +
md_prop_offset = metadata_raw_mode_offset +
offsetof(md_fisheye_mode, fisheye_mode) +
offsetof(md_fisheye_normal_mode, intel_capture_stats);

// attributes of md_capture_stats
md_prop_offset = offsetof(metadata_raw, mode) +
md_prop_offset = metadata_raw_mode_offset +
offsetof(md_fisheye_mode, fisheye_mode) +
offsetof(md_fisheye_normal_mode, intel_configuration);

Expand All @@ -435,7 +435,7 @@ namespace librealsense
_raw_fisheye_ep->register_metadata((rs2_frame_metadata_value)RS2_FRAME_METADATA_HEIGHT, make_attribute_parser(&md_configuration::height, md_configuration_attributes::height_attribute, md_prop_offset));

// attributes of md_fisheye_control
md_prop_offset = offsetof(metadata_raw, mode) +
md_prop_offset = metadata_raw_mode_offset +
offsetof(md_fisheye_mode, fisheye_mode) +
offsetof(md_fisheye_normal_mode, intel_fisheye_control);

Expand Down
50 changes: 49 additions & 1 deletion src/metadata-parser.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@

#include "frame.h"
#include "metadata.h"
#include <cmath>

#include <cmath>
#include <rsutils/number/crc32.h>
#include <rsutils/string/from.h>


Expand Down Expand Up @@ -321,6 +322,53 @@ namespace librealsense
}
};

template<class S, class Attribute, typename Flag>
class md_attribute_parser_with_crc : public md_attribute_parser<S, Attribute, Flag>
{
public:
md_attribute_parser_with_crc(Attribute S::* attribute_name, Flag flag, unsigned long long offset, attrib_modifyer mod, unsigned long long crc_offset)
: md_attribute_parser<S, Attribute, Flag>(attribute_name, flag, offset, mod), _crc_offset(crc_offset) {}

protected:
unsigned long long _crc_offset; // offset of the crc inside the S class
bool is_attribute_valid(const S* s) const override
{
if (!md_attribute_parser<S, Attribute, Flag>::is_attribute_valid(s))
return false;

if (!is_crc_valid(s))
{
LOG_DEBUG("Metadata CRC mismatch");
Copy link
Collaborator

@Nir-Az Nir-Az Dec 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'LOG_WARNING?'

Copy link
Contributor Author

@AviaAv AviaAv Dec 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the way it was originally, and it is also LOG_DEBUG in the parent class in is_attribute_valid function, when we find "metadata mismatch"

return false;
}

return true;
}

private:
md_attribute_parser_with_crc() = delete;
md_attribute_parser_with_crc(const md_attribute_parser_with_crc&) = delete;

bool is_crc_valid(const S* md_info) const
{
Attribute crc = *(Attribute*)(reinterpret_cast<const uint8_t*>(md_info) + _crc_offset); //Attribute = CRC's type (ex. uint32_t)
auto computed_crc32 = rsutils::number::calc_crc32(reinterpret_cast<const uint8_t*>(md_info),
sizeof(S) - sizeof(crc));
Copy link
Collaborator

@Nir-Az Nir-Az Dec 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this for?
sizeof(S) - sizeof(crc));?

Copy link
Contributor Author

@AviaAv AviaAv Dec 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

S is the struct that have all of the attributes, including the crc.
So we pass the size of all other attributes as a buffer for calc_crc32

return (crc == computed_crc32);
}
};

/**\brief A helper function to create a specialized attribute parser.
* **Note that this class is assuming that the CRC is the last variable in the struct**
* Return it as a pointer to a base-class*/
template<class S, class Attribute, typename Flag>
std::shared_ptr<md_attribute_parser_base> make_attribute_parser_with_crc(Attribute S::* attribute, Flag flag, unsigned long long offset, unsigned long long crc_offset, attrib_modifyer mod = nullptr)
{
std::shared_ptr<md_attribute_parser<S, Attribute, Flag>> parser(new md_attribute_parser_with_crc<S, Attribute, Flag>(attribute, flag, offset, mod, crc_offset));
return parser;
}



class ds_md_attribute_actual_fps : public md_attribute_parser_base
{
Expand Down
1 change: 1 addition & 0 deletions src/metadata.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,7 @@ namespace librealsense
platform::uvc_header header;
md_modes mode;
};
constexpr int metadata_raw_mode_offset = sizeof(metadata_raw::header);

/**\brief metadata_mipi_raw - metadata structure
* layout as transmitted and received by backend */
Expand Down