Skip to content

Conversation

@radfordi
Copy link
Contributor

Avoid creating multiple unused rs2::contexts.

@radfordi radfordi requested review from dorodnic and ev-mp January 10, 2020 18:45
@radfordi
Copy link
Contributor Author

@dorodnic, this compiles now and passes the unit-tests tests locally. I don't think the one remaining CI failure is related to this PR.

}

ux_window::ux_window(const char* title) :
ux_window::ux_window(const char* title, context &ctx) :
Copy link
Collaborator

Choose a reason for hiding this comment

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

@radfordi , can you please provide the rationale for this PR ?
I don't think that it could a performance issue. And if not - then the PR may suggests that multiple context instances are somehow incompatible with a certain HW setup which in turn implies that there is an underlying issue that need to be resolved.
To this points there were no specific limitations on the number of ctx objects except for T265, so adding the constrain is at least a potential compatibility break.

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 ran into the multiple contexts when trying to test @bfulkers-i' hotplug PR (#5492). It's annoying/challenging to debug hotplug in the context in realsense-viewer because we keep creating/deleting contexts every 200ms! The (longer term) goal of that PR is to move all polling, checking and looping (with sleeps), to the OS level and to make the realsense-viewer (and tm_boot) to rely on the device changed notification instead.

It's not a large performance issue currently because the viewer is such a resource hog as it is, but this is small step toward making it less so and making the device detection use hotplug only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, T265's can't currently be detected in multiple contexts at the same time. This should be fixed, but in the mean time it's trivial to have a single context and avoid the issue.

@dorodnic dorodnic merged commit d188511 into IntelRealSense:development Feb 3, 2020
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