Skip to content
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

refactor & optimize frame creation, invocation, metadata syncing #11615

Merged
merged 8 commits into from
Mar 26, 2023

Conversation

maloel
Copy link
Collaborator

@maloel maloel commented Mar 26, 2023

  • formalized metadata_array type with proper initialization and ctor in frame_additional_data
  • additional move semantics inside device-server, per last CR
  • frame_metadata_syncer no longer a template
  • removed the profile-storage need in the syncer
  • removed the need to allocate another pixel vector, instead using frame::data
  • exception catching around on_metadata_available
  • frame_holder usage for proper lifecycle

There 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.

@maloel maloel requested a review from OhadMeir March 26, 2023 07:29
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" ) ),
Copy link
Contributor

Choose a reason for hiding this comment

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

Might throw

Copy link
Collaborator Author

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

std::mutex _queues_lock;
};


class dds_sensor_proxy : public software_sensor
{
std::map< std::string, frame_metadata_syncer > _stream_name_to_syncer;
Copy link
Contributor

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

Copy link
Collaborator Author

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;
Copy link
Contributor

Choose a reason for hiding this comment

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

stoi might throw

Copy link
Collaborator Author

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

Copy link
Contributor

@OhadMeir OhadMeir left a comment

Choose a reason for hiding this comment

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

LGTM

@maloel maloel merged commit dafb8d1 into IntelRealSense:dds Mar 26, 2023
@maloel maloel deleted the dds branch March 26, 2023 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants