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
rs-convert converts frames using sensor callback #6308
Conversation
aseelegbaria
commented
Apr 27, 2020
- the tool now names frames based on frame timestamp (instead of frame number)
- the tool creates metadata text file for the frame
- the tool converts frames by running over sensors and using sensor callback (instead of wait_for_frames), for ply converter it still uses wait_for_frames since we need synced frames
if (frames_map_get_and_set(frame.get_profile().stream_type(), frame.get_frame_number())) { | ||
continue; | ||
} | ||
if (depthframe && (_streamType == rs2_stream::RS2_STREAM_ANY || depthframe.get_profile().stream_type() == _streamType)) { |
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.
Can we do this outside the worker? So the thread won't even start of not the right frame/stream?
if (fs) { | ||
uint8_t buffer[4]; | ||
add_sub_worker( | ||
[filenameS, depthframe] { |
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.
Indentation seems off here -- please check
} | ||
} | ||
}); | ||
std::stringstream metadata_file; |
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.
Why is the frame written from a sub_worker but the metadata from here?
for (size_t i = 0; i < frameset.size(); i++) { | ||
auto frame = frameset[i].as<rs2::depth_frame>(); | ||
[this, &frame] { | ||
auto depthframe = frame.as<rs2::depth_frame>(); |
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.
Check indent
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.
(Same comments here and the other convert hpp files as above)
tools/convert/rs-convert.cpp
Outdated
cout << posP << "%" << "\r" << flush; | ||
} | ||
|
||
auto plyconverter = make_shared<rs2::tools::converter::converter_ply>( |
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.
No need to allocate anything until we check that filename is set
|
||
rs2::frameset frameset; | ||
auto frameNumber = 0ULL; | ||
std::mutex mutex; |
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.
Why is the mutex needed?
tools/convert/rs-convert.cpp
Outdated
playback.set_real_time(false); | ||
std::vector<rs2::sensor> sensors = playback.query_sensors(); | ||
|
||
rs2::frameset frameset; |
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.
Don't need 'frameset'
tools/convert/converter.hpp
Outdated
// Record all the available metadata attributes | ||
for (size_t i = 0; i < RS2_FRAME_METADATA_COUNT; i++) | ||
{ | ||
if (frm.supports_frame_metadata((rs2_frame_metadata_value)i)) |
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.
Maybe make i of type rs2_frame_metadata_value?