⚡ Match Highlights: realsenseai/librealsense/pull/

AviaAv · 2026-05-24T13:24:43Z

Tracked on: [RSDSO-20658]

Nir-Az

🤖 rs-agentic bot — automated code review

Thanks for the PR! A few issues and enhancement suggestions below (inline). Overall the approach is sound — using cv2 as a reference decoder for SDK output is a solid cross-check strategy.

Scope reviewed: (new), (modified), (new ), .

⚠️ This comment was auto-generated by the rs-agentic bot

Nir-Az · 2026-05-24T13:30:39Z

+pytestmark = [pytest.mark.timeout(30)]
+
+W, H = 640, 480
+TOL_MEAN = 30   # cv2 and SDK YUV->RGB differ by ~25 even when both correct


⚠️ Potential Bug — tolerance too permissive

TOL_MEAN = 30 with a known ~25-pixel mean difference «when both correct» leaves only a ~5-pixel margin to catch real SDK decoder bugs. This threshold might silently pass subtle implementation errors.

Consider supplementing the random-data test with a structured test input (e.g., pure-gray or a set of known YUV triplets) where the expected RGB output is analytically known, and assert with a tighter tolerance (e.g., ≤ 1 or ≤ 2).

⚠️ This comment was auto-generated by the rs-agentic bot

Fixed, reduced

Nir-Az

🤖 rs-agentic bot — automated code review

Thanks for the PR! Solid approach using cv2 as a reference decoder for cross-checking SDK output. A few issues and enhancement suggestions are noted inline below.

Files reviewed:

unit-tests/sw-dev/pytest-multi-format-grid.py (new)
unit-tests/sw-dev/sw_device.py (modified)
include/librealsense2/hpp/rs_processing.hpp (new uyvy_decoder)
wrappers/python/pyrs_processing.cpp (new Python bindings)

⚠️ This comment was auto-generated by the rs-agentic bot

Nir-Az · 2026-05-24T13:31:31Z

+pytestmark = [pytest.mark.timeout(30)]
+
+W, H = 640, 480
+TOL_MEAN = 30   # cv2 and SDK YUV->RGB differ by ~25 even when both correct


⚠️ Potential Bug — tolerance is too permissive

TOL_MEAN = 30 with a known ~25-pixel mean difference «when both correct» leaves only a ~5-pixel margin to catch real SDK decoder bugs. This threshold could silently pass subtle conversion errors.

Consider adding a structured test alongside the random-data one, using known YUV triplets (e.g. pure gray: Y=128, U=128, V=128 → RGB≈(128,128,128), or a pure-color swatch) where the expected RGB output is analytically known, asserting with a tight tolerance (≤ 1 or ≤ 2). This would complement the fuzzy random test and make regressions much easier to detect.

⚠️ This comment was auto-generated by the rs-agentic bot

Fixed, reduced

Nir-Az · 2026-05-24T13:31:31Z

+        stream = s.video_stream("Color", rs.stream.color, fmt, bpp)
+        s.start(stream)
+        f = stream.frame()
+        f.pixels = raw


⚠️ Potential Bug — pixel data override after frame construction

stream.frame() pre-fills f.pixels with default dummy bytes (self._pixels), and then f.pixels = raw overrides it. This works only if rs.software_video_frame stores a reference to the pixel buffer rather than copying it at construction time. If the binding copies pixels on assignment to f.pixels, this line is safe; but if on_video_frame() reads from self._pixels via the original reference, the override will be silently ignored and the test will always run the wrong data.

Recommendation: either (a) extend video_stream.frame() to accept an optional pixels parameter so the intent is explicit, or (b) add a comment here explaining why the late override is safe.

⚠️ This comment was auto-generated by the rs-agentic bot

added a comment

Nir-Az · 2026-05-24T13:31:31Z

+        f = stream.frame()
+        f.pixels = raw
+        received = s.publish(f)
+        return np.asanyarray(decoder_cls().process(received).get_data()).reshape(H, W, 3).copy()


⚠️ Potential Bug — unchecked reshape assumption

reshape(H, W, 3) assumes the decoder always produces a valid RGB8 frame of exactly H×W×3 bytes. If decoder_cls().process(received) returns a null/invalid frame (e.g. due to unsupported input), get_data() may return an empty or wrongly-sized buffer, and reshape will raise an opaque ValueError rather than a meaningful test failure.

Suggestion: assert received is not null and that the decoded frame's format is RS2_FORMAT_RGB8 before the reshape.

⚠️ This comment was auto-generated by the rs-agentic bot

Skipping. If the decoder ever produces a malformed frame the test still fails clearly enough via the reshape exception - extra assertions just duplicate that signal for the test code.

Nir-Az · 2026-05-24T13:31:31Z

+    cv2 and the SDK decode the same bytes; big pixel diff means SDK bug.
+    """
+    rng = np.random.default_rng(0)
+    for fmt, decoder_cls, cv2_decode in CASES:


💡 Enhancement — prefer @pytest.mark.parametrize over a for loop

Using a bare for loop means all four formats run inside a single test case. If an unexpected exception (not a soft check failure) is raised for one format, the remaining formats are skipped silently.

Parametrizing gives per-format test entries in the report, independent pass/fail status, and clearer CI output:

@pytest.mark.parametrize("fmt,decoder_cls,cv2_decode", CASES, ids=[c[0].name for c in CASES]) def test_sdk_vs_cv2_decoder(fmt, decoder_cls, cv2_decode): size, _ = fmt_layout(fmt) raw = np.random.default_rng(0).integers(0, 256, size=size, dtype=np.uint8) rgb_sdk = sdk_decode(raw, fmt, decoder_cls) rgb_cv2 = cv2_decode(raw) mean = float(np.abs(rgb_sdk.astype(int) - rgb_cv2.astype(int)).mean()) log.info(f"{fmt}: SDK vs cv2 mean |d|={mean:.2f}") assert mean <= TOL_MEAN, f"{fmt}: SDK decoder diverges from cv2 reference: mean |d|={mean:.2f} > {TOL_MEAN}"

⚠️ This comment was auto-generated by the rs-agentic bot

Nir-Az · 2026-05-26T08:17:45Z

+
+
+@pytest.mark.parametrize("fmt,decoder_cls,cv2_decode", [
+    (rs.format.yuyv, rs.yuy_decoder,  cv2_decode_yuyv),


What is the output format?
We need to test
rgba,bgr,,rgb8... and compare them

e.g.
INPUTS = [rs.format.yuyv, rs.format.uyvy, rs.format.nv12, rs.format.m420]
OUTPUTS = [rs.format.rgb8, rs.format.rgba8, rs.format.bgr8]

@pytest.mark.parametrize("in_fmt", INPUTS)
@pytest.mark.parametrize("out_fmt", OUTPUTS)
def test_sdk_vs_cv2(in_fmt, out_fmt):
raw = np.random.default_rng(0).integers(0, 256, size=fmt_layout(in_fmt)[0], dtype=np.uint8)
sdk = sdk_convert(raw, in_fmt, out_fmt) # via approach 1 or 2
ref = cv2_to(cv2_decode(raw, in_fmt), out_fmt)
assert mean_abs(sdk, ref) <= tol_for(out_fmt)

Added input/output formats as discussed

Nir-Az · 2026-05-27T12:03:57Z

                                                          "get better performance. Othere implementations (GLSL, OpenCL, Neon, NCS) should follow.");
    yuy_decoder.def(py::init<>());

+    py::class_<rs2::m420_decoder, rs2::filter> m420_decoder(m, "m420_decoder",


Check if used 'if no remove

Was unused. PR simplified now

Nir-Az

Great

add color convertor test

796ac75

Nir-Az reviewed May 24, 2026

View reviewed changes

tighten tolerance, parametrize, document layouts

25c523f

AviaAv requested a review from Nir-Az May 25, 2026 08:21

Nir-Az reviewed May 26, 2026

View reviewed changes

cover BGR/RGBA/BGRA targets

4bf95b1

AviaAv requested a review from Nir-Az May 27, 2026 11:21

Merge branch 'development' into rsdso-20658-multi-format-grid

4f81f0e

Nir-Az reviewed May 27, 2026

View reviewed changes

Nir-Az approved these changes May 27, 2026

View reviewed changes

remove unused no-arg color decoder bindings and uyvy_decoder stack

60ecf7c

AviaAv merged commit ac2bb6e into realsenseai:development May 27, 2026
29 of 30 checks passed

AviaAv deleted the rsdso-20658-multi-format-grid branch May 27, 2026 13:02



		@pytest.mark.parametrize("fmt,decoder_cls,cv2_decode", [
		(rs.format.yuyv, rs.yuy_decoder, cv2_decode_yuyv),

Conversation

AviaAv commented May 24, 2026

Uh oh!

Nir-Az left a comment

Choose a reason for hiding this comment

🤖 rs-agentic bot — automated code review

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nir-Az left a comment

Choose a reason for hiding this comment

🤖 rs-agentic bot — automated code review

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nir-Az left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants