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.

Sign up for GitHub

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

Jump to bottom

Updated frame validator #4788

Merged
merged 2 commits into from
Sep 3, 2019
Merged

Updated frame validator #4788

merged 2 commits into from
Sep 3, 2019

Conversation

aangerma
Copy link
Contributor

@aangerma aangerma commented Sep 2, 2019 •
edited
Loading

Tracked on RS5-5244

ev-mp
ev-mp reviewed Sep 2, 2019
View reviewed changes
Copy link
Collaborator

@ev-mp ev-mp left a comment

Choose a reason for hiding this comment

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

Several remarks
The uneasy point is regarding the reset pipe flow.

src/frame-validator.cpp Outdated
{
_user_callback->on_frame(f);
_sensor->set_frames_callback(_user_callback);
if(is_user_requseted_frame((frame_interface*)f))
Copy link
Collaborator

Choose a reason for hiding this comment

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

requested

src/frame-validator.cpp Outdated
return false;

//start to validate only from the second frame
if(frame_num++ < 2)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pls rename to _frame_num to differentiate from local vars

src/frame-validator.cpp
//all streams except ir will be droped untill the validation on ir,
//after the validation all streams will be passeded to user callback directly
if (stream->get_stream_type() != RS2_STREAM_INFRARED)
return false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

If for some reason Ir frames do not arrive for a considerable amount of time but others do - is it possible to generate a log, like after 100 consecutive "return false" ?

src/frame-validator.cpp
{
for (int x = x_begin; x < x_end; x++)
if (((byte*)data)[y] == 0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is IR stream 8-bit only, no 12 bit formats?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes 8 bit

src/frame-validator.cpp Outdated
else
{
LOG_ERROR("frame_source received an empty depth frame, restarting the sensor...");

LOG_ERROR("frame_source received corrupted depth frame ("<<(float)invalid_pixels/(w*h)<<"% invalid pixels), restarting the sensor...");
Copy link
Collaborator

Choose a reason for hiding this comment

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

corrupted IR frame

src/frame-validator.cpp
{
try {
std::this_thread::sleep_for(std::chrono::milliseconds(500));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add a comment for maintainers

src/frame-validator.cpp
s->start(uc);
}
catch (...) {}
catch (...)
Copy link
Collaborator

Choose a reason for hiding this comment

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

If an exception is raised during the stop->close->open->start then the system state is not defined.
the thread's shall complete with either streaming or a full stop. In both cases a log shall be provided

src/frame-validator.h

return l->get_framerate() == r->get_framerate() &&
vl->get_width() == vr->get_width() &&
vl->get_height() == vr->get_height();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Stream format?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I try to find IR stream that corresponding to depth/confidence if the user asked depth/confidence and not ir ,so the format will not be the same only resolution and frame rate.

src/l500/l500-depth.cpp Outdated
return sp->get_stream_type() == RS2_STREAM_INFRARED && frame_validator::is_stream_profiles_equals(sp.get(), user_request_profile);
});

_validator_requests.push_back(*coresponding_ir);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add a guard for coresponding_ir==sp.end()

src/l500/l500-depth.cpp Outdated

auto sp = uvc_sensor::get_stream_profiles();

auto coresponding_ir = std::find_if(sp.begin(), sp.end(), [&](std::shared_ptr<stream_profile_interface> sp)
Copy link
Collaborator

Choose a reason for hiding this comment

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

corresponding_ir

@aangerma aangerma changed the title Updated frame validator Updated frame validator ( RS5-5244 ) Sep 3, 2019
@aangerma aangerma changed the title Updated frame validator ( RS5-5244 ) Updated frame validator Sep 3, 2019
@ev-mp ev-mp merged commit 04fb73f into IntelRealSense:development Sep 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ev-mp ev-mp ev-mp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aangerma @ev-mp