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

fix bug in Viewer syncer loop #9222

Merged
merged 1 commit into from Jun 15, 2021
Merged

Conversation

maloel
Copy link
Collaborator

@maloel maloel commented Jun 14, 2021

Found by Nir during sanity checks:

Switching to IR in 3D texture source froze the view.
The render_loop() depends only on synchronization_enable but sometimes we bypass it and so it cannot get the frames...

Tracked on [LRS-116]

@maloel maloel requested review from ev-mp, aangerma and Nir-Az June 14, 2021 16:36
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.

It must be correct, indeed.
Additionally, on transitioning to/from syncer_on mode it makes sense to flush queues and release internal resources

@@ -2114,7 +2114,8 @@ namespace rs2
try {
s->start([&, syncer](frame f)
{
if (viewer.synchronization_enable && is_synchronized_frame(viewer, f))
// The condition here must match the condition inside render_loop()!
if( viewer.synchronization_enable )
Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand the fix but I don't understand the regression cause, do you?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I got here by debugging it. I noticed that we get IR frames that are somehow not answering the is_synchronized_frame() call (the return is false). Following the logic with Evgeni's help, we saw that the render_loop() checks the syncer but we didn't push to the syncer...

So we have someone pushing to a frame queue on which we do not wait, instead waiting on a syncer that gets no frames. And we know that a syncer that does not get any input has no output! So we just wait. And nothing happens.

My guess is that the concurrency work has simply made this come out, whereas before something didn't work. Or maybe it's the other way around... but this fix definitely makes more sense, and the problem goes away with it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@maloel maloel merged commit 716d310 into IntelRealSense:development Jun 15, 2021
@maloel maloel deleted the syncerloop branch June 15, 2021 08:37
maloel added a commit that referenced this pull request Jun 15, 2021
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.

None yet

4 participants