Feature: support bayer encoding - #1749
Open
abaeyens wants to merge 10 commits into
Open
Conversation
Resolves ros2#887. Bayer encodings (bayer_rggb8 / bayer_bggr8 / bayer_gbrg8 / bayer_grbg8 and their 16-bit variants) were previously routed to a stub that handed the raw mosaic to Ogre as PF_BYTE_L, rendering as a visible checkerboard. They are now bilinearly demosaiced to sRGB-encoded RGB. The demosaic is hand-rolled to avoid pulling in OpenCV / cv_bridge as a dependency, matching the policy established in ros-visualization/rviz#1009 and the existing YUYV / UYVY / NV12 converter pattern in this file. A single templated implementation (parameterised on the input pixel type and on the (R_DY, R_DX) position of R within the 2x2 mosaic cell) covers all four layouts at both 8-bit and 16-bit. The interior loop is bounds-check-free; four separate border passes handle the 1-pixel edge with clamp-to-edge and a count-of-valid- neighbours average. Each demosaiced linear value passes through an sRGB transfer-function LUT (256 entries at 8-bit, 65536 entries / 64 KiB at 16-bit, built once on first use). The sRGB step is always-on for Bayer because the bilinear output is linear-light and the texture is displayed as sRGB; a user-toggleable linear-input property is left for a follow-up if a real use case appears. For 16-bit Bayer, the existing Normalize Range / Min Value / Max Value / Median window properties are reused. Cameras packing 10/12/14-bit data into 16-bit messages can be displayed correctly by disabling Normalize Range and setting Max Value to the actual bit-depth maximum (1023 / 4095 / 16383). Min/max statistics are computed over the input mosaic (not the 3x-larger demosaiced output) using a stride-aware row-by-row scan that ignores padding bytes between rows; this scan and the bayer_16bit_padded_stride regression test become removable once PR ros2#1719 lands and repacks padded buffers in update() before dispatch (see in-file comment for cleanup notes in both directions). Input validation rejects malformed messages upfront with UnsupportedImageEncoding: zero or oversize dimensions (> 32768), inconsistent stride (< width * bpp), truncated data (< height * stride), odd stride for 16-bit, and misaligned data_ptr for 16-bit (the latter guards against strict-alignment platforms like ARMv7 where the uint16_t reinterpret_cast would otherwise be undefined behaviour). Unknown bayer_* encodings now throw rather than being silently rendered as luminance; a small deliberate user-visible behaviour change. In ImageDisplay, the private got_float_image_ flag is renamed to has_normalizable_range_ to reflect that it now also gates the normalize controls for 16-bit Bayer. Tooltip on Normalize Range is extended to mention Bayer applicability. Tests cover: all four 8-bit layouts (pure-red and pure-blue, which are discriminating across layouts because a layout-swap bug surfaces as swapped output channels); all four 16-bit layouts with fixed Max Value; 16-bit black-point via Min Value; running-median smoke test for the auto-normalisation path; padded-stride regression test (verified to fail under the previous contiguous min/max scan); scale-then-sRGB ordering; multi-anchor sRGB transfer-function validation across the linear toe, the knee at 0.0031308, and the upper exponent range; small-image edge cases (2x2 and 3x3); unknown-encoding rejection; and oversize / truncated / odd-stride-16-bit rejection. Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>
The hover-over pixel readout introduced in ros2#1716 has no Bayer branch, so for bayer_* encodings it falls through to the raw-byte fallback at image_display.cpp:592, showing one byte for 8-bit Bayer or two raw bytes for 16-bit Bayer with no channel context. This pre-existed our demosaic work but is far more visible now that Bayer images actually render correctly. Add a Bayer-aware branch to formatPixel that, given a hover position (px, py), reads the four sensel values from the 2x2 mosaic cell anchored at (px & ~1, py & ~1) and labels each by its channel based on the layout (R / G / G / B for RGGB; mirrored for the other three). The readout is honest about what the sensor recorded — the on-screen pixel values shown are interpolated, the readout values are not. Output is HTML-formatted to match the existing colour-coded RGB8 / RGBA8 styles. Edge pixels clamp neighbour coordinates to the image bounds, matching the demosaic's edge handling. No unit test added; the existing image_display.cpp pixel-readout helpers are in an anonymous namespace and the formatPixelAt accessor is private. Visual verification with a Bayer publisher is sufficient for this UI-formatting change, matching the precedent of ros2#1716 itself. Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>
This was referenced May 25, 2026
…upport-bayer-encoding Signed-off-by: Arne Baeyens <mail@arnebaeyens.com> Conflicts: rviz_default_plugins/include/rviz_default_plugins/displays/image/image_display.hpp rviz_default_plugins/src/rviz_default_plugins/displays/image/image_display.cpp rviz_default_plugins/test/rviz_default_plugins/displays/image/ros_image_texture_test.cpp
The previous "Bayer input is linear-light, apply sRGB gamma" rule was hardcoded. Expose it as a per-display "Linear input" BoolProperty on ImageDisplay; hidden unless the current message is Bayer; default off. Toggling on a latched frame re-runs the converter without needing a new publish (mirrors setSmoothScaling). Now that PR ros2#1719 (buffer repack in update()) has landed upstream: - Replace the inline stride-aware 16-bit min/max scan with a call to the shared getMinimalAndMaximalValueToNormalize<uint16_t>. - Delete unreachable defensive stride/alignment checks and the now-obsolete odd_stride_16bit_rejected test. Also: parameterise linear-passthrough tests over all four Bayer layouts (8 and 16-bit); add tests for fixed-max above actual, median-frames tracking, toggle round-trip on a held frame, and initialize-time propagation of the default. Fix a -Wreorder in the constructor init list. Name the sRGB piecewise constants (IEC 61966-2-1). Apply ament_uncrustify to ros_image_texture.cpp. Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>
The two header files only gained small declaration additions (a setter override, a Q_SLOT, a couple of tracking bools). That is below the threshold for a new copyright attribution; the substantive work lives in the corresponding .cpp files and the test file, which keep the 2026 line. Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>
Test-quality fixes:
- srgb_transfer_anchor_points: drop the five tautological EXPECT_* against
the test-side srgbEncodeByte reference; the remaining test drives four
anchor input values (0, 1, 128, 255) through ROSImageTexture and checks
the demosaic output. Comment that the piecewise-linear segment is
unreachable via 8-bit input.
- bayer_16bit_padded_stride: fix a misleading docstring (update() repacks
the buffer, so the min/max scan never sees padding) and drop the
incorrect "padding biased min_value to 0" reasoning on the G/B
assertions.
- small_{2,3}x{2,3}_image tests: pure-red mosaic + per-pixel R >= 100 and
G = B = 0 assertions instead of asserting only rgb.size().
- bayer_16bit_normalize_range_smoke: deleted; the running-median path is
now covered by bayer_16bit_median_frames_tracks_median_not_last with
proper output assertions.
- bayer_16bit_srgb_after_scaling / linear-mode fixed-max helpers: move the
ASSERT_GT/LT sentries from the test-side reference onto the actual
output. A wrong-ordering or auto-detect bug now surfaces as a pixel
mismatch, not a test-reference sanity failure.
Consolidation:
- buildUniformMosaic8 / buildUniformMosaic16 folded into one templated
buildUniformMosaic<T>; 14 call sites updated.
- checkPureRedDemosaicSrgb / checkPureBlueDemosaicSrgb folded into one
checkPureChannelDemosaicSrgb(encoding, channel).
- Twenty hand-copied TEST_F entries collapsed into four TEST_P suites
(Bayer8bitSrgbPurChannelTest, Bayer16bitSrgbFixedMaxTest,
Bayer8bitLinearTest, Bayer16bitLinearFixedMaxTest) parameterised over
the four Bayer layouts. RosImageTextureTestFixture::SetUpTestCase is
guarded against re-entry so parameterised subclasses can share the
singleton Ogre environment.
Display test:
- initialize_propagates_linear_input_default_to_texture: tighten
Times(AtLeast(1)) -> Times(1) so a redundant propagation would surface.
Deferred for now: display-side visibility gating (needs a test shim for
protected processMessage), interior-vs-border assertions on the 8x8 pure-
red tests, uppercase encoding fallthrough test, readTextureRGB format
assertion.
Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>
- Move the encoding -> layout / bit-depth table into bayer_format.hpp as the single source of truth, shared by the demosaic, the property gating, and the pixel read-out; gate the Bayer-dependent UI on this whitelist rather than image_encodings::isBayer() - Report malformed frames (zero/oversize dimensions, bad step, truncated data) via a new MalformedImageMessage exception naming the actual fault, instead of blaming the encoding - Reset the Image status to OK once a subsequent frame converts cleanly, so a transient bad frame no longer latches a permanent error; broaden the display catch to std::exception so allocation failure degrades to an error status instead of aborting - Bound the total pixel count (2^28) in addition to the per-dimension cap, and require exactly packed rows, making the precondition of the contiguous 16-bit min/max scan explicit - Fold the 16-bit rescale (and optional sRGB transfer) into a per-frame lookup table so the demosaic inner loop does one table lookup per channel instead of floating-point math - Match convertTo8bit's isfinite() guard; avoid a std::clamp precondition violation in the pixel read-out for absurd dimensions - Tests: drop a stale comment, fix a fixture typo, unify fixture suffixes, drop assertions implied by adjacent EXPECT_NEARs, split a two-behaviour test - Mention Bayer support in docs/FEATURES.md Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>
abaeyens
force-pushed
the
abaeyens/rolling/support-bayer-encoding
branch
from
July 6, 2026 21:01
0fd6136 to
477749f
Compare
abaeyens
marked this pull request as ready for review
July 6, 2026 21:02
Contributor
|
Tick the box to add this pull request to the merge queue (same as
|
Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>
no code changes, only reordening Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>
abaeyens
commented
Jul 8, 2026
Comment on lines
+76
to
+78
| // When enabled, apply sRGB gamma to converter output instead of passthrough. | ||
| // Default no-op for out-of-tree implementors. | ||
| virtual void setLinearInput(bool /*enabled*/) {} |
Contributor
Author
There was a problem hiding this comment.
This, together with a few other changes, breaks ABI, therefore to ease backporting this, I suggest I split this PR up in two - one that adds pure bayer support, without the "Linear input" toggle that breaks ABI, and a second PR which adds that toggle (and the associated logic)? We can then rather easily backport the former, and just not backport the latter (as that toggle is only of secondary importance).
ahcorde
requested changes
Jul 9, 2026
| #include "rviz_rendering/render_window.hpp" | ||
| #include "sensor_msgs/image_encodings.hpp" | ||
|
|
||
| #include "./bayer_format.hpp" |
Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Handle the eight bayer encodings ROS Image messages can have (= 4 sensel layouts for 8- and 16-bit). Given that bayer images tend to be in linear light, also add a "Linear input" toggle, similar to the one recently introduced in rqt_image_view (though here it's currently only active for bayer images).
Fixes #887
Screenshot of a bayer image with these changes:
Is this user-facing behavior change?
Yes, with this PR bayer-encoded images are shown in full color instead of in grey with a checkerboard pattern (for 8-bit bayer encodings), screenshot:
... or completely unsupported for the case of 16-bit, given "No Image" and the following error:
Did you use Generative AI?
Claude Opus 4.7, Claude Fable 5
Additional Information
Work in progress, I'm aware of several things to clean up. That said, please feel free to already provide feedback on the architecture and functional aspects.=> ready for reviewA remaining question for me is how to handle linear vs sRGB color space (see also my question here). Currently, this PR considers all bayer-encoded images to be expressed in linear light. If they instead follow an sRGB-like OETF, then they'll be shown with their shadows too bright. One solution is to add a toggle "Linear input" like we're doing for RQt.=> added toggle "Linear input"