change metadata_blob to fit ALL metadata; software_sensor now uses array indexing #11551
Conversation
| RS2_FRAME_METADATA_HW_TYPE = RS2_FRAME_METADATA_COUNT +1 , /**< 8-bit Module type: RS4xx, IVCAM*/ | ||
| RS2_FRAME_METADATA_SKU_ID , /**< 8-bit SKU Id*/ | ||
| RS2_FRAME_METADATA_FORMAT , /**< 16-bit Frame format*/ | ||
| RS2_FRAME_METADATA_WIDTH , /**< 16-bit Frame width. pixels*/ |
There was a problem hiding this comment.
Not needed? d400-device uses this
There was a problem hiding this comment.
Ohh... I see you moved it to types.h
| @@ -57,7 +57,7 @@ struct frame_additional_data : frame_header | |||
| { | |||
| uint32_t metadata_size = 0; | |||
| bool fisheye_ae_mode = false; // TODO: remove in future release | |||
| std::array< uint8_t, MAX_META_DATA_SIZE > metadata_blob; | |||
| std::array< uint8_t, RS2_FRAME_METADATA_ACTUAL_COUNT * sizeof( rs2_metadata_type ) > metadata_blob; | |||
There was a problem hiding this comment.
Why not use an array of rs2_metadata_type?
There was a problem hiding this comment.
Because that's just one usage for software sensors. Most time it's binary data.
src/software-device.cpp
Outdated
| { | ||
| if( ! _metadata_parsers->count( key ) ) |
There was a problem hiding this comment.
I don't think we should test this. If setting same key again overwrite the value. This is also the current behavior.
There was a problem hiding this comment.
It's necessary. But I have a better way that will be faster, I think.
There was a problem hiding this comment.
Why is it necessary? If a user wants to override a value that was previously set he should be able to do so without clearing the whole table first
There was a problem hiding this comment.
It's necessary because now the only thing saying that the metadata exists is the presence of the parser. Why do you think it requires clearing the table first? The second time the parser would already exist and only the map is changed directly.
| throw invalid_value_exception( "invalid metadata key " + std::to_string( int( key ) ) ); | ||
| // At this time (and therefore for backwards compatibility) no register_metadata is required for SW sensors, | ||
| // and metadata persists between frames (!!!!!!!) unless clear_metadata() is used... | ||
| _metadata_parsers->emplace_hint( range.second, key, std::make_shared< md_array_parser >( key ) ); |
There was a problem hiding this comment.
For DDS devices make_shared will be called repeatedly for every metadata field on every frame.
We should allocate the parsers only once at start up and reference them on each relevant frame.
There was a problem hiding this comment.
This is development.
For DDS I already put in a erase_metadata function that would make only unset if a value isn't present, instead of the current clear_metadata.
For this PR, it only inserts a "parser" if it wasn't there before, so only the first time this function is called per metadata.
Tracked on [LRS-585]