W10 support #3715

matkatz · 2019-04-09T06:49:08Z

The new W10 stream profile allows to stream IR and Depth streams with different resolutions and same frame rate.
Using wild cards is currently limited when streaming combinations of IR and Depth.
If both IR and Depth are required, both profiles should be configured without wildcards.

For example the following configuration will fail:

    cfg.enable_stream(RS2_STREAM_DEPTH);
    cfg.enable_stream(RS2_STREAM_INFRARED, 1, 1920, 1080, RS2_FORMAT_Y10BPACK, 30);

While this configuration is valid:

    cfg.enable_stream(RS2_STREAM_DEPTH, 0, 640, 480, RS2_FORMAT_Z16, 30);
    cfg.enable_stream(RS2_STREAM_INFRARED, 1, 1920, 1080, RS2_FORMAT_Y10BPACK, 30);

The Color sensor as no limitations and the following configuration is also valid:

    cfg.enable_stream(RS2_STREAM_COLOR);
    cfg.enable_stream(RS2_STREAM_DEPTH, 0, 640, 480, RS2_FORMAT_Z16, 30);
    cfg.enable_stream(RS2_STREAM_INFRARED, 1, 1920, 1080, RS2_FORMAT_Y10BPACK, 30);

If only IR is configured (on the Depth sensor) any wildcard is valid:

    cfg.enable_stream(RS2_STREAM_COLOR);
    cfg.enable_stream(RS2_STREAM_INFRARED, RS2_FORMAT_Y10BPACK);

ev-mp

Need to be tested with DQT.
We also need dedicated unit-test to verify the match/contradict change is not affected:

W10 + Depth with different resolutions is picked up properly. Also with wildcards.
Negative tests:
Depth/IR with different resolutions
IR1/IR2 with different formats

Metadata/UVC format patch - can be handled in a separate PR.

ev-mp · 2019-04-10T05:54:59Z

common/rendering.h

@@ -1280,6 +1280,9 @@ namespace rs2
 				}
 				break;
 			}
+            case RS2_FORMAT_Y10BPACK:


Check indentation

ev-mp · 2019-04-10T05:57:05Z

include/librealsense2/h/rs_sensor.h

@@ -74,6 +74,7 @@ typedef enum rs2_format
    RS2_FORMAT_GPIO_RAW        , /**< Raw data from the external sensors hooked to one of the GPIO's */
    RS2_FORMAT_6DOF            , /**< Pose data packed as floats array, containing translation vector, rotation quaternion and prediction velocities and accelerations vectors */
    RS2_FORMAT_DISPARITY32     , /**< 32-bit float-point disparity values. Depth->Disparity conversion : Disparity = Baseline*FocalLength/Depth */
+    RS2_FORMAT_Y10BPACK        , /**< 16-bit per-pixel grayscale image unpacked from 10 bits per pixel packed grey-scale image. The data is unpacked to LSB and padded with 6 zero bits */


Please add the packed format description [8:8:8:8:2222] or [10:10:10:10] to RW10 and Y10BPACK

I don't think we should define how raw10 is packed

ev-mp · 2019-04-10T05:58:29Z

src/core/streaming.h

@@ -263,6 +263,8 @@ namespace librealsense
        virtual void tag_profiles(stream_profiles profiles) const = 0;

        virtual bool compress_while_record() const = 0;
+
+        virtual bool contradicts(stream_profile_interface* a, const std::vector<stream_profile>& others) = 0;


make stream_profile_interface* a const
an the function itself const

ev-mp · 2019-04-10T06:02:00Z

src/device.cpp

+                return true;
+        }
+
+        for (auto request : others)


Both cases can be evaluated in a single for (auto request : others)

ev-mp · 2019-04-10T06:07:20Z

src/ds5/ds5-factory.cpp

+
+                for (auto request : others)
+                {
+                    // Patch for DS5U_S that allows different resolutions on multi-pin device


PLs drop the comment

ev-mp · 2019-04-10T06:10:31Z

src/ds5/ds5-factory.cpp

+            auto usb_spec = get_usb_spec();
+            if (usb_spec >= platform::usb3_type || usb_spec == platform::usb_undefined)
+            {
+                tags.push_back({ RS2_STREAM_DEPTH, -1, 720, 720, RS2_FORMAT_Z16, 30, profile_tag::PROFILE_TAG_SUPERSET | profile_tag::PROFILE_TAG_DEFAULT });


Shouldn't it be RS2_FORMAT_Y10BPACK instead of RS2_FORMAT_RAW10 ?

ev-mp · 2019-04-10T06:11:52Z

src/ds5/ds5-factory.cpp

+        {
+            if (auto vid_a = dynamic_cast<video_stream_profile_interface*>(a))
+            {
+                for (auto request : others)


PLs check if those two can be merged as in comment above

its probably not required anymore, the only limitation now is matching FPS, will remove

ev-mp · 2019-04-10T06:13:29Z

src/ds5/ds5-rolling-shutter.cpp

@@ -37,6 +37,7 @@ namespace librealsense
            // RS400 rolling-shutter Skus allow to get low-quality color image from the same viewport as the depth
            get_depth_sensor().register_pixel_format(pf_uyvyl);
            get_depth_sensor().register_pixel_format(pf_rgb888);
+            get_depth_sensor().register_pixel_format(pf_w10);


Pls check whether this should be propagated to D415/D410

ev-mp · 2019-04-10T06:16:31Z

src/pipeline/config.cpp

@@ -16,7 +16,7 @@ namespace librealsense
        {
            std::lock_guard<std::mutex> lock(_mtx);
            _resolved_profile.reset();
-            _stream_requests[{stream, index}] = { stream, index, width, height, format, fps };
+            _stream_requests[{stream, index}] = { stream, index, width, height, fps, format };


Is it a bug fix ?

I removed struct request_type and replaced it with stream_profile. the fps and format are in different locations.

ev-mp · 2019-04-10T06:18:35Z

src/pipeline/resolver.h

@@ -84,11 +75,11 @@ namespace librealsense
                return true;
            }

-            static bool match(stream_profile_interface* a, const request_type& b)
+            static bool match(stream_profile_interface* a, const stream_profile& b)


Pls make the func and params const

ev-mp

Please validate W10 with D416

add support in W10 format for d4xx rolling-shutter cameras

5711d2d

matkatz requested a review from ev-mp April 9, 2019 06:49

ev-mp reviewed Apr 10, 2019

View reviewed changes

matkatz added 3 commits April 14, 2019 11:19

raw10 - CR fix

c0f6b64

Merge branch 'development' into raw10-support

e362539

remove D416 from factory creation list

e7c8e54

matkatz requested a review from ev-mp April 14, 2019 15:01

ev-mp added the api_change label Apr 15, 2019

ev-mp approved these changes Apr 15, 2019

View reviewed changes

ev-mp merged commit 3e91600 into IntelRealSense:development Apr 15, 2019

matkatz deleted the raw10-support branch April 16, 2019 12:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

W10 support #3715

W10 support #3715

matkatz commented Apr 9, 2019 •

edited

Loading

ev-mp left a comment

ev-mp Apr 10, 2019

ev-mp Apr 10, 2019

matkatz Apr 11, 2019

ev-mp Apr 10, 2019

ev-mp Apr 10, 2019

ev-mp Apr 10, 2019

ev-mp Apr 10, 2019

ev-mp Apr 10, 2019

matkatz Apr 11, 2019

ev-mp Apr 10, 2019

ev-mp Apr 10, 2019

matkatz Apr 11, 2019

ev-mp Apr 10, 2019

ev-mp left a comment

W10 support #3715

W10 support #3715

Conversation

matkatz commented Apr 9, 2019 • edited Loading

ev-mp left a 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

ev-mp left a comment

Choose a reason for hiding this comment

matkatz commented Apr 9, 2019 •

edited

Loading