diff --git a/AGENTS.md b/AGENTS.md index c03eb2a..4ae4065 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -24,7 +24,9 @@ CI=true SOURCE_FRAMES=30 dora run dataflow.yml --uv # the same, headless ## Architecture - **`dora_image_view/main.py`** — Argument parsing and the dora event loop, nothing else. The env var wins over the CLI flag for `VIEW_WIDTH`/`VIEW_HEIGHT`. The upstream node spells these `PLOT_WIDTH`/`PLOT_HEIGHT`; they were renamed with the node, so a dataflow copied from upstream needs its env keys changed. `Node(args.name)` rather than `Node()` so the node also runs standalone as a dynamic node. -- **`dora_image_view/decode.py`** — `to_bgr()`: every supported encoding to a writable BGR array. Two behaviours are deliberate, not bugs: `bgr8` **copies** (the payload may be a read-only view on shared memory, and the frame is annotated in place), and `yuv420` converts with `COLOR_YUV420p2RGB` rather than `...2BGR` — the upstream node converts to RGB and then displays it as BGR, and that channel swap is part of the behaviour callers already see. +- **`dora_image_view/decode.py`** — `to_bgr()`: every supported encoding to a writable BGR array. `unpack()` accepts two payload shapes — the `std/media/v1/Image` struct (`width`, `height`, `encoding`, `data`), and the flat `UInt8` buffer described by message metadata that a `std/core/v1/UInt8` producer sends. A struct describes itself, so its metadata is ignored; everything after unpacking is common. + + **The declared type is still `std/core/v1/UInt8` and the reason is interop.** Type compatibility is directional: `Image` satisfies a `Bytes` input, but a `std/core/v1/UInt8` output wired into an `std/media/v1/Image` input is a validate error. dora-zed declares `image_bgr` as `std/core/v1/UInt8`, so flipping this manifest to `Image` breaks the camera until dora-zed is migrated. Accepting both shapes first is what makes that migration orderable: producers move, then the contract. Two behaviours are deliberate, not bugs: `bgr8` **copies** (the payload may be a read-only view on shared memory, and the frame is annotated in place), and `yuv420` converts with `COLOR_YUV420p2RGB` rather than `...2BGR` — the upstream node converts to RGB and then displays it as BGR, and that channel swap is part of the behaviour callers already see. - **`dora_image_view/view.py`** — `View` (the panels, the overlay state and the window) and `Bboxes`. `headless` (from `CI=true`) skips only the window: every input is still decoded and composed, which is what makes the node safe in a pipeline test. Panels are tiled in the fixed order in `PANELS`, not in arrival order, so a panel does not jump when a stream drops a frame. Boxes are drawn on the `image` panel before scaling, because they are in image pixels; the caption is drawn on the composite, because the panel it would otherwise land on may not be the leftmost one. - **`dora_image_view/depth.py`** — `to_millimetres()` and `colorize()`. Decoding normalises to float32 millimetres with 0 for missing, so the colouring only ever sees one representation. One thing is deliberate: the auto range is the valid pixels' full min/max, **not** a percentile. Trimming the tails looks safer and breaks the exact scene this node exists for — a small object in front of a large wall is a few percent of the pixels, so both tails land on the wall and the object clips away. A rendered check caught that; do not reintroduce percentile ranging. - **`dora_image_view/payload.py`** — `as_float32()`: reinterprets a `uint8` payload rather than converting it, because a C++ node sending a raw byte slice produces a `UInt8` arrow array despite declaring `Float32`. Shared by `depth` and `intrinsics`, which both meet it. @@ -45,6 +47,10 @@ CI=true SOURCE_FRAMES=30 dora run dataflow.yml --uv # the same, headless `opencv-python-headless` needs none of them, but it also has no `imshow`, which is this node's whole purpose. Removing the step therefore means making the GUI wheel an optional extra — headless by default, `pip install .[gui]` for a real window — and that is a change to what a consumer installs, not just to CI. It has not been done. +## On `output_framing` + +`output_framing: {image: arrow-ipc}` sets a per-output wire format — `raw` (the default, plain Arrow buffer layout) or `arrow-ipc` (self-describing: schema plus record batches). It is *not* required for images: a struct payload was verified to arrive identically under both framings between Python nodes, and `dora validate` does not ask for it when an output is annotated `std/media/v1/Image`. Reach for `arrow-ipc` when the wire has to be self-describing — crossing languages, or recording — not because a payload is an image. + ## A caveat on typed optional inputs `dora validate --strict` injects a contract type for *every* typed manifest input and then warns `input_types key "..." not found in inputs list` for any the dataflow does not wire. So a typed optional input forces the example dataflow to wire it. If a typed input is added to the manifest, either wire it in `dataflow.yml` or leave it untyped. This is why `examples/source.py` publishes a depth map and a camera_info at all: both are typed, so the example has to drive them, and a new output it sends needs a matching entry in the dataflow's `output_types:`. diff --git a/README.md b/README.md index 0ec34b0..b9bb0df 100644 --- a/README.md +++ b/README.md @@ -106,12 +106,24 @@ no window. That is what makes it safe to run in a pipeline test. | Input | Payload | Metadata | |---|---|---| -| `image` | Flat `UInt8` array | `width`, `height`, `encoding` | +| `image` | `std/media/v1/Image` struct, **or** a flat `UInt8` array | none for the struct; `width`, `height`, `encoding` for the flat form | | `depth` | Flat `Float32` array, `[H, W]` | `shape`, `units` | | `camera_info` | Flat `Float32` array, `[fx, fy, cx, cy]` or a 9-element K | `distortion`, `distortion_model` | | `bbox` | Arrow struct array, element `[0]` with fields `bbox`, `conf`, `labels` | `format` | | `text` | Arrow array of size 1 | — | +Two image payload shapes are accepted. The typed one is `std/media/v1/Image`, an Arrow +struct of `width: UInt32`, `height: UInt32`, `encoding: Utf8` and `data: LargeBinary`, +which carries inside the payload what the flat form carries beside it; when it is +received the message metadata is not consulted at all. The flat form is a raw `UInt8` +buffer described by `width`, `height` and `encoding` metadata, which is what a node +declaring `std/core/v1/UInt8` sends. + +The declared contract is still `std/core/v1/UInt8`, so a producer sending the struct +should keep declaring that until every producer in the dataflow has moved: dora's type +compatibility is directional, and a `std/core/v1/UInt8` output wired into an +`std/media/v1/Image` input fails `dora validate`. + Supported `encoding` values: `bgr8`, `rgb8`, `jpeg`/`jpg`/`jpe`/`bmp`/`webp`/`png` (encoded byte stream), `yuv420`, `avif`. `width` and `height` are only needed by the raw encodings — the rest carry their own dimensions. An unsupported encoding raises. diff --git a/dora_image_view/decode.py b/dora_image_view/decode.py index df95e80..a1745b4 100644 --- a/dora_image_view/decode.py +++ b/dora_image_view/decode.py @@ -1,4 +1,12 @@ -"""Decoding of the `image` input into a BGR frame that can be drawn on.""" +"""Decoding of the `image` input into a BGR frame that can be drawn on. + +Two payload shapes are accepted, because dora has both in the field. The typed +one is `std/media/v1/Image`: an Arrow struct carrying width, height, encoding +and data together, so the frame is self-describing. The older one is a flat +UInt8 buffer whose dimensions and encoding travel beside it in the message +metadata, which is what a node declaring `std/core/v1/UInt8` sends -- dora-zed +among them. Everything after unpacking is identical. +""" import io @@ -14,13 +22,18 @@ SELF_DESCRIBING = frozenset({"jpeg", "jpg", "jpe", "bmp", "webp", "png"}) SUPPORTED = SELF_DESCRIBING | {"bgr8", "rgb8", "yuv420", "avif"} -def to_bgr(storage, encoding: str, width: int | None, height: int | None) -> np.ndarray: +# The fields of std/media/v1/Image. A payload carrying them describes itself and +# its metadata is not consulted. +IMAGE_FIELDS = ("width", "height", "encoding", "data") + + +def to_bgr(storage, metadata: dict) -> np.ndarray: """Decode one `image` payload into a writable BGR array. - `width` and `height` come from the message metadata and are only required by - the raw encodings, which carry no dimensions of their own. + Accepts either an `std/media/v1/Image` struct or a flat buffer described by + the message metadata. """ - buffer = storage.to_numpy().astype(np.uint8) + buffer, encoding, width, height = unpack(storage, metadata) if encoding == "bgr8": # Copy so the frame can be annotated: to_numpy() may be a view on the @@ -53,6 +66,36 @@ def to_bgr(storage, encoding: str, width: int | None, height: int | None) -> np. raise RuntimeError(f"unsupported image encoding: {encoding}") +def unpack(storage, metadata: dict) -> tuple[np.ndarray, str, int | None, int | None]: + """Split a payload of either shape into buffer, encoding, width and height.""" + if _is_image_struct(storage): + # One image per message: the struct is a single-row array. + row = storage[0].as_py() + return ( + np.frombuffer(row["data"], dtype=np.uint8), + str(row["encoding"]).lower(), + row["width"], + row["height"], + ) + + return ( + storage.to_numpy().astype(np.uint8), + str(metadata.get("encoding", "bgr8")).lower(), + metadata.get("width"), + metadata.get("height"), + ) + + +def _is_image_struct(storage) -> bool: + """Whether the payload is an Arrow struct with the std/media/v1/Image fields.""" + fields = getattr(getattr(storage, "type", None), "fields", None) + + if fields is None: + return False + + return all(name in {field.name for field in fields} for name in IMAGE_FIELDS) + + def _reshape(buffer: np.ndarray, width: int | None, height: int | None, encoding: str): width, height = _dimensions(width, height, encoding) return _take(buffer, width * height * 3, encoding).reshape(height, width, 3) diff --git a/dora_image_view/main.py b/dora_image_view/main.py index 3b052ed..1335f35 100644 --- a/dora_image_view/main.py +++ b/dora_image_view/main.py @@ -67,12 +67,7 @@ def _decode_panel(event_id: str, value, metadata: dict) -> tuple[str, np.ndarray event loop to treat it as overlay state instead. """ if event_id == "image": - return "image", decode.to_bgr( - value, - metadata.get("encoding", "bgr8").lower(), - metadata.get("width"), - metadata.get("height"), - ) + return "image", decode.to_bgr(value, metadata) if event_id == "depth": return "depth", depth.colorize(depth.to_millimetres(value, metadata)) diff --git a/tests/test_dora_image_view.py b/tests/test_dora_image_view.py index e12e9c9..fb15416 100644 --- a/tests/test_dora_image_view.py +++ b/tests/test_dora_image_view.py @@ -4,6 +4,7 @@ import io import cv2 import numpy as np +import pyarrow as pa import pytest from PIL import Image @@ -13,6 +14,11 @@ from dora_image_view.view import Bboxes, View WIDTH, HEIGHT = 12, 8 +def _meta(encoding: str, width: int | None = None, height: int | None = None) -> dict: + """Build message metadata as a node sending a flat buffer attaches it.""" + return {"encoding": encoding, "width": width, "height": height} + + class FakeStorage: """Stands in for the Arrow array dora hands to the node.""" @@ -30,7 +36,7 @@ def frame() -> np.ndarray: def test_bgr8_round_trips_unchanged(frame): - decoded = decode.to_bgr(FakeStorage(frame.ravel()), "bgr8", WIDTH, HEIGHT) + decoded = decode.to_bgr(FakeStorage(frame.ravel()), _meta("bgr8", WIDTH, HEIGHT)) np.testing.assert_array_equal(decoded, frame) @@ -38,7 +44,7 @@ def test_bgr8_round_trips_unchanged(frame): def test_rgb8_round_trips_unchanged(frame): rgb = cv2.cvtColor(frame, cv2.COLOR_BGR2RGB) - decoded = decode.to_bgr(FakeStorage(rgb.ravel()), "rgb8", WIDTH, HEIGHT) + decoded = decode.to_bgr(FakeStorage(rgb.ravel()), _meta("rgb8", WIDTH, HEIGHT)) np.testing.assert_array_equal(decoded, frame) @@ -53,7 +59,7 @@ def test_self_describing_encodings_need_no_dimensions(frame, suffix, encoding): ok, buffer = cv2.imencode(suffix, frame) assert ok - decoded = decode.to_bgr(FakeStorage(buffer.ravel()), encoding, None, None) + decoded = decode.to_bgr(FakeStorage(buffer.ravel()), _meta(encoding)) assert decoded.shape == (HEIGHT, WIDTH, 3) @@ -61,7 +67,7 @@ def test_self_describing_encodings_need_no_dimensions(frame, suffix, encoding): def test_yuv420_is_decoded(frame): planes = cv2.cvtColor(frame, cv2.COLOR_BGR2YUV_I420) - decoded = decode.to_bgr(FakeStorage(planes.ravel()), "yuv420", WIDTH, HEIGHT) + decoded = decode.to_bgr(FakeStorage(planes.ravel()), _meta("yuv420", WIDTH, HEIGHT)) assert decoded.shape == (HEIGHT, WIDTH, 3) @@ -71,7 +77,7 @@ def test_avif_is_decoded(frame): Image.fromarray(cv2.cvtColor(frame, cv2.COLOR_BGR2RGB)).save(buffer, format="AVIF") payload = np.frombuffer(buffer.getvalue(), dtype=np.uint8) - decoded = decode.to_bgr(FakeStorage(payload), "avif", None, None) + decoded = decode.to_bgr(FakeStorage(payload), _meta("avif")) assert decoded.shape == (HEIGHT, WIDTH, 3) @@ -79,7 +85,7 @@ def test_avif_is_decoded(frame): def test_decoded_frame_is_writable(): storage = FakeStorage(np.arange(2 * 3 * 3, dtype=np.uint8)) - decoded = decode.to_bgr(storage, "bgr8", width=3, height=2) + decoded = decode.to_bgr(storage, _meta("bgr8", 3, 2)) # The frame is annotated in place, so it must not be a read-only view. decoded[0, 0] = 0 @@ -88,24 +94,22 @@ def test_decoded_frame_is_writable(): def test_raw_encodings_need_dimensions(): storage = FakeStorage(np.zeros(18, dtype=np.uint8)) with pytest.raises(RuntimeError, match="width"): - decode.to_bgr(storage, "bgr8", width=None, height=None) + decode.to_bgr(storage, _meta("bgr8")) def test_short_payload_is_rejected(): storage = FakeStorage(np.zeros(4, dtype=np.uint8)) with pytest.raises(RuntimeError, match="expected at least"): - decode.to_bgr(storage, "bgr8", width=3, height=2) + decode.to_bgr(storage, _meta("bgr8", 3, 2)) def test_unsupported_encoding_is_rejected(): storage = FakeStorage(np.zeros(4, dtype=np.uint8)) with pytest.raises(RuntimeError, match="unsupported image encoding"): - decode.to_bgr(storage, "tiff", width=None, height=None) + decode.to_bgr(storage, _meta("tiff")) def test_xywh_is_converted_to_corners(): - import pyarrow as pa - value = pa.array( [ { @@ -283,3 +287,52 @@ def test_the_camera_model_is_drawn_on_the_composite(): with_model = view.compose() assert not np.array_equal(without, with_model), "the model should be drawn" + + +def _image_struct(frame: np.ndarray, encoding: str = "bgr8"): + """Build a payload shaped like std/media/v1/Image.""" + return pa.array( + [{ + "width": frame.shape[1], + "height": frame.shape[0], + "encoding": encoding, + "data": frame.tobytes(), + }], + type=pa.struct([ + ("width", pa.uint32()), + ("height", pa.uint32()), + ("encoding", pa.utf8()), + ("data", pa.large_binary()), + ]), + ) + + +def test_image_struct_is_decoded(frame): + # No metadata at all: the struct describes itself. + decoded = decode.to_bgr(_image_struct(frame), {}) + + np.testing.assert_array_equal(decoded, frame) + + +def test_image_struct_wins_over_message_metadata(frame): + # Metadata disagreeing with the struct must not be consulted. + decoded = decode.to_bgr(_image_struct(frame), _meta("rgb8", 1, 1)) + + np.testing.assert_array_equal(decoded, frame) + + +def test_image_struct_carrying_an_encoded_frame_is_decoded(frame): + ok, encoded = cv2.imencode(".png", frame) + assert ok + payload = np.frombuffer(encoded.tobytes(), dtype=np.uint8).reshape(1, -1) + + decoded = decode.to_bgr(_image_struct(payload, "png"), {}) + + np.testing.assert_array_equal(decoded, frame) + + +def test_a_decoded_image_struct_frame_is_writable(frame): + decoded = decode.to_bgr(_image_struct(frame), {}) + decoded[0, 0] = (1, 2, 3) + + assert tuple(decoded[0, 0]) == (1, 2, 3)