feat(sdk): builtin-object pipeline — SchemaHandler table, BufferAnchor, push_message_v2#86
Open
pabloinigoblasco wants to merge 13 commits into
Open
feat(sdk): builtin-object pipeline — SchemaHandler table, BufferAnchor, push_message_v2#86pabloinigoblasco wants to merge 13 commits into
pabloinigoblasco wants to merge 13 commits into
Conversation
pabloinigoblasco
changed the title
|
Green light from my side. Squash and merge would be nice. |
…hor, push_message_v2 ABI A coherent set of changes across pj_base and pj_plugins that establishes the canonical-object pipeline. Canonical-object SDK (pj_base/sdk/canonical_object.hpp): - BufferAnchor + PayloadView for zero-copy payload sharing (Span<const uint8_t> view + shared_ptr<const void> anchor). - sdk::Image, CompressedImage, PointCloud canonical types built around the view+anchor pattern so parsers can return them without copying the source bytes. - CanonicalObjectKind enum + SchemaClassification descriptor. - PixelFormat with kRGB888/kRGBA8888/kBGR888/kBGRA8888/kMono8/kMono16. MessageParser plugin base: - SchemaHandler table: per-schema registration with parse_scalars and parse_object callables. The base's classifySchema / parseScalars / parseObject methods are now table lookups. Plugins call registerSchemaHandler() in their constructor (or in bindSchema) to declare what they know about each type name. - parse() is no longer pure virtual: default implementation routes through parseScalars + writeHost.appendRecord, so plugins that register all their schemas via the table inherit parse() for free. C ABI: - PJ_payload_t / PJ_payload_anchor_t / PJ_payload_fetcher_t cross-ABI types: idempotent byte-fetcher with a release callback. - New push_message_v2 tail slot on PJ_data_source_runtime_host_vtable_t: the DataSource hands the host an idempotent fetcher; the host applies the active ObjectIngestPolicy (kPureLazy / kLazyObjectsEagerScalars / kEager) to decide when (and whether) to invoke it. - vtable size sentinel updated deliberately: 80 -> 104 bytes (MIN_VTABLE_SIZE pinned at the v4.0 baseline of 80). SDK C++ helpers: - DataSourceRuntimeHostView::pushMessage template: wraps a C++ closure (returning either PayloadView or vector<uint8_t>) into a PJ_payload_fetcher_t and delegates to push_message_v2. Returns an explicit error when the host doesn't expose the slot — no silent fallback to the legacy raw-message path. - ObjectIngestPolicy + ObjectIngestPolicyResolver with hierarchical override cascade: topic > data_source > kind > default. - MessageParserHandle::classifySchema wrapper for the tail-slot call. Canonical-object blob serialization: - Flat byte layout for Image / CompressedImage / PointCloud crossing the C ABI. Writer/reader pair under sdk/detail/. Tests: - object_ingest_policy_test: cascade rules at all four levels + last-write-wins. - push_message_v2_test: mock host exercising the template's fetcher wrap (vector and PayloadView shapes), idempotency under repeated fetch, ctx lifetime via shared_ptr canary, anchor propagation past fetcher release, and the explicit error when the host predates the slot. Status: design sketch posted as a draft. Compiles cleanly with the companion parser/runtime work; not yet exercised end-to-end against real data sources.
No behavior change. Reformats SDK headers, ABI headers, message parser plumbing, and the new ingest policy / push_message_v2 tests per .clang-format (Google style, 120-col, InsertBraces: true). Output of running pre-commit's clang-format hook over the files touched in this branch.
The file-level docstrings of canonical_object.hpp and object_ingest_policy.hpp pointed at a private report path that does not exist in this repository. Remove the dangling references; the surrounding prose already explains the design.
…functional scalars The canonical-object pipeline and the pure-functional scalar path are C++ SDK contracts: MessageParserPluginBase::parseObject() and parseScalars() are called directly on the C++ pointer by the in-process runtime host, preserving zero-copy via BufferAnchor. The C ABI slots (parse_object, parse_scalars) and their wire-format support are removed; pure-C plugins emit scalars via the parse() slot writing to writeHost. Removed: - detail/canonical_object_serialization.hpp (full file). - PJ_canonical_object_blob_t and PJ_named_field_value_buffer_t from canonical_object_abi.h. - parse_object and parse_scalars slots from PJ_message_parser_vtable_t. - trampoline_parse_object and trampoline_parse_scalars. - Per-instance buffers (scalars_owned_buf_, scalars_abi_buf_, object_blob_buf_) that only the trampolines used. Kept: - SchemaHandler::parse_scalars / parse_object (C++ callables registered by plugins) and MessageParserPluginBase::parseScalars / parseObject (C++ methods invoked by the host). - classify_schema C ABI slot (used by MessageParserHandle::classifySchema). - The parse() slot for pure-C plugins that emit scalars to writeHost. Vtable size: PJ_message_parser_vtable_t shrinks from 104 to 88 bytes (80 v4.0 baseline + 1 tail slot for classify_schema). PJ_MESSAGE_PARSER_MIN_VTABLE_SIZE stays pinned at 80.
ObjectIngestPolicy is a host-owned concern: the host instantiates its own ObjectIngestPolicyResolver during file-load setup and consults it on each push_message_v2 dispatch. DataSource plugins are policy-agnostic — they fabricate a payload fetcher via runtimeHost().pushMessage() and hand it off without inspecting or configuring policy. Exposing a per-view resolver in DataSourceRuntimeHostView contradicted that contract: mutations went to a local instance the host never read. The accessor is removed; the type sdk::ObjectIngestPolicyResolver stays in pj_base/sdk/object_ingest_policy.hpp as host-side vocabulary. Changes: - Drop the objectIngestPolicy() accessor. - Drop the mutable policy_resolver_ member. - Replace transitive include of object_ingest_policy.hpp with an explicit include of canonical_object.hpp (needed for PayloadView in pushMessage).
…le comments - Mark classifySchema / parseScalars / parseObject as virtual...final so derived plugins get a compile error if they try to override (the methods are invoked on MessageParserPluginBase*, so an override would silently not run). Matches the FINAL contract from the RFC. - Adapt the "Pure-functional API" section header to reflect the current state (no "added in protocol v5" — those slots are not at the C ABI level; the contract is C++ SDK direct-call only). - Rename test DefaultPolicyIsLazyScalars to DefaultPolicyIsLazyObjectsEagerScalars to match the enum value. - Rewrite the push_message_v2_test.cpp header summary so case 5 matches the assertion (explicit error when the host doesn't expose the slot; no silent fallback).
…, variant extension, fetcher type - push_raw_message: clarify it's the eager-only path; plugins needing lazy materialization or ObjectIngestPolicy dispatch use push_message_v2. - PJ_payload_t::size: size_t -> uint64_t for ABI-explicit 64-bit width (the protocol pins everything else to 64-bit; staying with size_t was a portability hazard on cross-compiles). - CanonicalObject variant: rewrite the extension-policy comment. Appending alternatives is forward-compatible — older hosts receiving an unknown kind reject the message, no protocol bump required. - pushMessage template: static_assert that the Fetcher returns either sdk::PayloadView or std::vector<uint8_t>. Catches misuse at compile time instead of falling through to UB in the else branch.
…e host vtable Compile-time sentinels for the public ABI types added in the canonical-object pipeline series: - PJ_canonical_object_kind_t: enum size pinned at 4 bytes. - PJ_schema_classification_t: 4 bytes; field offsets pinned. - PJ_payload_anchor_t: 16 bytes (ctx + release fn ptr). - PJ_payload_t: 32 bytes (data + size + anchor); field offsets pinned. - PJ_payload_fetcher_t: 24 bytes (ctx + fetch + release fn ptr). - PJ_data_source_runtime_host_vtable_t: 104 bytes (12 fn ptrs + prefix). Offsets of report_message, push_raw_message, list_available_encodings, and the push_message_v2 tail slot are pinned. struct size grows deliberately as future tail slots append. Any unintended reorder or size change of these public ABI types now fails the build instead of silently breaking binary compatibility with existing plugins.
pabloinigoblasco
force-pushed
the
feat/canonical-object-pipeline
branch
from
4d0333b to
1c778de
Compare
…, magic_enum)
Pervasive restructure of the canonical-object SDK. Zero behaviour
changes — rename, relocation and type unification only. The contract
(fetcher, in-process dispatch, ObjectIngestPolicy, anchor zero-copy) is
preserved.
Naming
- canonical-object → builtin-object across the SDK (headers, types,
macros, ABI symbols, CMake targets).
- PJ_payload_fetcher_t → PJ_message_data_fetcher_t; field .fetch →
.fetchMessageData.
Layout
- Builtin object types live in pj_scene_protocol/builtin/, one file per
type (BuiltinObject, BuiltinObjectKind, Image, DepthImage, PointCloud,
ImageAnnotations, CommonImageEncoding, depth_image_utils).
- MessageParser SDK moves out of pj_base into pj_plugins/sdk/
(message_parser_plugin_base, object_ingest_policy, trampolines).
- New pj_plugin_sdk INTERFACE library aggregates pj_base +
pj_scene_protocol for plugin authors.
Type model
- Image and CompressedImage unified into a single Image with std::string
encoding ("rgb8", "jpeg", "png", "compressedDepth", ...). PixelFormat
enum dropped; encoding is open-ended.
- Image::pixels -> Image::data.
- New DepthImage builtin with K + D + distortion_model; R and P are
derived via free helpers in depth_image_utils.h.
- ImageAnnotations promoted to top-level variant alternative.
Vocabulary helpers
- magic_enum-backed name/parse helpers for BuiltinObjectKind and
CommonImageEncoding. magic_enum/0.9.7 added to conanfile.
ABI
- Layout sentinels updated to pin the renamed PJ_message_data_fetcher_t
and PJ_builtin_object_kind_t. v4 vtable offsets unchanged.
Build: PJ4 green (69/69 tests pass).
Promote ImageAnnotations from a typedef alias into a fully-defined SDK builtin, aligned with the shape of Image / DepthImage / PointCloud. - Define struct PJ::sdk::ImageAnnotations and its companions (Point2, ColorRGBA, AnnotationTopology, PointsAnnotation, CircleAnnotation, TextAnnotation) directly in pj_scene_protocol/builtin/ImageAnnotations.h. - Delete the legacy pj_scene_protocol/image_annotation.h. - Move SceneFrame (codec output wrapper) into a dedicated pj_scene_protocol/scene_frame.h, now wrapping std::vector<sdk::ImageAnnotations>. - Codec: serializeImageAnnotation -> serializeImageAnnotations, signature switched to PJ::sdk::ImageAnnotations. - Update consumers (codec impl, decoder, tests, docs). Build green; 69/69 PJ4 tests pass.
pabloinigoblasco
changed the title
Continue the rename started by PJ_payload_fetcher_t → PJ_message_data_fetcher_t. Drop the "fetcher" shorthand in identifiers and prose; refer to the deferred byte-producing callable by its concrete name FetchMessageData (template/type) or fetch_message_data (parameter, snake_case per coding convention). - pushMessage<FetchMessageData> template parameter and the local FetchMessageDataT / FetchMessageDataResult aliases. - Local variable abi_fetch_message_data in the wrapper construction site. - Docstrings in data_source_protocol.h, data_source_host_views.hpp and object_ingest_policy.hpp refer to "the FetchMessageData callable" / "the callable" rather than "the fetcher". - Tests renamed: invokeFetchMessageDataAndExpect, FetchMessageDataCtxReleasedAfterHostCalls. No ABI change; the C struct PJ_message_data_fetcher_t and its field fetchMessageData were already in their final form. Build green; tests pass.
Per client feedback on the SDK's binary-compatibility story: replace
the closed variant<Image, PointCloud, DepthImage, ImageAnnotations>
with an opaque std::any holder.
Why
A std::variant alternative list is part of the SDK type and binds
every consumer to know the full set of alternatives at compile time.
Appending a kind would technically be a forward-compatible source
change but is a hard binary-compat boundary in practice — older
plugins linking the SDK would still produce the *old* variant type,
which is a different ABI from the host's *new* variant of the same
name. std::any sidesteps this entirely: producers and consumers only
need to agree on the concrete types they actually use; kindOf() does
the dispatch via RTTI.
How
- BuiltinObject is `std::any`.
- kindOf(obj) inspects obj.type() and maps it to BuiltinObjectKind;
returns kNone for an empty any or an unknown type.
- Producers continue to write `BuiltinObject{sdk::Image{...}}`,
`BuiltinObject{sdk::PointCloud{...}}`, etc., unchanged.
- Consumers use std::any_cast<T>(&obj) to recover the concrete value.
No behaviour change in the demos / host. Build green; tests pass
(modulo the pre-existing RosParserTest.{Embedded,GenericEmbedded}Timestamp
fails, unrelated to this change).
Add sections to the plugin SDK docs that cover the runtime-host slot introduced by the builtin-object pipeline: - data-source-guide.md: `pushMessage(handle, ts, fetchMessageData)` + PayloadView / BufferAnchor, `PJ_message_data_fetcher_t` C ABI, policy-agnostic plugin contract, thread-safety expectations. - message-parser-guide.md: `classifySchema` + `parseScalars` + `parseObject` virtual entry points, `BuiltinObject` as `std::any`, builtin type catalog (`Image` unified by encoding string, `DepthImage`, `PointCloud`, `ImageAnnotations`). - ARCHITECTURE.md: push_message_v2 tail slot at offset 96, host-side ObjectIngestPolicyResolver cascade (topic > source > kind > default), forward-compatibility rationale for the type-erased BuiltinObject.
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.
Summary
Foundational SDK pieces of the builtin-object pipeline: a contract by which
MessageParserplugins produce decoded scene primitives (images, depth images, point clouds, image annotations) and the host routes them throughObjectStorewith configurable lazy/eager policy.This PR is the SDK boundary only. The companion changes (plugin-side adoption in
parser_rosandmcap_source; host-side dispatch and consumer pull-path in the application) have been validated end-to-end against multi-GB real-world MCAPs.What this adds
1. Builtin-object types (
pj_scene_protocol/builtin/)One file per type, all under
namespace PJ::sdk:Image— unified raw + compressed image, withstd::string encoding("rgb8","bgr8","mono8","jpeg","png","compressedDepth", …). Encoding is open-ended; the legacyPixelFormatenum is gone. Pixel buffer accessed viadatafield (was previouslypixels).DepthImage— depth pixels + camera intrinsics (K,D,distortion_model).RandPderivable via free helpers indepth_image_utils.h.PointCloud— packed point cloud +PointFielddescriptors.ImageAnnotations— first-class builtin with its own struct + companion types (Point2,ColorRGBA,AnnotationTopology,PointsAnnotation,CircleAnnotation,TextAnnotation).BuiltinObject = std::variant<Image, PointCloud, DepthImage, ImageAnnotations>. Forward-compatible: older hosts receiving an unknown kind reject the message; no protocol bump required.Vocabulary helpers (
BuiltinObjectKind::name()/parse(),CommonImageEncoding::name()/parse()) are magic_enum-backed;magic_enum/0.9.7is added toconanfile.txt.2. Zero-copy payload sharing (
pj_base/buffer_anchor.hpp)BufferAnchor = std::shared_ptr<const void>— opaque keep-alive for the underlying bytes.PayloadView { Span<const uint8_t> bytes; BufferAnchor anchor; }— view + keep-alive pair. Parsers return them without copying source bytes in the in-process path.3. MessageParser plugin base (
pj_plugins/sdk/)The parser becomes a pure function. Three virtual entry points on
MessageParserPluginBase(all default tounexpectedto keep legacy plugins compiling):classifySchema(type_name, schema) → SchemaClassification— a-priori classification (kNone,kImage,kPointCloud,kDepthImage,kImageAnnotations) without parsing the payload.parseScalars(ts, payload) → Expected<vector<NamedFieldValue>>— scalar fields for the Datastore.parseObject(ts, payload) → Expected<BuiltinObject>— typed builtin for the ObjectStore.4. DataSource FetchMessageData path
DataSourceRuntimeHostView::pushMessage(handle, ts, fetchMessageData)— the callable produces that produces aPayloadView. The host invokes it 0 / 1 / N times depending on policy. Idempotent.PJ_message_data_fetcher_t(withfetchMessageDatacallback) andPJ_payload_t/PJ_payload_anchor_t.5. Ingest policy (
pj_plugins/sdk/object_ingest_policy.hpp)ObjectIngestPolicyResolverwith topic > source > kind > default cascade:kPureLazy— host never invokes the callable at ingest. Only registers in ObjectStore.kLazyObjectsEagerScalars— host invokes the callable once at ingest, parses scalars, drops bytes; the callable is retained for pulls.kEager— host invokes the callable at ingest, parses scalars + object, both persist in their stores.6. Aggregator:
pj_plugin_sdkINTERFACE libraryCMake INTERFACE library that aggregates
pj_base + pj_scene_protocolfor plugin authors. Plugins linkpj_plugin_sdkto get every header they need (builtin types, parser SDK, ingest policy, FetchMessageData).7. ABI safety
static_assertsentinels inpj_base/tests/abi_layout_sentinels_test.cpp(sizes, alignments, field offsets).push_message_v2slot appended to the runtime host vtable;MIN_VTABLE_SIZEconstants pinned at v4.0 release. Old plugins keep loading; old hosts reject unknown messages instead of crashing.PJ_ABI_VERSIONstays at 4 — backward-compatible within the v4 series.What's NOT in this PR (deliberately scoped)
ObjectStore.pushLazyintegration forkPureLazy/kLazyObjectsEagerScalarspaths (in PJ4 host).Test plan
plotjuggler_core+ downstreampj-official-plugins+ PJ4 host (build.sh).