[Jacob Kaplan (qwertey6)]

Summary

Camera::encode_depth_map and Camera::decode_depth_map processed one value at a time.

• Encode: read m(i, j) and wrote it with a bounds-checked append_big_endian.
• Decode: read each value with a bounds-checked read_big_endian, pushed it into a temporary std::vector, then copied that into the map.

Now, after the existing header and size checks:

• Encode copies the map's storage (linear_cbegin()/linear_cend()) straight into the payload with std::uninitialized_copy, constructing each value as a boost::endian::big_uint16_buf_t.
• Decode copies the payload into the map's storage, then converts each value in place with boost::endian::big_to_native_inplace.
• depth_map (xt::xarray<uint16_t>) is always row-major, the encoding's order. A static_assert checks that, and another checks that big_uint16_buf_t is two unaligned bytes.

The encoded bytes and the API are unchanged.

Why

Modules encode a depth frame on every GetImages call that returns depth, and readers decode one per frame. We're reading depth more often, and on a Raspberry Pi CM5 this was several ms per frame.

Measurements

Raspberry Pi Compute Module 5, one core, three runs. Built with Debian bookworm's g++ 12, the image CI uses. Every version produces identical bytes and maps. Median ms per call:

	1280×720 main	1280×720 this PR	848×480 main	848×480 this PR
encode, -O2	2.96–3.34	0.97	1.31–1.47	0.42–0.43
decode, -O2	3.12–4.18	0.95–0.96	1.63–1.64	0.39–0.40
encode, -O3	2.80–3.08	0.53–0.65	1.23–1.36	0.19–0.21
decode, -O3	2.83–2.91	0.30–0.32	1.07–1.10	0.09

Tests

• test_depth_map_encoding_bytes checks the exact encoded bytes for a 2×3 map: the DEPTHMAP magic, big-endian width and height, then big-endian values including 0x00FF, 0xFF00 and 0xFFFF.
• test_depth_map_round_trip_camera_size round-trips an 848×480 map whose values cover the whole 16-bit range.
• test_camera passes 12/12 in the cpp-sdk-system-debian:bookworm image (arm64), built with -Wall -Werror.
• clang-tidy-15 reports nothing on camera.cpp. Formatted with the image's clang-format.

🤖 Generated with Claude Code

[CLAassistant]

All committers have signed the CLA.

[lia (lia-viam)]

can you look into xtensor's documentation, i am pretty sure they have iterator/range helpers to make it so you never have to do the type of manual indexing that is happening here, same with the read loop below

[Jacob Kaplan (qwertey6)]

Done in d652835: both directions now use linear_cbegin()/linear_cend() and linear_begin()/linear_end(), with no indexing.

depth_map is xt::xarray<uint16_t>, whose layout is fixed row-major, so I dropped the other-layout fallback and added a static_assert that depth_map::static_layout is row-major.

Raspberry Pi CM5, 1280×720, median per call:

	main	this PR
encode, -O2	3.0–3.3 ms	0.97 ms
decode, -O2	3.1–4.2 ms	0.95 ms
encode, -O3	2.8–3.1 ms	0.53–0.65 ms
decode, -O3	2.8–2.9 ms	0.30–0.32 ms

[lia (lia-viam)]

like other parts of this file we should be using boost.endian rather than doing these conversions manually, which may result in us removing this helper entirely. in particular it seems like a poor choice to implement the read/write ops on a raw pointer which is assumed to be a buffer which is always two bytes. consider doing this with the boost endian conversion helpers or big_uint16_t

[Jacob Kaplan (qwertey6)]

Done in d652835, and the hand-written helpers are gone.

• Encode constructs each value as a boost::endian::big_uint16_buf_t directly in the payload with std::uninitialized_copy, so the buffer objects are actually created there rather than aliased.
• Decode copies the payload into the map's storage, then converts each value with boost::endian::big_to_native_inplace. That's the same memcpy-then-convert read_big_endian already does for the header.
• A static_assert checks that big_uint16_buf_t is two unaligned bytes.

Decode is now two passes, which costs about 0.18 ms at -O2 against the previous commit (1280×720 on a CM5) and nothing at -O3. Reading through a const big_uint16_buf_t* overlay would keep it to one pass, but those objects don't exist in the input buffer, so I went with the well-defined copy.

[lia (lia-viam)]

minor comment but lgtm

[lia (lia-viam)]

minor but let's prefer to use std::copy here rather than a manual memcpy. for contiguous storage pretty much any standard library implementation will do this as a memmove (see eg https://github.com/google/libcxx/blob/master/include/algorithm#L1517) and we get better bounds/type safety

[Jacob Kaplan (qwertey6)]

Done in 4f8d4e1: std::copy(data.begin() + offset, data.end(), reinterpret_cast<unsigned char*>(m.data())). The source range is now the vector's own [offset, end) rather than a computed byte count. test_camera passes.

Timed on a CM5 (median of 200 runs each, two runs), it matches the memcpy version:

	std::copy	memcpy
1280x720	1.06 / 1.07 ms	0.96 / 1.07 ms
848x480	0.39 / 0.43 ms	0.39 / 0.43 ms

[Ethan (stuqdog)]

looks reasonable to me, CI compilation issues aside

[Jacob Kaplan (qwertey6)]

Ethan (@stuqdog) thanks! The CI failure was from my std::copy change: clang-tidy's bugprone-narrowing-conversions rejects data.begin() + offset with a size_t offset. cd3c6ca bounds the copy with pointers instead (data.data() + offset, data.data() + data.size()), the same pattern the encoder and the earlier memcpy used.

Checked locally in the CI image (cpp-sdk-system-debian:bookworm, amd64) with clang-tidy 15.0.2 and the repo's .clang-tidy at -std=c++14: it reproduces the CI error on 4f8d4e1 and passes on cd3c6ca. test_camera passes.

The workflow runs for cd3c6ca are waiting for a maintainer to approve them.