From 6b02430ab4b2a62b434b33dcca1993285ce27769 Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Mon, 28 Sep 2026 07:19:00 +0200 Subject: [PATCH] =?UTF-8?q?feat(codec):=20deserializePointCloudView=20?= =?UTF-8?q?=E2=80=94=20decode=20PJ.PointCloud=20without=20copying=20the=20?= =?UTF-8?q?points=20(0.35.0)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit deserializePointCloud() copies the packed point bytes out of the wire buffer for every sample. deserializePointCloudView(data, size, anchor) points the cloud's `data` into the input instead and keeps it alive through the caller's BufferAnchor, like deserializeVideoFrameView. A null anchor copies, exactly as before. A new name rather than an overload: builtin_object_codec.cpp (and any consumer) takes the address of deserializePointCloud, which an overload would make ambiguous. Motivation (PJ4, Iris Xe): the host's per-sample copy of a 10M-point cloud costs a memmove plus ~29k first-touch page faults per frame; with the view the 10M stream frame went from 137 to 71 ms of wall time. MINOR: an additive client-side helper; host contract unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01V3ZZU95hWdyWcCj8dQkmAi --- CHANGELOG.md | 9 +++ VERSION | 2 +- .../pj_base/builtin/point_cloud_codec.hpp | 9 +++ pj_base/src/builtin/point_cloud_codec.cpp | 24 +++++++- pj_base/tests/point_cloud_codec_test.cpp | 55 +++++++++++++++++++ 5 files changed, 95 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f6b2595..7e97156e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,15 @@ All notable changes to `plotjuggler_sdk` are recorded here. Versioning policy is in [`CLAUDE.md`](./CLAUDE.md) → "Release Versioning". +## [0.35.0] + +Host contract: unchanged (no floor impact) — a client-side codec helper, no new +host surface. + +- Add `deserializePointCloudView(data, size, anchor)`: decodes a canonical + PJ.PointCloud without copying its packed point bytes (`data` aliases the input, + `anchor` keeps it alive), like `deserializeVideoFrameView`. A null anchor copies. + ## [0.34.1] - Fix a data race in `JobControl::armWatchdog`: the self-join guard read the diff --git a/VERSION b/VERSION index cd46610f..7b52f5e5 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.34.1 +0.35.0 diff --git a/pj_base/include/pj_base/builtin/point_cloud_codec.hpp b/pj_base/include/pj_base/builtin/point_cloud_codec.hpp index fce6d404..41c4830d 100644 --- a/pj_base/include/pj_base/builtin/point_cloud_codec.hpp +++ b/pj_base/include/pj_base/builtin/point_cloud_codec.hpp @@ -7,6 +7,7 @@ #include #include +#include "pj_base/buffer_anchor.hpp" #include "pj_base/builtin/point_cloud.hpp" #include "pj_base/expected.hpp" @@ -22,4 +23,12 @@ inline constexpr std::string_view kSchemaPointCloud = "PJ.PointCloud"; /// its packed point bytes via `anchor`. [[nodiscard]] Expected deserializePointCloud(const uint8_t* data, size_t size); +/// Decodes canonical PJ.PointCloud wire bytes without copying the packed point +/// bytes: the returned cloud's `data` ALIASES the input buffer and its `anchor` +/// is the supplied `anchor`, which must keep that buffer alive and unchanged for +/// as long as the cloud is used. A null `anchor` copies, like +/// deserializePointCloud(). +[[nodiscard]] Expected deserializePointCloudView( + const uint8_t* data, size_t size, sdk::BufferAnchor anchor); + } // namespace PJ diff --git a/pj_base/src/builtin/point_cloud_codec.cpp b/pj_base/src/builtin/point_cloud_codec.cpp index 557fe632..b5b04395 100644 --- a/pj_base/src/builtin/point_cloud_codec.cpp +++ b/pj_base/src/builtin/point_cloud_codec.cpp @@ -28,12 +28,18 @@ using sdk::PointField; // ---------- PointCloud payload bytes ---------- -bool readBytesIntoCloud(Reader& reader, PointCloud& out) { +// Points `out.data` into the input when `input_anchor` owns it, else copies. +bool readBytesIntoCloud(Reader& reader, PointCloud& out, const sdk::BufferAnchor& input_anchor) { const uint8_t* data = nullptr; size_t size = 0; if (!reader.readBytes(data, size)) { return false; } + if (input_anchor) { + out.data = Span(data, size); + out.anchor = input_anchor; + return true; + } auto owned = std::make_shared>(data, data + size); out.data = Span(owned->data(), owned->size()); out.anchor = owned; @@ -62,7 +68,9 @@ std::vector serializePointCloud(const PointCloud& cloud) { return out; } -Expected deserializePointCloud(const uint8_t* data, size_t size) { +namespace { + +Expected decodePointCloud(const uint8_t* data, size_t size, const sdk::BufferAnchor& input_anchor) { if (data == nullptr || size == 0) { return unexpected(std::string("PointCloud wire: empty buffer")); } @@ -143,7 +151,7 @@ Expected deserializePointCloud(const uint8_t* data, size_t size case 8: return tag.type == WireType::kLengthDelimited && readPointFieldIntoVector(r, cloud.fields); case 9: - return tag.type == WireType::kLengthDelimited && readBytesIntoCloud(r, cloud); + return tag.type == WireType::kLengthDelimited && readBytesIntoCloud(r, cloud, input_anchor); case 10: return tag.type == WireType::kLengthDelimited && r.readString(cloud.frame_id); default: @@ -158,4 +166,14 @@ Expected deserializePointCloud(const uint8_t* data, size_t size return cloud; } +} // namespace + +Expected deserializePointCloud(const uint8_t* data, size_t size) { + return decodePointCloud(data, size, nullptr); +} + +Expected deserializePointCloudView(const uint8_t* data, size_t size, sdk::BufferAnchor anchor) { + return decodePointCloud(data, size, anchor); +} + } // namespace PJ diff --git a/pj_base/tests/point_cloud_codec_test.cpp b/pj_base/tests/point_cloud_codec_test.cpp index 374bf98f..738cb358 100644 --- a/pj_base/tests/point_cloud_codec_test.cpp +++ b/pj_base/tests/point_cloud_codec_test.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include "protobuf_wire_test_helpers.hpp" @@ -79,5 +80,59 @@ TEST(PointCloudCodecTest, FrameIdAbsentRoundTrips) { EXPECT_TRUE(out->frame_id.empty()); } +PointCloud smallCloud(std::vector& payload) { + PointCloud in; + in.width = 2; + in.point_step = 12; + in.row_step = 24; + in.frame_id = "lidar"; + in.fields = { + {.name = "x", .offset = 0, .datatype = PointField::Datatype::kFloat32, .count = 1}, + {.name = "y", .offset = 4, .datatype = PointField::Datatype::kFloat32, .count = 1}, + {.name = "z", .offset = 8, .datatype = PointField::Datatype::kFloat32, .count = 1}, + }; + payload.assign(24, 0); + for (size_t i = 0; i < payload.size(); ++i) { + payload[i] = static_cast(i * 7); + } + in.data = Span(payload.data(), payload.size()); + return in; +} + +TEST(PointCloudCodecTest, ViewAliasesAnchoredBytes) { + std::vector points; + const PointCloud in = smallCloud(points); + const auto wire = std::make_shared>(serializePointCloud(in)); + auto out = deserializePointCloudView(wire->data(), wire->size(), wire); + ASSERT_TRUE(out.has_value()); + EXPECT_EQ(out->width, in.width); + EXPECT_EQ(out->frame_id, in.frame_id); + ASSERT_EQ(out->fields.size(), 3u); + ASSERT_EQ(out->data.size(), points.size()); + EXPECT_EQ(std::memcmp(out->data.data(), points.data(), points.size()), 0); + // No copy: the bytes live inside the wire buffer, which the result keeps alive. + EXPECT_GE(out->data.data(), wire->data()); + EXPECT_LE(out->data.data() + out->data.size(), wire->data() + wire->size()); + EXPECT_EQ(out->anchor, wire); +} + +TEST(PointCloudCodecTest, ViewWithoutAnchorCopies) { + std::vector points; + const PointCloud in = smallCloud(points); + const std::vector wire = serializePointCloud(in); + auto out = deserializePointCloudView(wire.data(), wire.size(), nullptr); + ASSERT_TRUE(out.has_value()); + ASSERT_EQ(out->data.size(), points.size()); + EXPECT_EQ(std::memcmp(out->data.data(), points.data(), points.size()), 0); + EXPECT_TRUE(out->data.data() < wire.data() || out->data.data() >= wire.data() + wire.size()); + EXPECT_NE(out->anchor, nullptr); +} + +TEST(PointCloudCodecTest, ViewRejectsWhatTheCopyingDecoderRejects) { + const auto garbage = std::make_shared>(std::vector{0xFF, 0xFF, 0xFF}); + EXPECT_FALSE(deserializePointCloudView(garbage->data(), garbage->size(), garbage).has_value()); + EXPECT_FALSE(deserializePointCloudView(nullptr, 0, nullptr).has_value()); +} + } // namespace } // namespace PJ