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

Support for selecting stream_type specific profile #3019

Closed

Conversation

Arun-Prasad-V
Copy link
Contributor

Tracked by RSDEV-1745

rs2::stream_profile VideoProfilesManager::validateAndGetSuitableProfile(rs2_stream stream_type, rs2::stream_profile given_profile)
{
if (_all_profiles.empty())
throw std::runtime_error("Wrong commands sequence. No profiles set.");
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you elaborate on this situation? can you please give an example ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This error check was already there in getDefaultProfiles() function. Will remove it as it is not required here.

}

// If given profile is not suitable, choose the first available profile for the given stream.
if (!is_given_profile_suitable)
Copy link
Collaborator

Choose a reason for hiding this comment

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

and also here, can you give me an example ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Depth Mapping Camera module doesn't have any default profiles (for both occupancy and labeled pointcloud streams). So, getDefaultProfiles() function will consider the first listed profile as default profile. Due to this reason, lpc stream's profile will be considered as default profile. But that profile is not supported by Occupancy stream.

So here, for Occupancy stream, the given profile (lpc's profile) is not suitable. If not suitable, consider the first detected Occupancy stream profile.

Let's apply same scenario for Depth module. Depth stream has default profile, but IR streams doesn't. Fortunately, depth stream's default profile is suitable for IR streams. No issues for this scenario.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This case can be handled either this way or inside getDefaultProfiles() function itself by providing one default profile per stream.

@SamerKhshiboun
Copy link
Collaborator

Looks good. Only answer my two questions please.
We need to decide if we take this change to development, and when, because it breaks old API.

@Arun-Prasad-V Arun-Prasad-V marked this pull request as ready for review March 4, 2024 06:47
@Arun-Prasad-V
Copy link
Contributor Author

This feature is now available in ros2-development branch. Reference PR #3052. It will be taken to ros2-hkr branch while merging.

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.

2 participants