Refactor pipeline resolve #2006

matkatz · 2018-07-05T06:41:04Z

Tracked on: DSO-9472,
Following-up on #1543

dorodnic · 2018-07-11T18:44:33Z

It's finally green! 🎉

dorodnic · 2018-07-11T18:56:21Z

src/context.cpp

+        {
+            std::vector<tagged_profile> markers;
+            markers.push_back({ RS2_STREAM_COLOR, -1, 640, 480, RS2_FORMAT_RGB8, 30, profile_tag::PROFILE_TAG_SUPERSET | profile_tag::PROFILE_TAG_DEFAULT });
+            markers.push_back({ RS2_STREAM_DEPTH, -1, 640, 480, RS2_FORMAT_Z16, 30, profile_tag::PROFILE_TAG_SUPERSET | profile_tag::PROFILE_TAG_DEFAULT });


I would not expect depth and infrared to be available on a platform camera

it probably can also default to higher resolution... 1920x1080?

dorodnic · 2018-07-11T18:57:13Z

src/core/streaming.h

+    typedef enum profile_tag
+    {
+        PROFILE_TAG_ANY = 0,
+        PROFILE_TAG_SUPERSET = 1,


Consider adding comment:
PROFILE_TAG_SUPERSET // to be included in enable_all
PROFILE_TAG_DEFAULT // to be included in default pipeline start

dorodnic · 2018-07-11T18:59:46Z

src/ds5/ds5-device.cpp

+    platform::usb_spec ds5_device::get_usb_spec() const
+    {
+        if(!supports_info(RS2_CAMERA_INFO_USB_TYPE_DESCRIPTOR))
+            return platform::usb_undefined;


Why not USB3?

it is better to let the caller decide what to do in case usb type is unknown

dorodnic · 2018-07-11T19:01:57Z

src/ds5/ds5-factory.cpp

@@ -37,6 +37,25 @@ namespace librealsense
              ds5_advanced_mode_base(ds5_device::_hw_monitor, get_depth_sensor()) {}

        std::shared_ptr<matcher> create_matcher(const frame_holder& frame) const override;
+
+        std::vector<tagged_profile> get_profiles_tags() const override


Suggestion, to consider:
Profile tags are queries every time you call tag_profile, so the resulting vector will be created and re-created over and over again. This is only during start-up, but still.
Perhaps you can add another (non-virtual) getter that would fetch a lazy<std::vector<tagged_profile>> that would be initialized with lambda to get_profiles_tags?

dorodnic · 2018-07-11T19:03:38Z

src/ds5/ds5-motion.cpp

@@ -79,7 +79,7 @@ namespace librealsense
                    assign_stream(_owner->_gpio_streams[p->get_stream_index()-1], p);
                if (p->get_framerate() == 1000 &&
                    p->get_format() == RS2_FORMAT_MOTION_XYZ32F)
-                    p->make_default();
+                    get_device().tag_profile(p.get());


Is there a way to ensure that library writer will not forget to call tag_profile on one of the camera profiles?

actually this "if" should be removed and added is a default stream.
for both this comment and the one above I can init the profiles on the device creation, but I guess that I will need to record everything again.

dorodnic · 2018-07-11T19:05:10Z

src/mock/recorder.cpp

+                int min_version[ver_size];
+                std::sscanf(api_version.c_str(), "%d.%d.%d", &section_version[0], &section_version[1], &section_version[2]);
+                std::sscanf(min_api_version.c_str(), "%d.%d.%d", &min_version[0], &min_version[1], &min_version[2]);
+                for (int i = 0; i < ver_size; i++)


What if invalid string format will be passed?

throw exception in case of bad format
done

dorodnic · 2018-07-11T19:05:28Z

src/mock/recorder.cpp

@@ -403,6 +403,19 @@ namespace librealsense
                select_api_version.bind(1, section_id);
                select_api_version.bind(2, API_VERSION_KEY);
                auto api_version = select_api_version()[0].get_string();
+                const int ver_size = 3;
+                int section_version[ver_size];


Maybe initialize with zeros?

dorodnic · 2018-07-11T19:07:04Z

src/sensor.cpp

+        if (tag == profile_tag::PROFILE_TAG_ANY)
+            return *_profiles;
+
+        stream_profiles profiles;


Consider renaming to something like result. profiles is confusing with _profiles

dorodnic · 2018-07-11T19:08:47Z

unit-tests/unit-tests-live.cpp

+    {
+        auto it = std::find_if(begin(all_streams), end(all_streams), [d](const rs2::stream_profile profile)
+        {
+            return d == profile;


Wouldn't this work with regular find?

matkatz added 29 commits July 4, 2018 13:00

refactor pipeline resolve

377dcae

remove presets from pipeline config

3a4d4f1

pipeline live tests added

8c479c1

default playback config test added to pipeline tests

29e4858

add make_context to pipeline tests

5246c0a

pipeline wildcard streams test added

17dd9c3

revert some pipeline changes that break test recordings

ea3fc8d

fix sort_best_quality

6e2a697

remove default streams filter from pipeline

69d1db9

add profile marker

5c70699

rename marker enum

e95836f

remove make default

9e69831

resolve default configurations first

002d04e

override set_marker

93ae858

set default configurations for each device

799584f

remove USB type check

6a8c7b8

fix default streams

1491a95

rename marker to profile tag

4e1c0c1

fix profile tagging

261f8f4

fix pipeline tests

73b10be

add fisheye default stream in d430 mm

f1b3720

unit-tests - filter test by min version

a823877

versiond create mock method added

79e9fda

filter old records

3ecf649

add [using_pipeline] tag

73afbea

ignore mayfail tests

cbbf132

check if usb version is supported by the camera info

339743b

enable mayfail tests

ec67832

empty

82d026e

dorodnic suggested changes Jul 11, 2018

View reviewed changes

fix CR comments

fd28850

dorodnic approved these changes Jul 12, 2018

View reviewed changes

matkatz added 2 commits July 12, 2018 18:09

tag Pipeline start ignores previous config if it was changed as may fail

2d277e6

one more may fail

f9b9f5a

dorodnic merged commit e3021cc into IntelRealSense:development Jul 12, 2018

matkatz deleted the refactor_pipeline_resolve branch December 23, 2018 07:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor pipeline resolve #2006

Refactor pipeline resolve #2006

matkatz commented Jul 5, 2018 •

edited

Loading

dorodnic commented Jul 11, 2018

dorodnic Jul 11, 2018

matkatz Jul 12, 2018

dorodnic Jul 11, 2018

matkatz Jul 12, 2018

dorodnic Jul 11, 2018

matkatz Jul 12, 2018

dorodnic Jul 11, 2018

dorodnic Jul 11, 2018

matkatz Jul 12, 2018

dorodnic Jul 11, 2018

matkatz Jul 12, 2018

dorodnic Jul 11, 2018

matkatz Jul 12, 2018

dorodnic Jul 11, 2018

matkatz Jul 12, 2018

dorodnic Jul 11, 2018

matkatz Jul 12, 2018

Refactor pipeline resolve #2006

Refactor pipeline resolve #2006

Conversation

matkatz commented Jul 5, 2018 • edited Loading

dorodnic commented Jul 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matkatz commented Jul 5, 2018 •

edited

Loading