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

Python wrapper - Explicit config enable_stream(...) to have unique arg count (API CHANGE) #11861

Merged
merged 4 commits into from
Jun 15, 2023
Merged

Python wrapper - Explicit config enable_stream(...) to have unique arg count (API CHANGE) #11861

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b01450e
string enable_stream unique arg count
Nir-Az May 31, 2023
825db53
align example and comment
Nir-Az Jun 1, 2023
f5cf3d3
add single param wrapper function and restore the example usage
Nir-Az Jun 14, 2023
691ad31
keep def arg at C++ call
Nir-Az Jun 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions wrappers/python/pyrs_pipeline.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ void init_pipeline(py::module &m) {
"It also allows the user to find a matching device for the config filters and the pipeline, in order to select a device explicitly, "
"and modify its controls before streaming starts.");
config.def(py::init<>())
// NOTE: do not overload functions with default arg, python cannot distinguish between enums
// We specifically only allow "enable_stream" functions with unique arguments count (LibRS SDK allow overload as it's C/C++ types)
.def("enable_stream", (void (rs2::config::*)(rs2_stream, int, int, int, rs2_format, int)) &rs2::config::enable_stream, "Enable a device stream explicitly, with selected stream parameters.\n"
"The method allows the application to request a stream with specific configuration.\n"
"If no stream is explicitly enabled, the pipeline configures the device and its streams according to the attached computer vision modules and processing blocks "
Expand All @@ -45,11 +47,14 @@ void init_pipeline(py::module &m) {
"Multiple enable stream calls for the same stream override each other, and the last call is maintained.\n"
"Upon calling resolve(), the config checks for conflicts between the application configuration requests and the attached computer vision "
"modules and processing blocks requirements, and fails if conflicts are found.\n"
"Before resolve() is called, no conflict check is done.", "stream_type"_a, "stream_index"_a, "width"_a, "height"_a, "format"_a = RS2_FORMAT_ANY, "framerate"_a = 0)
.def("enable_stream", (void (rs2::config::*)(rs2_stream, int)) &rs2::config::enable_stream, "Stream type and possibly also stream index. Other parameters are resolved internally.", "stream_type"_a, "stream_index"_a = -1)
.def("enable_stream", (void (rs2::config::*)(rs2_stream, rs2_format, int))&rs2::config::enable_stream, "Stream type and format, and possibly frame rate. Other parameters are resolved internally.", "stream_type"_a, "format"_a, "framerate"_a = 0)
.def("enable_stream", (void (rs2::config::*)(rs2_stream, int, int, rs2_format, int)) &rs2::config::enable_stream, "Stream type and resolution, and possibly format and frame rate. Other parameters are resolved internally.", "stream_type"_a, "width"_a, "height"_a, "format"_a = RS2_FORMAT_ANY, "framerate"_a = 0)
.def("enable_stream", (void (rs2::config::*)(rs2_stream, int, rs2_format, int)) &rs2::config::enable_stream, "Stream type, index, and format, and possibly framerate. Other parameters are resolved internally.", "stream_type"_a, "stream_index"_a, "format"_a, "framerate"_a = 0)
"Before resolve() is called, no conflict check is done.", "stream_type"_a, "stream_index"_a, "width"_a, "height"_a, "format"_a, "framerate"_a)


.def("enable_stream", []( rs2::config* c, rs2_stream s ) -> void { return c->enable_stream( s, -1 ); }, "Stream type only. Other parameters are resolved internally.", "stream_type"_a )
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the explicit -1 instead of using the C++ enable_stream that takes only 1 arg? (i.e., it has a default second arg)

Copy link
Collaborator

@maloel maloel Jun 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Or it can be:

.def("enable_stream", (void (rs2::config::*)(rs2_stream)) &rs2::config::enable_stream, ...)

No? Not sure it'll work -- but if not, then just remove the explicit -1

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It can be yes, I wanted to show that I explicitly call this command arguments but without it it will also work the same.
Do you think it's preferred?

Copy link
Collaborator

@maloel maloel Jun 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The only thing I don't like is the explicit -1, because it presumes to know what the default is. Up to you how to do it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed

.def("enable_stream", (void (rs2::config::*)(rs2_stream, int)) &rs2::config::enable_stream, "Stream type and possibly also stream index. Other parameters are resolved internally.", "stream_type"_a, "stream_index"_a)
.def("enable_stream", (void (rs2::config::*)(rs2_stream, rs2_format, int))&rs2::config::enable_stream, "Stream type and format, and possibly frame rate. Other parameters are resolved internally.", "stream_type"_a, "format"_a, "framerate"_a)
.def("enable_stream", (void (rs2::config::*)(rs2_stream, int, int, rs2_format, int)) &rs2::config::enable_stream, "Stream type and resolution, and possibly format and frame rate. Other parameters are resolved internally.", "stream_type"_a, "width"_a, "height"_a, "format"_a, "framerate"_a)
.def("enable_stream", (void (rs2::config::*)(rs2_stream, int, rs2_format, int)) &rs2::config::enable_stream, "Stream type, index, and format, and possibly framerate. Other parameters are resolved internally.", "stream_type"_a, "stream_index"_a, "format"_a, "framerate"_a)
.def("enable_all_streams", &rs2::config::enable_all_streams, "Enable all device streams explicitly.\n"
"The conditions and behavior of this method are similar to those of enable_stream().\n"
"This filter enables all raw streams of the selected device. The device is either selected explicitly by the application, "
Expand Down