Skip to content

Add APIs to inject IMU and pose to software_device - #2673

Merged
dorodnic merged 5 commits into
realsenseai:developmentfrom
belkinirena:software-device
Nov 25, 2018
Merged

Add APIs to inject IMU and pose to software_device#2673
dorodnic merged 5 commits into
realsenseai:developmentfrom
belkinirena:software-device

Conversation

@belkinirena

@belkinirena belkinirena commented Nov 5, 2018

Copy link
Copy Markdown
Contributor

Added new API to software sensor:

Added support for IMU stream and recording IMU frames in software sensor:

  • stream_profile add_motion_stream(rs2_motion_stream motion_stream)
  • rs2_stream_profile* rs2_software_sensor_add_motion_stream(rs2_sensor* sensor, rs2_motion_stream motion_stream, rs2_error** error);
  • void on_motion_frame(rs2_software_motion_frame frame)
  • void rs2_software_sensor_on_motion_frame(rs2_sensor* sensor, rs2_software_motion_frame frame, rs2_error** error);

Added support for pose stream and recording pose frames in software sensor:

  • stream_profile add_pose_stream(rs2_pose_stream pose_stream)
  • rs2_stream_profile* rs2_software_sensor_add_pose_stream(rs2_sensor* sensor, rs2_pose_stream pose_stream, rs2_error** error);
  • void on_pose_frame(rs2_software_pose_frame frame)
  • void rs2_software_sensor_on_pose_frame(rs2_sensor* sensor, rs2_software_pose_frame frame, rs2_error** error);

@belkinirena belkinirena changed the title Add APIs to record IMU and pose to software_device Add APIs to inject IMU and pose to software_device Nov 5, 2018
@dorodnic

dorodnic commented Nov 5, 2018

Copy link
Copy Markdown
Contributor

Related to #1880. While not directly solving the request of recording external IMU, software device + recorder lets user combine any list of sensors and record them into a bag file. This API adds the ability to record IMU as well.

ev-mp
ev-mp previously requested changes Nov 5, 2018

@ev-mp ev-mp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The PR is C++ only.
Can you verify it with Python wrapper?

Comment thread include/librealsense2/h/rs_internal.h Outdated
rs2_intrinsics intrinsics;
} rs2_video_stream;

/** \brief All the parameters are requaired to defind motion stream*/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

...that are required to define...

Comment thread CMake/realsense.def Outdated
rs2_create_software_device
rs2_software_device_add_sensor
rs2_software_sensor_on_video_frame
rs2_software_sensor_on_motion_frame

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can this be achieved via refactoring the rs_software_sensor/stream/frame into class hierarchy and unifying the invocation calls into rs_software_sensor_on_frame(sw_frame_base) , the same way as with live devices?
Though this would introduce API breaking change but also will establish infra for future extensions.
It will also will minimize API impact

Comment thread src/software-device.cpp Outdated

if (exist)
{
LOG_WARNING("Stream unique ID already exist!");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Motion stream

Comment thread src/software-device.cpp Outdated
{
auto exist = (std::find_if(_profiles.begin(), _profiles.end(), [&](std::shared_ptr<stream_profile_interface> profile)
{
if (profile->get_unique_id() == motion_stream.uid)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe "return profile->get_unique_id() == motion_stream.uid" for readability

Comment thread src/software-device.cpp Outdated

if (exist)
{
LOG_WARNING("Stream unique ID already exist!");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pose stream id...

Comment thread src/software-device.cpp
continue; //stop adding metadata to frame
}
memcpy(data.metadata_blob.data() + data.metadata_size, &i.first, size_of_enum);
data.metadata_size += static_cast<uint32_t>(size_of_enum);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In case of "sizeof(rs2_frame_metadata_value)>32bit" data will be overrun

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think it is safe to add values of any size, since the data.metadata_blob is a byte array,

Comment thread src/software-device.cpp
Comment thread src/software-device.cpp Outdated
{
return;
}
auto mid_profile = dynamic_cast<motion_stream_profile_interface*>(software_frame.profile->profile);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Declaration no used

Comment thread src/software-device.cpp Outdated
return;
}
auto mid_profile = dynamic_cast<motion_stream_profile_interface*>(software_frame.profile->profile);
auto mid_frame = dynamic_cast<motion_frame*>(frame);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here

Comment thread src/software-device.cpp
data.timestamp_domain = software_frame.domain;
data.frame_number = software_frame.frame_number;

data.metadata_size = 0;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Metadata initialization (Lines 281-296) looks identical with "on_motion_frame(...

@dorodnic

Copy link
Copy Markdown
Contributor

@belkinirena could you please update CMake/realsense.def

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.

3 participants