-
Notifications
You must be signed in to change notification settings - Fork 4.9k
refactor & optimize frame creation, invocation, metadata syncing #11615
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.
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
Conversation
| auto it = _stream_name_to_syncer.find( stream_name ); | ||
| if( it != _stream_name_to_syncer.end() ) | ||
| it->second.enqueue_metadata( | ||
| std::stoull( rsutils::json::get< std::string >( dds_md["header"], "frame-id" ) ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might throw
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's OK here, it'll be caught and report it
|
|
||
| class dds_sensor_proxy : public software_sensor | ||
| { | ||
| std::map< std::string, frame_metadata_syncer > _stream_name_to_syncer; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep it with the rest of the private members
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
src/context.cpp
Outdated
| data.timestamp; // from metadata | ||
| data.timestamp_domain; // from metadata | ||
| data.depth_units; // from metadata | ||
| data.frame_number = ! dds_frame.frame_id.empty() ? std::stoi( dds_frame.frame_id ) : 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stoi might throw
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed both (video/motion) to set to 0 if invalid
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
metadata_arraytype with proper initialization and ctor inframe_additional_dataframe_metadata_syncerno longer a templateframe::dataon_metadata_availableframe_holderusage for proper lifecycleThere may be a use-case where the timestamp+domain aren't set at all if no metadata is available. I'll take care of this in the next PR.
Also next, I'll be moving the syncer out of
context.cpp(into realdds), generating unit-tests for it, and finish the metadata unit-tests. Then will be changing things to be timestamp-based.