Lazy loading large assets - #1923
Conversation
|
Warning Review limit reached
Next review available in: 20 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe change adds asset path, size, and load-state tracking. File-backed assets load lazily and serialize in chunks with size validation. Pipeline consumers and Python bindings use asset accessors. Tests cover serialization limits, path-backed assets, and RVC4 model-loading paths. ChangesAsset serialization flow
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested labels: Sequence Diagram(s)sequenceDiagram
participant PipelineImpl
participant AssetManager
participant FileSystem
PipelineImpl->>AssetManager: getSerializedSize(offset)
AssetManager->>FileSystem: validate path and size
FileSystem-->>AssetManager: return asset size
AssetManager-->>PipelineImpl: return serialized capacity
PipelineImpl->>AssetManager: serialize assets
AssetManager->>FileSystem: read file chunks
FileSystem-->>AssetManager: return file data
AssetManager-->>PipelineImpl: write serialized asset data
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates depthai-core’s pipeline asset handling to avoid retaining redundant in-memory copies of large file-backed assets (notably large RVC4 DLCs). It switches path-backed assets to store path/size metadata until serialization time, then streams file contents directly into aggregate asset storage, keeping the on-wire format unchanged while reducing peak memory usage.
Changes:
- Make path-backed
Assets lazy: storepath+size, loaddataonly on explicit request, and stream from disk during serialization. - Pre-compute and pre-reserve total serialized asset storage capacity; update blob/tuning metadata to use
Asset::getSize(). - Add/extend tests for asset size accounting, 4 GiB serialized storage cap behavior, and RVC4 NN model loading via multiple API paths; adjust Python bindings for lazy asset data access and NNArchive model retrieval.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/src/onhost_tests/serialization_test.cpp | Adds unit tests around AssetManager size tracking and the 4 GiB serialized storage limit. |
| tests/src/ondevice_tests/pipeline/node/neural_network_node_test.cpp | Adds an RVC4-focused on-device test covering multiple NN model-loading entrypoints. |
| src/pipeline/Pipeline.cpp | Pre-reserves asset storage using getSerializedSize() and switches tuning/blob size reporting to getSize(); updates resource load path to use getData(). |
| src/pipeline/node/NeuralNetwork.cpp | Uses Asset::getSize() for blob size metadata instead of data.size(). |
| src/pipeline/AssetManager.cpp | Implements lazy path-backed assets, streaming serialization, 4 GiB limit enforcement, and serialized-size precomputation. |
| include/depthai/pipeline/AssetManager.hpp | Extends Asset with path/size and adds getData(), getSize(), and AssetManager::getSerializedSize(). |
| cmake/Depthai/DepthaiDeviceRVC4Config.cmake | Updates the RVC4 device snapshot version identifier. |
| bindings/python/src/pipeline/AssetManagerBindings.cpp | Updates Python Asset.data getter/setter to materialize via getData() and clear path metadata on set. |
| bindings/python/src/nn_archive/NNArchiveBindings.cpp | Changes Python getOtherModelFormat() binding to return bytes/None instead of the default vector conversion. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| std::vector<std::uint8_t>& Asset::getData() { | ||
| if(data.empty() && !path.empty()) { | ||
| std::ifstream stream(path, std::ios::in | std::ios::binary); | ||
| if(!stream.is_open()) { | ||
| throw std::runtime_error(fmt::format("Cannot load asset, file at path {} doesn't exist.", path)); | ||
| } | ||
| data = std::vector<std::uint8_t>(std::istreambuf_iterator<char>(stream), {}); | ||
| } | ||
| return data; | ||
| } |
| std::size_t getSerializedEndOffset(std::size_t offset, std::uint32_t alignment, std::size_t assetSize) { | ||
| if(offset > MAX_ASSET_STORAGE_SIZE || assetSize > MAX_ASSET_STORAGE_SIZE) { | ||
| throw std::runtime_error("Asset storage cannot exceed 4 GiB"); | ||
| } | ||
|
|
||
| std::size_t padding = 0; | ||
| if(alignment > 1 && offset % alignment != 0) { | ||
| padding = alignment - (offset % alignment); | ||
| } | ||
|
|
||
| if(padding > MAX_ASSET_STORAGE_SIZE - offset || assetSize > MAX_ASSET_STORAGE_SIZE - offset - padding) { | ||
| throw std::runtime_error("Asset storage cannot exceed 4 GiB"); | ||
| } | ||
| return offset + padding + assetSize; | ||
| } |
| std::array<std::uint8_t, 1024 * 1024> buffer{}; | ||
| std::size_t streamedSize = 0; | ||
| while(stream) { | ||
| stream.read(reinterpret_cast<char*>(buffer.data()), buffer.size()); | ||
| auto bytesRead = stream.gcount(); | ||
| storage.insert(storage.end(), buffer.data(), buffer.data() + bytesRead); | ||
| streamedSize += static_cast<std::size_t>(bytesRead); | ||
| } |
| nnArchive.def( | ||
| "getOtherModelFormat", | ||
| [](const NNArchive& archive) -> py::object { | ||
| const auto model = archive.getOtherModelFormat(); | ||
| if(!model.has_value()) return py::none(); |
There was a problem hiding this comment.
This change was necessary as std::vector<uint8_t> is declared as opaque so the previous version of the binding didn't work.
| const auto modelData = archive.getOtherModelFormat(); | ||
| REQUIRE(modelData.has_value()); | ||
|
|
||
| const std::filesystem::path directModelPath = std::filesystem::temp_directory_path() / "depthai-rvc4-model-loading-test.dlc"; |
| startPipeline("setModelFromDeviceZoo", [model](const auto& neuralNetwork) { neuralNetwork->setModelFromDeviceZoo(model); }); | ||
| } | ||
|
|
||
| std::filesystem::remove(directModelPath); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pipeline/AssetManager.cpp`:
- Around line 227-242: Bound both path-backed read sites to the declared asset
size: in AssetManager’s serialization flow at
src/pipeline/AssetManager.cpp:227-242, append at most assetSize bytes and then
detect trailing data before appending it; in Asset::getData() at
src/pipeline/AssetManager.cpp:45-59, read at most size bytes and reject short
reads or trailing data before assigning data.
In `@tests/src/ondevice_tests/pipeline/node/neural_network_node_test.cpp`:
- Around line 75-83: Add a scope-bound cleanup guard immediately after creating
directModelPath so the temporary DLC file is removed on every exit path,
including failed assertions and startup exceptions. Ensure the guard removes
directModelPath when the test scope exits, while preserving the existing
file-writing and pipeline logic.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: aba44002-f253-473c-afa3-b8c203f54fa3
📒 Files selected for processing (9)
bindings/python/src/nn_archive/NNArchiveBindings.cppbindings/python/src/pipeline/AssetManagerBindings.cppcmake/Depthai/DepthaiDeviceRVC4Config.cmakeinclude/depthai/pipeline/AssetManager.hppsrc/pipeline/AssetManager.cppsrc/pipeline/Pipeline.cppsrc/pipeline/node/NeuralNetwork.cpptests/src/ondevice_tests/pipeline/node/neural_network_node_test.cpptests/src/onhost_tests/serialization_test.cpp
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-03-24T22:39:04.364Z
Learnt from: MaticTonin
Repo: luxonis/depthai-core PR: 1732
File: src/pipeline/Pipeline.cpp:705-705
Timestamp: 2026-03-24T22:39:04.364Z
Learning: Do not flag the `!= ""` part of the auto-calibration condition as redundant when it appears in `PipelineImpl::build()` (or closely related pipeline build logic). If the code uses `utility::getEnvAs<std::string>(..., default)` with a default such as `"ON_START"`, the explicit empty-string guard may still be intentional to treat an explicitly empty env var as “OFF/disabled” (or to avoid special-casing elsewhere). Only consider removing `!= ""` if the codebase has an explicit, enforceable guarantee that `DEPTHAI_AUTOCALIBRATION` can never be set to an empty string (e.g., via validated parsing/CI checks); otherwise, keep the guard.
Applied to files:
src/pipeline/node/NeuralNetwork.cppsrc/pipeline/Pipeline.cppsrc/pipeline/AssetManager.cpp
🪛 Cppcheck (2.21.0)
src/pipeline/AssetManager.cpp
[error] 28-28: There is an unknown macro here somewhere. Configuration is required. If DEPTHAI_NLOHMANN_DEFINE_TYPE_INTRUSIVE is a macro then please configure it.
(unknownMacro)
🔇 Additional comments (10)
src/pipeline/node/NeuralNetwork.cpp (1)
232-232: LGTM!bindings/python/src/pipeline/AssetManagerBindings.cpp (1)
35-42: LGTM!bindings/python/src/nn_archive/NNArchiveBindings.cpp (1)
82-89: LGTM!tests/src/ondevice_tests/pipeline/node/neural_network_node_test.cpp (2)
3-8: LGTM!
85-112: LGTM!cmake/Depthai/DepthaiDeviceRVC4Config.cmake (1)
6-6: LGTM!include/depthai/pipeline/AssetManager.hpp (1)
20-25: LGTM!Also applies to: 135-135
src/pipeline/AssetManager.cpp (1)
19-37: LGTM!Also applies to: 109-111, 128-129, 138-138, 252-257
src/pipeline/Pipeline.cpp (1)
302-306: LGTM!Also applies to: 596-613, 1397-1409
tests/src/onhost_tests/serialization_test.cpp (1)
2-2: LGTM!Also applies to: 52-81
| if(!a.path.empty()) { | ||
| std::ifstream stream(a.path, std::ios::in | std::ios::binary); | ||
| if(!stream.is_open()) { | ||
| throw std::runtime_error(fmt::format("Cannot load asset, file at path {} doesn't exist.", a.path)); | ||
| } | ||
| std::vector<std::uint8_t> buffer(1024 * 1024); | ||
| std::size_t streamedSize = 0; | ||
| while(stream) { | ||
| stream.read(reinterpret_cast<char*>(buffer.data()), buffer.size()); | ||
| auto bytesRead = stream.gcount(); | ||
| storage.insert(storage.end(), buffer.data(), buffer.data() + bytesRead); | ||
| streamedSize += static_cast<std::size_t>(bytesRead); | ||
| } | ||
| if(streamedSize != a.getSize()) { | ||
| throw std::runtime_error(fmt::format("Asset at path {} changed while serializing.", a.path)); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Limit reads to the declared asset size before validation.
If a path-backed file grows after registration, Line 237 appends all new bytes before Line 240 detects the change. This can exceed the 4 GiB storage limit or exhaust host memory before the error is raised. Asset::getData() has the same unbounded-read behavior.
src/pipeline/AssetManager.cpp#L227-L242: read no more thanassetSize, then detect trailing data before appending it.src/pipeline/AssetManager.cpp#L45-L59: load no more thansize, then reject short reads or trailing data before assigningdata.
📍 Affects 1 file
src/pipeline/AssetManager.cpp#L227-L242(this comment)src/pipeline/AssetManager.cpp#L45-L59
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pipeline/AssetManager.cpp` around lines 227 - 242, Bound both path-backed
read sites to the declared asset size: in AssetManager’s serialization flow at
src/pipeline/AssetManager.cpp:227-242, append at most assetSize bytes and then
detect trailing data before appending it; in Asset::getData() at
src/pipeline/AssetManager.cpp:45-59, read at most size bytes and reject short
reads or trailing data before assigning data.
There was a problem hiding this comment.
Yes! in addition I would add a try / catch so in the case of corruption of change of asset path, the storage can be resized back to starting storage size in the catch.
There was a problem hiding this comment.
@aljazkonec1, agreed.
AssetManager::serialize should save const auto initialStorageSize = storage.size() before it modifies storage. If streaming or validation throws, a catch block should call storage.resize(initialStorageSize) and then rethrow.
The implementation should also limit each read to assetSize. It must detect a short read or trailing data before it appends bytes beyond the declared size.
This keeps storage unchanged when serialization fails. 🐇
You are interacting with an AI system.
| const std::filesystem::path directModelPath = | ||
| std::filesystem::temp_directory_path() | ||
| / ("depthai-rvc4-model-loading-test_" + std::to_string(std::chrono::steady_clock::now().time_since_epoch().count()) + ".dlc"); | ||
| { | ||
| std::ofstream modelFile(directModelPath, std::ios::binary | std::ios::trunc); | ||
| REQUIRE(modelFile.is_open()); | ||
| modelFile.write(reinterpret_cast<const char*>(modelData->data()), static_cast<std::streamsize>(modelData->size())); | ||
| REQUIRE(modelFile.good()); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Remove the temporary model file on every exit path.
Line 114 runs only after all assertions and pipeline starts succeed. A failed REQUIRE or startup exception can bypass that cleanup and retain a large DLC file. Create a scope-bound cleanup guard immediately after directModelPath is created.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/src/ondevice_tests/pipeline/node/neural_network_node_test.cpp` around
lines 75 - 83, Add a scope-bound cleanup guard immediately after creating
directModelPath so the temporary DLC file is removed on every exit path,
including failed assertions and startup exceptions. Ensure the guard removes
directModelPath when the test scope exits, while preserving the existing
file-writing and pipeline logic.
| std::filesystem::path path; | ||
| std::size_t size = 0; | ||
| bool dataLoaded = false; |
There was a problem hiding this comment.
Should these fields be public? If they are exposed, each one can be independently mutable so it could be corrupted.
|
|
||
| /// Serializes | ||
| void serialize(AssetsMutable& assets, std::vector<std::uint8_t>& assetStorage, std::string prefix = "") const; | ||
| std::size_t getSerializedSize(std::size_t offset = 0) const; |
| if(!a.path.empty()) { | ||
| std::ifstream stream(a.path, std::ios::in | std::ios::binary); | ||
| if(!stream.is_open()) { | ||
| throw std::runtime_error(fmt::format("Cannot load asset, file at path {} doesn't exist.", a.path)); | ||
| } | ||
| std::vector<std::uint8_t> buffer(1024 * 1024); | ||
| std::size_t streamedSize = 0; | ||
| while(stream) { | ||
| stream.read(reinterpret_cast<char*>(buffer.data()), buffer.size()); | ||
| auto bytesRead = stream.gcount(); | ||
| storage.insert(storage.end(), buffer.data(), buffer.data() + bytesRead); | ||
| streamedSize += static_cast<std::size_t>(bytesRead); | ||
| } | ||
| if(streamedSize != a.getSize()) { | ||
| throw std::runtime_error(fmt::format("Asset at path {} changed while serializing.", a.path)); | ||
| } |
There was a problem hiding this comment.
Yes! in addition I would add a try / catch so in the case of corruption of change of asset path, the storage can be resized back to starting storage size in the catch.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/pipeline/AssetManager.cpp (2)
231-254: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winReserve the validated final capacity before streaming.
Line 231 calculates the required end offset but does not reserve it. Chunked
insertcalls can reallocatestoragerepeatedly. For a large aggregate, a reallocation can temporarily duplicate most of the aggregate buffer and cause allocation failure. Store the validated end offset and callstorage.reserve(endOffset)before resizing or streaming.Proposed fix
- getSerializedEndOffset(storage.size(), a.alignment, assetSize); + const auto storageEnd = getSerializedEndOffset(storage.size(), a.alignment, assetSize); + storage.reserve(storageEnd);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pipeline/AssetManager.cpp` around lines 231 - 254, Update the asset serialization flow around getSerializedEndOffset to store its validated final end offset, then call storage.reserve(endOffset) before storage.resize and any chunked streaming inserts. Use that stored value for the existing offset calculation as appropriate, preserving the current alignment and asset-copy behavior.
219-269: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftRoll back the complete serialization transaction.
Line 223 records the start position for one asset only. If a later path-backed asset fails, earlier asset bytes remain in
storage. EarliermutableAssetsentries also remain. Capture the initial state before the loop. Stage metadata or restore it in the outer catch. Add a test with a valid first asset and a modified second asset.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pipeline/AssetManager.cpp` around lines 219 - 269, Make asset serialization transactional across the entire loop: capture the initial storage size and mutableAssets state before iterating in the serialization function, then restore both in an outer catch if any asset fails. Do not rely on the per-asset storageStart rollback alone; ensure earlier bytes and mutableAssets entries are removed when a later path-backed asset throws. Add coverage for a valid first asset followed by a modified second asset, verifying complete rollback.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/pipeline/AssetManager.cpp`:
- Around line 231-254: Update the asset serialization flow around
getSerializedEndOffset to store its validated final end offset, then call
storage.reserve(endOffset) before storage.resize and any chunked streaming
inserts. Use that stored value for the existing offset calculation as
appropriate, preserving the current alignment and asset-copy behavior.
- Around line 219-269: Make asset serialization transactional across the entire
loop: capture the initial storage size and mutableAssets state before iterating
in the serialization function, then restore both in an outer catch if any asset
fails. Do not rely on the per-asset storageStart rollback alone; ensure earlier
bytes and mutableAssets entries are removed when a later path-backed asset
throws. Add coverage for a valid first asset followed by a modified second
asset, verifying complete rollback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2978b287-d0a3-4954-86bb-37f1477178fb
📒 Files selected for processing (2)
src/pipeline/AssetManager.cpptests/src/onhost_tests/serialization_test.cpp
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-03-24T22:39:04.364Z
Learnt from: MaticTonin
Repo: luxonis/depthai-core PR: 1732
File: src/pipeline/Pipeline.cpp:705-705
Timestamp: 2026-03-24T22:39:04.364Z
Learning: Do not flag the `!= ""` part of the auto-calibration condition as redundant when it appears in `PipelineImpl::build()` (or closely related pipeline build logic). If the code uses `utility::getEnvAs<std::string>(..., default)` with a default such as `"ON_START"`, the explicit empty-string guard may still be intentional to treat an explicitly empty env var as “OFF/disabled” (or to avoid special-casing elsewhere). Only consider removing `!= ""` if the codebase has an explicit, enforceable guarantee that `DEPTHAI_AUTOCALIBRATION` can never be set to an empty string (e.g., via validated parsing/CI checks); otherwise, keep the guard.
Applied to files:
src/pipeline/AssetManager.cpp
🔇 Additional comments (4)
src/pipeline/AssetManager.cpp (3)
9-76: LGTM!
116-151: LGTM!
273-278: LGTM!tests/src/onhost_tests/serialization_test.cpp (1)
2-105: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
tests/src/onhost_tests/serialization_test.cpp (2)
85-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTemporary files leak when an assertion fails. Both tests remove their temporary file with a trailing
std::filesystem::removecall. Catch2 throws on a failedREQUIRE, so the removal is skipped and the file stays in the shared temp directory under a fixed name. The shared root cause is cleanup placed as a trailing statement instead of a scope-bound action.
tests/src/onhost_tests/serialization_test.cpp#L85-L105: replace the trailingstd::filesystem::remove(path)at Line 104 with a scope guard, or move the file creation and removal into a Catch2 fixture whose destructor removes the file.tests/src/onhost_tests/serialization_test.cpp#L107-L132: apply the same scope guard or fixture to the removal at Line 131.🧹 Proposed scope guard
+namespace { +struct TempFile { + std::filesystem::path path; + explicit TempFile(const std::string& name) : path(std::filesystem::temp_directory_path() / name) {} + ~TempFile() { + std::error_code ec; + std::filesystem::remove(path, ec); + } +}; +} // namespaceThen each test declares
TempFile temp("depthai_asset_manager_serialization_test.bin");, usestemp.path, and drops the trailingstd::filesystem::removecall. Using a unique name per test, as the current code already does, keeps the two tests independent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/src/onhost_tests/serialization_test.cpp` around lines 85 - 105, Ensure both serialization tests in tests/src/onhost_tests/serialization_test.cpp (anchor lines 85-105 and sibling lines 107-132) use scope-bound temporary-file cleanup instead of trailing std::filesystem::remove calls. Update each test to use the existing or introduced TempFile guard, access its path through temp.path, and remove the trailing cleanup so destruction occurs even when REQUIRE fails.
63-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd cases for the accumulated-overflow and zero-alignment branches.
This test covers only the single-asset check at
getSerializedEndOffsetLine 25, where one asset alone exceeds the limit. It does not cover the accumulation check at Line 34, where each asset fits but the runningoffsetplus padding plusassetSizeexceeds the limit. Line 34 holds the subtraction-based overflow arithmetic, so it carries the highest risk. The zero-alignment rejection at Line 21 is also untested.🧪 Proposed additional cases
+TEST_CASE("AssetManager rejects accumulated storage beyond 4 GiB") { + dai::AssetManager assetManager; + for(int i = 0; i < 2; ++i) { + dai::Asset asset("asset" + std::to_string(i)); + asset.path = "placeholder"; + asset.size = static_cast<std::size_t>(std::numeric_limits<std::uint32_t>::max()) / 2 + 1; + assetManager.set(std::move(asset)); + } + + REQUIRE_THROWS_WITH(assetManager.getSerializedSize(), "Asset storage cannot exceed 4 GiB"); +} + +TEST_CASE("AssetManager rejects zero alignment") { + dai::Asset asset("unaligned"); + asset.path = "placeholder"; + asset.size = 1; + asset.alignment = 0; + + dai::AssetManager assetManager; + assetManager.set(std::move(asset)); + + REQUIRE_THROWS_WITH(assetManager.getSerializedSize(), "Asset alignment cannot be zero"); +}The first case needs
<string>forstd::to_string.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/src/onhost_tests/serialization_test.cpp` around lines 63 - 72, Extend the serialization tests around AssetManager::getSerializedSize with cases where multiple individually valid assets overflow the accumulated offset after padding, using generated asset paths or names as needed, and assert the existing 4 GiB error. Also add a case that exercises zero alignment and verifies its rejection behavior, including the required <string> dependency for std::to_string.src/pipeline/AssetManager.cpp (1)
46-76: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winKeep path-backed and in-memory asset data consistent
Assetexposes mutabledataandgetData(). If a caller modifies a path-backed asset, serialization still reads the file becausepathremains non-empty. Clearpathand setsize = data.size()when replacing the data, or make path-backed data immutable. Add a regression test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pipeline/AssetManager.cpp` around lines 46 - 76, Ensure mutable data changes to an Asset remain authoritative during serialization: update the data replacement path so modifying a path-backed asset clears path and synchronizes size with data.size(), or enforce immutability for path-backed data. Preserve normal lazy loading in getData() and add a regression test covering modification of a path-backed asset followed by serialization.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@include/depthai/pipeline/AssetManager.hpp`:
- Around line 19-30: Make Asset’s path and size state private, alongside
dataLoaded, and add or use a setter that updates both values together so they
cannot become inconsistent. Update AssetManager and related callers to use this
setter while preserving getSize(), getData(), and serialize behavior.
- Around line 139-140: Update the getSerializedSize documentation to describe
how the offset parameter affects the serialized-size calculation and explicitly
document that it throws when alignment is zero or the storage size exceeds 4
GiB.
In `@src/pipeline/AssetManager.cpp`:
- Around line 219-235: Compute the final serialized end offset once before
entering the asset-processing loop using getSerializedSize(storage.size()), and
reserve that total capacity before any storage mutation. Remove the per-asset
storage.reserve call while retaining getSerializedEndOffset inside the loop to
bound each asset after size changes.
- Around line 228-234: Extract the alignment-padding calculation into a shared
helper, such as getAlignmentPadding, and replace the duplicate formula in both
getSerializedEndOffset and serialize. In serialize, call getSerializedEndOffset
before the helper so zero-alignment validation remains enforced, and use the
shared padding when writing storage and recording mutableAssets offsets.
In `@tests/src/onhost_tests/serialization_test.cpp`:
- Around line 114-116: Add a concise comment immediately before the AssetManager
setup in the serialization test explaining that assetMap is key-ordered,
requiring "first" to serialize before "second" so the rollback test exercises
multiple assets; preserve the existing keys and test behavior.
---
Outside diff comments:
In `@src/pipeline/AssetManager.cpp`:
- Around line 46-76: Ensure mutable data changes to an Asset remain
authoritative during serialization: update the data replacement path so
modifying a path-backed asset clears path and synchronizes size with
data.size(), or enforce immutability for path-backed data. Preserve normal lazy
loading in getData() and add a regression test covering modification of a
path-backed asset followed by serialization.
In `@tests/src/onhost_tests/serialization_test.cpp`:
- Around line 85-105: Ensure both serialization tests in
tests/src/onhost_tests/serialization_test.cpp (anchor lines 85-105 and sibling
lines 107-132) use scope-bound temporary-file cleanup instead of trailing
std::filesystem::remove calls. Update each test to use the existing or
introduced TempFile guard, access its path through temp.path, and remove the
trailing cleanup so destruction occurs even when REQUIRE fails.
- Around line 63-72: Extend the serialization tests around
AssetManager::getSerializedSize with cases where multiple individually valid
assets overflow the accumulated offset after padding, using generated asset
paths or names as needed, and assert the existing 4 GiB error. Also add a case
that exercises zero alignment and verifies its rejection behavior, including the
required <string> dependency for std::to_string.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e1b28cfb-8110-40c2-acd8-125d0a0d2d44
📒 Files selected for processing (5)
cmake/Depthai/DepthaiDeviceRVC4Config.cmakeinclude/depthai/pipeline/AssetManager.hppsrc/pipeline/AssetManager.cpptests/src/ondevice_tests/pipeline/node/neural_network_node_test.cpptests/src/onhost_tests/serialization_test.cpp
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-03-24T22:39:04.364Z
Learnt from: MaticTonin
Repo: luxonis/depthai-core PR: 1732
File: src/pipeline/Pipeline.cpp:705-705
Timestamp: 2026-03-24T22:39:04.364Z
Learning: Do not flag the `!= ""` part of the auto-calibration condition as redundant when it appears in `PipelineImpl::build()` (or closely related pipeline build logic). If the code uses `utility::getEnvAs<std::string>(..., default)` with a default such as `"ON_START"`, the explicit empty-string guard may still be intentional to treat an explicitly empty env var as “OFF/disabled” (or to avoid special-casing elsewhere). Only consider removing `!= ""` if the codebase has an explicit, enforceable guarantee that `DEPTHAI_AUTOCALIBRATION` can never be set to an empty string (e.g., via validated parsing/CI checks); otherwise, keep the guard.
Applied to files:
src/pipeline/AssetManager.cpp
🪛 Cppcheck (2.21.0)
tests/src/onhost_tests/serialization_test.cpp
[error] 120-120: There is an unknown macro here somewhere. Configuration is required. If DEPTHAI_NLOHMANN_DEFINE_TYPE_INTRUSIVE is a macro then please configure it.
(unknownMacro)
tests/src/ondevice_tests/pipeline/node/neural_network_node_test.cpp
[error] 120-120: There is an unknown macro here somewhere. Configuration is required. If DEPTHAI_NLOHMANN_DEFINE_TYPE_INTRUSIVE is a macro then please configure it.
(unknownMacro)
🔇 Additional comments (13)
tests/src/ondevice_tests/pipeline/node/neural_network_node_test.cpp (2)
89-97: RemovedirectModelPathon every test exit path.A failed
REQUIREor a pipeline-start exception bypasses the removal at lines 130-131. Add a scope-bound cleanup guard immediately after the file is created.
75-88: LGTM!Also applies to: 99-123
cmake/Depthai/DepthaiDeviceRVC4Config.cmake (1)
6-6: 🗄️ Data Integrity & IntegrationVerify the paired RVC4 snapshot before merge.
Confirm that
0.0.1+91a462a582fcb01dbd7e95b79cdea101beed5780resolves in CI and includes the requireddepthai-device-kbcompanion change. Otherwise standalone RVC4 model startup can fail after configuration succeeds.src/pipeline/AssetManager.cpp (6)
16-40: LGTM!
116-125: LGTM!
139-140: LGTM!
149-149: LGTM!
243-279: 📐 Maintainability & Code QualityRollback and bounded reads look correct.
The streaming loop reads at most
assetSizebytes, rejects a short read, and rejects trailing data. The outer catch restores bothstorageandmutableAssets. This resolves the earlier requests to bound the read and to restore the starting storage size on failure.One point to confirm: the inner catch at lines 264-267 resizes to
assetStorageStart, and the outer catch then resizes tostorageStart, which is always less than or equal toassetStorageStart. The inner catch is therefore redundant. Removing it would simplify the flow, but it is harmless as written.
282-288: LGTM!tests/src/onhost_tests/serialization_test.cpp (4)
2-4: LGTM!
54-61: LGTM!
74-83: LGTM!
122-129: 🗄️ Data Integrity & IntegrationNo change needed:
dai::Assetsdeclareshas
AssetsMutablecan use the inheriteddai::Assets::has(const std::string&)method.> Likely an incorrect or invalid review comment.
| /// Calculates the size of the serialized data | ||
| std::size_t getSerializedSize(std::size_t offset = 0) const; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Document the offset parameter and the throw behavior.
The docstring answers the earlier request. It does not state what offset means, nor that the function throws when alignment is zero or when the storage exceeds 4 GiB. Callers cannot see this from the signature.
📝 Proposed docstring
- /// Calculates the size of the serialized data
+ /**
+ * Calculates the size of the serialized asset data.
+ *
+ * `@param` offset Starting offset in the aggregate asset storage
+ * `@returns` End offset after all assets, including alignment padding
+ * `@throws` std::runtime_error If an asset alignment is zero, or if the storage would exceed 4 GiB
+ */
std::size_t getSerializedSize(std::size_t offset = 0) const;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Calculates the size of the serialized data | |
| std::size_t getSerializedSize(std::size_t offset = 0) const; | |
| /** | |
| * Calculates the size of the serialized asset data. | |
| * | |
| * `@param` offset Starting offset in the aggregate asset storage | |
| * `@returns` End offset after all assets, including alignment padding | |
| * `@throws` std::runtime_error If an asset alignment is zero, or if the storage would exceed 4 GiB | |
| */ | |
| std::size_t getSerializedSize(std::size_t offset = 0) const; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@include/depthai/pipeline/AssetManager.hpp` around lines 139 - 140, Update the
getSerializedSize documentation to describe how the offset parameter affects the
serialized-size calculation and explicitly document that it throws when
alignment is zero or the storage size exceeds 4 GiB.
| // Calculate additional bytes needed to offset to alignment. | ||
| std::size_t toAdd = 0; | ||
| if(a.alignment > 1 && storage.size() % a.alignment != 0) { | ||
| toAdd = a.alignment - (storage.size() % a.alignment); | ||
| } | ||
|
|
||
| const auto storageEnd = getSerializedEndOffset(storage.size(), a.alignment, assetSize); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove the duplicated alignment-padding formula.
Lines 229-232 recompute the padding that getSerializedEndOffset already computes at lines 29-32. The two copies must stay identical. If one changes, offset recorded in mutableAssets at Line 273 no longer matches the bytes written to storage, and the mismatch is silent.
Extract the padding into a shared helper and use it in both places.
♻️ Proposed refactor
+std::size_t getAlignmentPadding(std::size_t offset, std::uint32_t alignment) {
+ if(alignment > 1 && offset % alignment != 0) {
+ return alignment - (offset % alignment);
+ }
+ return 0;
+}
+
std::size_t getSerializedEndOffset(std::size_t offset, std::uint32_t alignment, std::size_t assetSize) {
if(alignment == 0) {
throw std::runtime_error("Asset alignment cannot be zero");
}
if(offset > MAX_ASSET_STORAGE_SIZE || assetSize > MAX_ASSET_STORAGE_SIZE) {
throw std::runtime_error("Asset storage cannot exceed 4 GiB");
}
- std::size_t padding = 0;
- if(alignment > 1 && offset % alignment != 0) {
- padding = alignment - (offset % alignment);
- }
+ const std::size_t padding = getAlignmentPadding(offset, alignment);Then in serialize:
- // Calculate additional bytes needed to offset to alignment.
- std::size_t toAdd = 0;
- if(a.alignment > 1 && storage.size() % a.alignment != 0) {
- toAdd = a.alignment - (storage.size() % a.alignment);
- }
-
const auto storageEnd = getSerializedEndOffset(storage.size(), a.alignment, assetSize);
+ // Calculate additional bytes needed to offset to alignment.
+ const std::size_t toAdd = getAlignmentPadding(storage.size(), a.alignment);
storage.reserve(storageEnd);Note that getSerializedEndOffset must run first, because it rejects a zero alignment that getAlignmentPadding does not check.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Calculate additional bytes needed to offset to alignment. | |
| std::size_t toAdd = 0; | |
| if(a.alignment > 1 && storage.size() % a.alignment != 0) { | |
| toAdd = a.alignment - (storage.size() % a.alignment); | |
| } | |
| const auto storageEnd = getSerializedEndOffset(storage.size(), a.alignment, assetSize); | |
| const auto storageEnd = getSerializedEndOffset(storage.size(), a.alignment, assetSize); | |
| // Calculate additional bytes needed to offset to alignment. | |
| const std::size_t toAdd = getAlignmentPadding(storage.size(), a.alignment); | |
| storage.reserve(storageEnd); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pipeline/AssetManager.cpp` around lines 228 - 234, Extract the
alignment-padding calculation into a shared helper, such as getAlignmentPadding,
and replace the duplicate formula in both getSerializedEndOffset and serialize.
In serialize, call getSerializedEndOffset before the helper so zero-alignment
validation remains enforced, and use the shared padding when writing storage and
recording mutableAssets offsets.
| dai::AssetManager assetManager; | ||
| assetManager.set("first", std::vector<std::uint8_t>{1, 2}); | ||
| assetManager.set("second", path); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Document the dependency on map key order.
This test only exercises the mutableAssets rollback because assetMap is a std::map ordered by key, so "first" serializes before "second" throws. If someone renames the keys, the failing asset can become the first one processed, and the test silently degrades to the single-asset case already covered at lines 85-105.
Add a comment that states the ordering requirement.
📝 Proposed comment
dai::AssetManager assetManager;
+ // assetMap is a std::map, so assets serialize in key order. "first" must
+ // serialize successfully before "second" fails, to exercise the rollback
+ // of already-recorded assets.
assetManager.set("first", std::vector<std::uint8_t>{1, 2});
assetManager.set("second", path);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| dai::AssetManager assetManager; | |
| assetManager.set("first", std::vector<std::uint8_t>{1, 2}); | |
| assetManager.set("second", path); | |
| dai::AssetManager assetManager; | |
| // assetMap is a std::map, so assets serialize in key order. "first" must | |
| // serialize successfully before "second" fails, to exercise the rollback | |
| // of already-recorded assets. | |
| assetManager.set("first", std::vector<std::uint8_t>{1, 2}); | |
| assetManager.set("second", path); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/src/onhost_tests/serialization_test.cpp` around lines 114 - 116, Add a
concise comment immediately before the AssetManager setup in the serialization
test explaining that assetMap is key-ordered, requiring "first" to serialize
before "second" so the rollback test exercises multiple assets; preserve the
existing keys and test behavior.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/pipeline/AssetManager.cpp (2)
46-47: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winInvalidate lazy-load state when the backing file changes.
getData()usesdata.empty()as part of the load decision. BecauseAsset::datais public, non-empty data can bypass loading whiledataLoadedis false.setFile()also leavesdataLoadedunchanged. After replacing a loaded path,getData()can return old bytes whileserialize()reads the new path.Use
dataLoadedas the load-state source. Reset it insetFile(). Load when!dataLoaded && !path.empty(). Preserve the source state inAssetManager::set(const std::string&, Asset).Suggested state fix
void Asset::setFile(std::filesystem::path path, std::size_t size) { this->path = std::move(path); this->size = size; + dataLoaded = false; } - if(data.empty() && !dataLoaded && !path.empty()) { + if(!dataLoaded && !path.empty()) {Also applies to: 78-81
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pipeline/AssetManager.cpp` around lines 46 - 47, Update Asset::getData() to use dataLoaded as the sole load-state check, loading whenever !dataLoaded and path is non-empty rather than relying on data.empty(). Reset dataLoaded in Asset::setFile() when replacing the backing path, and preserve the source asset’s dataLoaded state in AssetManager::set(const std::string&, Asset) instead of overwriting it.
64-65: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDetect same-size changes to path-backed files.
Registration records only
file_size. Lazy loading and serialization only detect short reads or trailing bytes. An in-place rewrite with the same size therefore passes and changes the serialized model bytes without an error.Use a reliable file identity or content-version check that detects same-size replacements. Add a regression test for a same-size rewrite. This follows the PR requirement that path-backed files remain unchanged until serialization.
Also applies to: 144-144, 265-266
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pipeline/AssetManager.cpp` around lines 64 - 65, Extend the path-backed asset tracking around the stream size check to record and validate a reliable file identity or content version, not just file_size. Apply the validation during lazy loading and serialization so same-size in-place rewrites are rejected before producing model bytes, while preserving existing short-read and trailing-byte checks. Add a regression test that rewrites a registered file with different contents but identical size and verifies serialization fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/pipeline/AssetManager.cpp`:
- Around line 46-47: Update Asset::getData() to use dataLoaded as the sole
load-state check, loading whenever !dataLoaded and path is non-empty rather than
relying on data.empty(). Reset dataLoaded in Asset::setFile() when replacing the
backing path, and preserve the source asset’s dataLoaded state in
AssetManager::set(const std::string&, Asset) instead of overwriting it.
- Around line 64-65: Extend the path-backed asset tracking around the stream
size check to record and validate a reliable file identity or content version,
not just file_size. Apply the validation during lazy loading and serialization
so same-size in-place rewrites are rejected before producing model bytes, while
preserving existing short-read and trailing-byte checks. Add a regression test
that rewrites a registered file with different contents but identical size and
verifies serialization fails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 17fd3268-824e-42f8-9def-e42898061d89
📒 Files selected for processing (4)
bindings/python/src/pipeline/AssetManagerBindings.cppinclude/depthai/pipeline/AssetManager.hppsrc/pipeline/AssetManager.cpptests/src/onhost_tests/serialization_test.cpp
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-03-24T22:39:04.364Z
Learnt from: MaticTonin
Repo: luxonis/depthai-core PR: 1732
File: src/pipeline/Pipeline.cpp:705-705
Timestamp: 2026-03-24T22:39:04.364Z
Learning: Do not flag the `!= ""` part of the auto-calibration condition as redundant when it appears in `PipelineImpl::build()` (or closely related pipeline build logic). If the code uses `utility::getEnvAs<std::string>(..., default)` with a default such as `"ON_START"`, the explicit empty-string guard may still be intentional to treat an explicitly empty env var as “OFF/disabled” (or to avoid special-casing elsewhere). Only consider removing `!= ""` if the codebase has an explicit, enforceable guarantee that `DEPTHAI_AUTOCALIBRATION` can never be set to an empty string (e.g., via validated parsing/CI checks); otherwise, keep the guard.
Applied to files:
src/pipeline/AssetManager.cpp
🔇 Additional comments (4)
bindings/python/src/pipeline/AssetManagerBindings.cpp (1)
35-41: LGTM!include/depthai/pipeline/AssetManager.hpp (1)
25-33: LGTM!src/pipeline/AssetManager.cpp (1)
9-40: LGTM!Also applies to: 125-127, 153-153, 286-291
tests/src/onhost_tests/serialization_test.cpp (1)
2-103: LGTM!
Purpose
The 2.08 GB FP16 vision DLC is large enough to expose redundant asset copies in the RVC4 pipeline path. A 1,054 MB INT8 model works, while the 2,084 MB FP16 model reaches the memory limit on a 7.2 GiB OAK4 device. In peripheral mode, the host retains the host-side copies and the device has its own copies, so the run survives; in standalone mode, the host library runs on the camera and all four full-size copies compete for the same RAM.
Before the patch, registering a DLC path read the full file into an
Asset::datavector, and serialization then appended another full copy to aggregate asset storage. The patched path retains only path/size metadata until serialization, then streams directly into aggregate storage, so the file is read once, straight into the buffer that was going to be transferred anyway.This PR is the
depthai-corehalf of the fix. It removes the host-side duplicate; the companiondepthai-device-kbpatch removes the device-side one. Both are required for the standalone result.Specification
Avoid retaining a second full-size in-memory copy of file-backed pipeline assets.
AssetManager::set(key, path)now stores the source path and size instead of eagerly loading the entire file. During pipeline serialization, path-backed assets are streamed directly into aggregate asset storage in 1 MiB chunks. Asset data is loaded lazily only when a caller explicitly requests it.Pipeline serialization pre-reserves the required asset-storage capacity, and blob/tuning metadata now uses the recorded asset size. The serialized wire format is unchanged; its existing
uint32_toffset/size limit (approximately 4 GiB aggregate asset storage) remains.This addresses large RVC4 DLCs where eager loading created an unnecessary additional full-size model buffer.
Dependencies & Potential Impact
The full standalone OAK4/RVC4 memory improvement requires the companion
depthai-device-kbchange, which avoids an additional device-side copy when passing the model to SNPE.This change alters the behavior of path-based assets:
pipeline.start().setModelPath().No wire-format or protocol change is introduced. Existing buffer-backed assets retain their behavior.
Deployment Plan
Release this change together with the companion RVC4 device-firmware change where large-DLC standalone support is required.
For applications using
setModelPath()or other path-backed assets, ensure models are fully written before registration and remain available until pipeline startup. Prefer writing to a temporary file and atomically renaming it into place.Rollback consists of reverting this change and, if deployed together, the companion device-firmware change. Monitor pipeline-start failures, startup duration, and device memory pressure/OOM events for large-model workloads.
Testing & Validation
Validated with the 2,083,613,027-byte FP16 vision DLC on an OAK4-D R9 running Luxonis OS RVC4 1.35.0.
(1, 256, 1152)output; peak process memory was approximately 2.0–2.1 GiB.VmHWMfrom approximately 6,138 MiB to 2,242 MiB.git diff --checkpasses.No dedicated unit tests were added. The reproduction commands, measurements, and test environment are documented in the large-DLC repro repository.
AI Usage
Assisted-by: ClaudaCode Fable 5 - XHigh
Submitted code was reviewed by a human: YES - to the best of my ability considering my depthai-core library knowledge
The author is taking the responsibility for the contribution: YES
Summary by CodeRabbit
New Features
bytesorNone.Bug Fixes