Skip to content

feat(venc): expose video0.maxIpProp, the RC I-to-P size cap the firmware honours - #111

Open
vertexodessa wants to merge 1 commit into
OpenIPC:masterfrom
vertexodessa:feature/max-ip-prop
Open

feat(venc): expose video0.maxIpProp, the RC I-to-P size cap the firmware honours#111
vertexodessa wants to merge 1 commit into
OpenIPC:masterfrom
vertexodessa:feature/max-ip-prop

Conversation

@vertexodessa

@vertexodessa vertexodessa commented Aug 22, 2026

Copy link
Copy Markdown

Summary

Exposes video0.maxIpProp (canonical video0.max_ip_prop): the u32MaxIPProp field of MI_VENC_RcParam_t (stParamH265Cbr / stParamH264Cbr), the CBR rate controller's cap on I-frame size as a proportion of P frames.

Motivation: the per-frame byte caps maxIBytes/maxPBytes are inert on Star6E firmware. Probed live 2026-08-22 under H.265 CBR at 10 Mbps, IDRs measured an identical 42-44 KB with the caps at 2000, 26000, and 8, and switching RC priority changed nothing. qpDelta barely moves IDR size either (43 KB at 0 and at -6) because the CBR RC owns the I/P QP relationship. u32MaxIPProp is the parameter the SigmaStar CBR RC actually honours; waybeam never set it, so the permissive SDK default is why recovery IDRs land at ~3x the P-frame budget and occupy ~60 ms of air at 10 Mbps.

Behaviour

  • Live on Star6E and Maruko via /set and /live/set, through a new LIVE_GROUP_MAX_IP_PROP. Applied at boot when persisted.
  • Range 0..100, CBR only. Validated in validate_field_cfg() like minQp/maxQp: values above 100 and a non-zero value under a non-CBR rc_mode are rejected with 409 on /set, /live/set, and at config load, so the backend apply never sees them (and a negative JSON value, which the loader wraps large, is caught the same way).
  • 0 = SDK default. The first apply captures the driver's value so 0 can restore it live, mirroring the minQp/maxQp pattern. The captured default lives in each backend's existing control context struct, not a new file-scope global.
  • CV610 reports the field unsupported.
  • Contract 0.18.2 -> 0.18.3 (additive): table row + changelog entry in HTTP_API_CONTRACT.md, /api/v1/version string bumped.
  • config/waybeam.default.json gains "maxIpProp": 0 so the save-layout byte-equal test keeps policing the printer.
  • VERSION 0.65.2 -> 0.66.0, HISTORY.md entry.

Verification

  • make test: 2491 passed, 0 failed (adds a loader validation table for maxIpProp)
  • make build SOC_BUILD=star6e and SOC_BUILD=maruko: clean
  • Star6E apply path exercised live on the bench link (the same session that established the byte caps are dead). Maruko apply is the same RC-param read-modify-write as Star6E but was not run on a Maruko device; please treat that backend as compile-tested only.

Independent of #110; whichever merges second needs a trivial VERSION/HISTORY.md rebase.

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Expose video0.maxIpProp (u32MaxIPProp) as live CBR I/P size cap

✨ Enhancement 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Add video0.maxIpProp/video0.max_ip_prop as a live-applied CBR RC control.
• Implement Star6E/Maruko RC-param read/modify/write with driver-default restore via 0.
• Bump contract/app versions and document backend support/limitations.
Diagram

graph TD
  A["Client: /set or /live/set"] --> B["venc_api.c: live apply groups"] --> C["VencApplyCallbacks.apply_max_ip_prop"] --> D["star6e_controls.c / maruko_controls.c"] --> E["MI_VENC Get/Set RC param"]
  F[("config JSON") ] --> G["*_runtime.c: startup apply"] --> C
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Expose a backend-agnostic “RC params” endpoint (generic struct patching)
  • ➕ Avoids adding one-off fields and live groups per RC knob
  • ➕ Scales better if more MI_VENC_RcParam_t fields become needed
  • ➖ Much larger API surface and validation burden
  • ➖ Harder to keep stable across backends/SDK versions
  • ➖ More footguns (users can set inconsistent param combinations)
2. Capture driver default at encoder init (instead of first apply)
  • ➕ Makes the meaning of 0 deterministic immediately after startup
  • ➕ Avoids relying on “first write” lifecycle behavior
  • ➖ Requires extra init-time driver queries even if the feature is unused
  • ➖ More wiring between init and control paths (per-channel state management)
3. Fold `maxIpProp` into existing max-frame-size live group
  • ➕ Fewer live groups to reason about
  • ➕ One apply path for I-frame sizing-related knobs
  • ➖ Conflates independent semantics (byte caps vs ratio cap) and mode restrictions
  • ➖ Harder to return accurate per-field support/errors (CBR-only vs general)

Recommendation: The PR’s approach (a dedicated video0.maxIpProp field, its own live group, and backend callbacks that are NULL when unsupported) is the best tradeoff: it keeps the API narrow, validates behavior cleanly (CBR-only), and preserves the existing “0 restores driver default” pattern used for QP bounds. If follow-up work is planned to expose more RC knobs, consider the generic RC-param patching approach, but it’s overkill for a single proven-effective control.

Files changed (12) +175 / -3

Enhancement (8) +149 / -1
venc_api.hAdd apply_max_ip_prop callback to VencApplyCallbacks +5/-0

Add apply_max_ip_prop callback to VencApplyCallbacks

• Introduces a new backend hook for live-updating 'u32MaxIPProp', including the '0' restore-to-driver-default behavior and support expectations (NULL when unsupported).

include/venc_api.h

venc_config.hAdd max_ip_prop to video config struct +6/-0

Add max_ip_prop to video config struct

• Extends 'VencConfigVideo' with 'max_ip_prop' and documents its purpose and observed firmware behavior motivating the control.

include/venc_config.h

maruko_controls.cImplement Maruko apply_max_ip_prop via RC param update +51/-0

Implement Maruko apply_max_ip_prop via RC param update

• Adds a CBR-only RC-param update function that captures the driver default on first use and allows restoring it by writing '0'. Wires the implementation into Maruko’s apply callback table.

src/maruko_controls.c

maruko_runtime.cApply max_ip_prop at Maruko startup when configured +5/-0

Apply max_ip_prop at Maruko startup when configured

• Applies a persisted non-zero 'video0.max_ip_prop' during runner initialization when the backend supports the callback.

src/maruko_runtime.c

star6e_controls.cImplement Star6E apply_max_ip_prop via MI_VENC RcParam +48/-0

Implement Star6E apply_max_ip_prop via MI_VENC RcParam

• Adds a CBR-only 'u32MaxIPProp' update path with driver-default capture/restore semantics and wires it into Star6E’s apply callbacks.

src/star6e_controls.c

star6e_runtime.cApply max_ip_prop at Star6E startup when configured +5/-0

Apply max_ip_prop at Star6E startup when configured

• Extends startup control application to set 'video0.max_ip_prop' when non-zero and the backend callback exists.

src/star6e_runtime.c

venc_api.cExpose max_ip_prop as live field with UI/aliasing and support checks +26/-1

Expose max_ip_prop as live field with UI/aliasing and support checks

• Adds field UI metadata, a camelCase alias ('video0.maxIpProp'), introduces a dedicated live-apply group, and routes live apply to the new backend callback. Also bumps the reported contract version to 0.18.3.

src/venc_api.c

venc_config.cParse/render/serialize maxIpProp in config +3/-0

Parse/render/serialize maxIpProp in config

• Adds JSON parsing from 'maxIpProp', includes the field in pretty-print rendering, and serializes it into the config JSON output.

src/venc_config.c

Documentation (2) +24 / -1
HISTORY.mdAdd 0.66.0 release notes for maxIpProp control +14/-0

Add 0.66.0 release notes for maxIpProp control

• Documents the new 'video0.maxIpProp' control, its motivation (byte caps inert on Star6E), backend support, and contract bump.

HISTORY.md

HTTP_API_CONTRACT.mdBump contract to 0.18.3 and document video0.max_ip_prop +10/-1

Bump contract to 0.18.3 and document video0.max_ip_prop

• Increments contract version, adds the new field to the backend divergence table, and records an additive changelog entry describing semantics, mode restrictions (CBR-only), and backend support.

documentation/HTTP_API_CONTRACT.md

Other (2) +2 / -1
VERSIONBump app version to 0.66.0 +1/-1

Bump app version to 0.66.0

• Updates the application version to reflect the new encoder control feature release.

VERSION

waybeam.default.jsonAdd default video0.maxIpProp field +1/-0

Add default video0.maxIpProp field

• Extends the default config with 'maxIpProp: 0' to keep the serialized layout stable and to default to SDK/driver behavior.

config/waybeam.default.json

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (1) 📜 Skill insights (0)

Grey Divider


Action required

1. Non-CBR returns 500 🐞 Bug ≡ Correctness
Description
apply_max_ip_prop() returns -1 when the channel isn’t CBR, and the live-set pipeline converts any
apply failure into HTTP 500, contradicting the contract’s promised 409 for non-CBR rate modes. This
makes an expected user error look like an internal failure and breaks clients relying on 409
semantics.
Code

src/star6e_controls.c[R522-523]

+	default:
+		return -1;
Evidence
The new backend apply returns -1 on non-CBR, while the live apply pipeline converts any apply
failure into a 500. The contract text states non-CBR should be a 409, and the API validation path
only emits 409 when validate_field_cfg()/validate_backend_config() rejects the config (which
currently has no rule for video0.max_ip_prop).

src/star6e_controls.c[512-524]
src/venc_api.c[2369-2402]
src/venc_api.c[2306-2337]
documentation/HTTP_API_CONTRACT.md[1697-1708]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
When `video0.max_ip_prop` is set while the encoder is in a non-CBR mode, `apply_max_ip_prop()` returns `-1`, which the live apply pipeline converts to HTTP 500. The contract explicitly states this should be a 409 validation failure.

## Issue Context
- Contract: non-CBR rate modes should answer `409` for `video0.maxIpProp` applies.
- Current code path: `apply_max_ip_prop()` returns `-1` on non-CBR, and `apply_live_group_sequence_locked()` treats any non-zero apply result as a 500.

## Fix Focus Areas
- src/venc_api.c[1074-1307]
- src/venc_api.c[2306-2337]
- src/venc_api.c[2369-2402]

## Proposed fix
1. Add a `validate_field_cfg()` rule for `video0.max_ip_prop`:
  - If `cfg->video0.max_ip_prop != 0` and `strcmp(cfg->video0.rc_mode, "cbr") != 0`, return a static error string like: `"video0.max_ip_prop requires video0.rcMode=cbr"`.
  - Also enforce bounds (see separate finding) so invalid values never reach the backend.
2. Ensure `video0.max_ip_prop` is included in the loaded-config validation set (so persisted configs get rejected early too).

This routes non-CBR attempts through the existing 409 validation path (`stage_params_into_cfg()`), matching the documented contract and avoiding 500s for expected user errors.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. New global g_ip_prop_default 📘 Rule violation ⚙ Maintainability Coding Conventions
Description
The PR introduces new file-scope mutable globals (g_ip_prop_default and
g_maruko_ip_prop_default) to cache the driver default, increasing hidden coupling and reducing
testability.
Code

src/star6e_controls.c[R494-497]

+static struct {
+	int      captured;
+	uint32_t prop;
+} g_ip_prop_default;
Evidence
PR Compliance ID 9 prohibits introducing new global mutable state. The added `static struct { ... }
g_ip_prop_default; in src/star6e_controls.c and the analogous g_maruko_ip_prop_default` in
src/maruko_controls.c are new mutable globals introduced by this PR.

Rule Coding Conventions: Avoid New Global Mutable State
src/star6e_controls.c[492-531]
src/maruko_controls.c[390-431]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New global mutable state was added (`g_ip_prop_default` / `g_maruko_ip_prop_default`) to store the captured driver default for `max_ip_prop`.

## Issue Context
Compliance requires avoiding new global mutable variables/state when possible; this cache can typically live in an existing per-backend context struct instead of file-scope globals.

## Fix Focus Areas
- src/star6e_controls.c[492-531]
- src/maruko_controls.c[390-431]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Signed parse wraps unsigned 🐞 Bug ☼ Reliability
Description
The config loader reads maxIpProp via json_get_int() (signed int) and casts to uint32_t, so
negative JSON values (e.g. -1) become huge values (e.g. 4294967295) and will be applied at
startup because the startup guard is > 0. This can push nonsensical values into
MI_VENC_SetRcParam and create hard-to-debug startup misconfiguration.
Code

src/venc_config.c[630]

+	v->max_ip_prop = (uint32_t)json_get_int(obj, "maxIpProp", (int)v->max_ip_prop);
Evidence
The loader’s json_get_int() returns a signed int, but the new maxIpProp field stores into
uint32_t via a cast; the startup runtime then applies any non-zero value, so wrapped negatives
become huge and are eligible to be pushed into the driver.

src/venc_config.c[42-48]
src/venc_config.c[623-633]
src/star6e_runtime.c[850-860]
src/maruko_runtime.c[143-154]
src/venc_api.c[1309-1344]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`maxIpProp` is loaded from JSON as an `int` (`cJSON`’s `valueint`) and then cast to `uint32_t`. Negative values wrap to very large unsigned values, and the runtime startup path applies any `>0` value.

## Issue Context
- `json_get_int()` returns `item->valueint` (signed).
- `load_video0()` assigns `v->max_ip_prop = (uint32_t)json_get_int(...)`.
- Startup calls `apply_max_ip_prop()` when `max_ip_prop > 0`.

## Fix Focus Areas
- src/venc_config.c[42-48]
- src/venc_config.c[623-633]
- src/venc_api.c[1309-1344]
- src/star6e_runtime.c[850-860]
- src/maruko_runtime.c[143-154]

## Proposed fix
1. Change `load_video0()` parsing of `maxIpProp` to avoid signed-to-unsigned wrap, e.g.:
  - Parse into a temporary `int tmp = json_get_int(obj, "maxIpProp", (int)v->max_ip_prop);`
  - If `tmp < 0`, set to 0.
  - Assign `v->max_ip_prop = (uint32_t)tmp;`
2. Add a validator in `validate_field_cfg()` enforcing a safe range (consistent with UI metadata: 0..100).
3. Add `"video0.max_ip_prop"` to `venc_api_validate_loaded_config()`’s `keys[]` list so persisted configs are rejected at load time.

This prevents wrapped values from being persisted/applied and makes bad configs fail loudly instead of silently misapplying at boot.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/star6e_controls.c Outdated
Comment thread src/star6e_controls.c
Comment thread src/venc_config.c
…are honours

The per-frame byte caps maxIBytes/maxPBytes are dead on Star6E firmware:
probed live 2026-08-22 under H.265 CBR, IDRs measured an identical
42-44 KB with the caps at 2000, 26000, and 8, and the RC priority switch
changed nothing. qpDelta barely moves IDR size either (43 KB at 0 and
at -6) because the CBR rate controller owns the I/P QP relationship.

The parameter the SigmaStar CBR RC actually honours for I-frame size is
u32MaxIPProp in MI_VENC_RcParam_t (stParamH265Cbr / stParamH264Cbr),
the cap on I-frame size as a proportion of P frames. waybeam never set
it, so the permissive SDK default is why recovery IDRs land at ~3x the
P-frame budget and occupy the air for ~60 ms at 10 Mbps.

This adds video0.maxIpProp (canonical video0.max_ip_prop), live on
Star6E and Maruko, CBR only (other rate modes fail the apply with 409).
0 = SDK default: the first apply captures the driver's value so 0 can
restore it live, mirroring the minQp/maxQp pattern. Applied at boot when
persisted and via /set and /live/set through a new LIVE_GROUP_MAX_IP_PROP.
CV610 reports the field unsupported.

Validation follows minQp/maxQp: validate_field_cfg() bounds the value to
0..100 and rejects a non-zero value under any rc_mode other than cbr, and
the key is in the loaded-config set. That is what makes the documented
409 true — the backend apply returns -1 on non-CBR, which the live-set
path would otherwise report as 500 — and it also catches a negative JSON
value, which the loader wraps large through the unsigned field. The
captured driver default lives in each backend's existing control context
rather than a new file-scope global.

Contract 0.18.2 -> 0.18.3 (additive). config/waybeam.default.json gains
the field at 0 so the save-layout byte-equal test keeps policing the
printer. tests/test_venc_config.c pins the loader-side rules.

Verified: make test 2491/0; star6e and maruko cross-builds clean.
@snokvist

Copy link
Copy Markdown
Collaborator

Adversarial review at e151a37b — one code blocker remains, plus targeted test gaps.

Blocking defect: the CBR dependency is validated only when the changed key is video0.max_ip_prop. Starting from a valid CBR config with maxIpProp=2, /api/v1/set?video0.rcMode=vbr validates only video0.rc_mode, is accepted and persisted, then requests a restart. The next config load iterates video0.max_ip_prop and rejects that same persisted config. This lets the API create a configuration that can prevent the daemon from returning after respawn. Please run the cross-field rule when either video0.max_ip_prop or video0.rc_mode changes, and add the reverse-direction API regression test (maxIpProp=2, then set rcMode=vbr -> 409 with no mutation/persist/reinit).

The current tests cover loader validation only. Please also cover:

  • /set and /live/set: 0, 1, 100 accepted; 101, negative, and non-CBR rejected with 409;
  • the reverse rcMode transition above;
  • callback invocation/argument, failure rollback, camelCase alias, capabilities true on Star6E/Maruko and false on CV610;
  • 0 restoring the captured default after a non-zero live apply (backend mock or a factored helper is sufficient for host coverage).

The PR text is accurate that Star6E and Maruko are wired through RC-param read/modify/write, CV610 is capability-gated, persisted validation catches wrapped negatives, and Maruko is compile-tested only. The statements that u32MaxIPProp is honoured and byte caps are inert are hardware-measurement claims, not established by this code or CI. The Star6E result is plausible but the PR currently has no raw procedure/log showing repeated IDR samples, RC-param readback, scene control, or restoration to default. Maruko still needs on-device verification before we call that backend confirmed; we can provide that hardware pass after the code/test blocker is fixed.

I reproduced make test = 2491/0 and make verify passing on this head.

@snokvist

Copy link
Copy Markdown
Collaborator

Maruko device test report (device observations only)

  • PR head tested: e151a37b20c283e951a07fb1e7d3ed8f6726146a
  • Built binary SHA-256: 41edf95ac13e721e5fa09f145a2bcabb3183931406286f31590623db81990712
  • Device: SSC378QE/Maruko, IMX335. /api/v1/version: app 0.66.0, contract 0.18.3, backend maruko.
  • /api/v1/capabilities advertised video0.max_ip_prop as supported/live, range 0..100.
  • Transport/measurement: isolated H.265 RTP host port plus rtp_timing_probe sidecar frame_size_bytes/frame_type; all captures and TSVs were stored on the host.

Live/API checks under H.265 CBR, 1280x720@30, 10000 kbps, one-second GOP, qpDelta=-4, ROI/OSD off, and maxIBytes=maxPBytes=0:

  • Values 2, 4, 100, and restored 0 each returned HTTP 200, read back immediately, and left the daemon PID unchanged.
  • Each 16-second sample received 480 RTP frames at 30.0 fps with 0 RTP gaps.
  • Matched sidecar/RTP frame-size samples were:
maxIpProp P median / P95 (bytes) IDR median / P95 (bytes) matched P / IDR samples
0 baseline 47236 / 50144 27511 / 28057 314 / 13
2 47225 / 49644 27396 / 28031 280 / 8
4 47393 / 50133 27930 / 28241 293 / 12
100 47252 / 50146 27693 / 28041 307 / 9
0 restored 47172 / 49831 18494 / 27672 281 / 9

The probe received 480 sidecar FRAME datagrams per run; the size table includes only frames for which the probe matched RTP and sidecar metadata. In this scene, IDRs were already smaller than median P frames, and the tested values did not produce a monotonic/reproducible decrease. This test therefore verifies API/apply stability but does not demonstrate a binding maxIpProp hardware effect.

A second matrix used 3000 kbps, qpDelta=-12, five-second GOP, and 12 accepted /request/idr calls at 1.2-second spacing:

  • maxIpProp=0: P median/P95 12860/13658 bytes (n=276); IDR 12195/12532 (n=11).
  • maxIpProp=2: P median/P95 12763/13907 bytes (n=279); IDR 12200/12256 (n=9).
  • Both runs remained about 29.3 fps with 0 RTP gaps. IDRs again were not larger than P frames, so this also did not establish a binding cap.

Validation/persistence observations:

  • Values 101 and -1 each returned HTTP 409 with the documented 0..100 validation message; readback stayed 0 and PID was unchanged.
  • Persistent /api/v1/set?video0.maxIpProp=20 returned 200, applied without a PID change, wrote 20 to /etc/waybeam.json, survived a normal service restart, read back as 20, and logged maxIpProp changed to 20 (0 = driver default 0). Persistent restore to 0 returned 200 and wrote/read back 0.

Existing byte-field comparison on the same device/config family, using forced IDRs:

  • Baseline maxIBytes=maxPBytes=0: IDR range 12063..12532 bytes, median 12195 (n=11).
  • maxIBytes=2000,maxPBytes=0: IDR range 4748..5918 bytes, median 5866 (n=3). No sampled IDR met a 2000-byte ceiling.
  • maxIBytes=0,maxPBytes=2000: P median 2057, P95 2440, range 1238..5122 bytes (n=372). The field changed the distribution but did not impose a 2000-byte ceiling.
  • Both capped samples continued about 29.3 fps with 0 RTP gaps.

Not tested here: Star6E, non-CBR modes, H.264, or a scene/config where IDRs exceed P frames enough to demonstrate whether maxIpProp binds. Post-test dmesg had no fault/timeout/watchdog match. The device was restored to its original binary and config; both pre-test hashes matched after restoration, and the original stream restarted.

@snokvist

Copy link
Copy Markdown
Collaborator

Star6E high-bitrate device test report (device observations only)

  • PR head tested: e151a37b20c283e951a07fb1e7d3ed8f6726146a
  • Built binary SHA-256: 27a04e2934e7e495d6ff559263f24b20a86105b46582000dcbfc6002d39f4463
  • Device: SSC338Q/Star6E, IMX335 mode 1 (2560x1920@60 capture), H.265 1920x1080@60. /api/v1/version: app 0.66.0, contract 0.18.3, backend star6e.
  • Test stream: isolated host UDP port; captures and TSVs were stored off-device.

Isolation correction: the first run was excluded after /api/v1/config showed the running waybeam_hub controller had rewritten maxIBytes/maxPBytes from 0 to 38728/5993. The controller was stopped through its init script; after that, bitrate, minQp, maxIBytes, maxPBytes, and maxIpProp remained unchanged for the test windows. The controller was restarted after testing.

Common isolated config: CBR, one-second GOP, qpDelta=-12, minQp=1, maxQp=0, maxIBytes=maxPBytes=0, resilience off, ROI/OSD off. minQp=1 was required for the encoder to spend the requested high bitrate; with the driver default it produced about 5.75 Mbps despite a 25 Mbps target.

At an achieved 24.7..25.5 Mbps, a continuous-motion paired sidecar sample produced:

maxIpProp RTP result P median / P95 bytes IDR median / P95 bytes matched P / IDR
0 1082 frames / 18 s, 60.1 fps, 0 gaps 54366 / 87414 5205 / 5352 681 / 18
2 1081 frames / 18 s, 60.1 fps, 0 gaps 53361 / 82072 4554 / 4734 693 / 7

Direct off-device H.265 access-unit measurements (ffprobe -show_packets, key flag checked against an Annex-B NAL walk showing one IDR_W_RADL per key AU) at the same achieved 25 Mbps:

maxIpProp key AUs mean key bytes mean non-key bytes
0, first run 12 5306.8 54214.3
1 8 3711.5 54093.6
2 12 4673.2 54157.4
4 8 5352.0 54114.8
100 8 5355.9 54120.3
0, repeat 8 5390.2 54340.8

At an achieved 38.1..40.8 Mbps, direct six-second captures produced:

maxIpProp key AUs mean key bytes mean non-key bytes
0 6 5877.5 86599.3
1 6 4181.0 86659.2
2 6 5819.2 86710.2

In these 25/40 Mbps configurations, value 1 reproducibly reduced key-AU size; value 2 reduced it in the 25 Mbps samples but not the 40 Mbps sample; values 4 and 100 matched the default-sized key AUs. Key AUs were already much smaller than ordinary AUs in every sample. Full-frame motion did not create the 42–44 KB IDR population described in the PR text on this device/config.

Existing byte-field comparison at achieved 25 Mbps:

  • maxIBytes=2000,maxPBytes=0: 8 key AUs, mean 5283.0 bytes; 473 non-key AUs, mean 54216.9 bytes.
  • maxIBytes=0,maxPBytes=2000: 8 key AUs, mean 5413.4 bytes; 472 non-key AUs, mean 54152.9 bytes.
  • Uncapped repeat: mean key 5390.2 bytes and mean non-key 54340.8 bytes. Neither byte field imposed the requested 2000-byte ceiling or materially changed the direct-AU means in this Star6E run.

Not tested here: H.264, non-CBR rate modes, IMX415, or another Star6E firmware build. After testing, the original binary and config SHA-256 hashes matched their pre-test values, the original frame-SHM configuration was active, and waybeam_hub plus its supervisor were running again.

@snokvist

Copy link
Copy Markdown
Collaborator

Setup details for the Star6E measurements above

The encoder binary under test was the PR-head Star6E build already identified above:

  • source: e151a37b20c283e951a07fb1e7d3ed8f6726146a
  • /usr/bin/waybeam SHA-256 while testing: 27a04e2934e7e495d6ff559263f24b20a86105b46582000dcbfc6002d39f4463

Sensor/IQ environment:

  • loaded module: sensor_imx335_star6e; module file /lib/modules/4.9.84/sigmastar/sensor_imx335_mipi.ko, SHA-256 ea6412c83faead6f3cc3b3de4686a50c7fb0de4564d176946282bcb59ad73eb4
  • sensor.index=-1, sensor.mode=1; runtime selected pad 0, 2560x1920@60
  • isp.sensorBin=/etc/sensors/imx335_spike5_colortrans.bin (configured, not auto-selected)
  • the startup log records Loading ISP file followed by ISP file loaded successfully for that exact path
  • IQ bin size: 88738 bytes; SHA-256 cbccca59740ba9bb6049865709f1223e9d900b1ed1d5247a68f983a6d7f5fe73
  • isp.aeEngine=sdk, aeFps=15, awbFps=15, gainMax=0, shutterMaxUs=4000, awbMode=auto, awbCt=5000, keepAspect=true, shutterRule180=false
  • runtime precrop: 2560x1440 @ 0,240

Encoder/test options after stopping the interfering waybeam_hub controller:

  • system.overclockLevel=2, system.verbose=true
  • H.265 CBR, video0.size=1920x1080, fps=60, gopSize=1, qpDelta=-12
  • requested/achieved bitrate pairs: 25000 kbps / approximately 24700..25500 kbps, and 40000 kbps / approximately 38100..40800 kbps
  • minQp=1, maxQp=0; minQp=1 was applied live so CBR actually spent the high target
  • maxIBytes=0, maxPBytes=0 except during the explicitly labelled byte-cap controls
  • maxIpProp=0/1/2/4/100 as labelled in each sample
  • resilience=off, fpv.roiEnabled=false, debug.showOsd=false
  • image mirror/flip disabled, rotation 0
  • isolated RTP output to the test workstation on UDP port 15600; maxPayloadSize=1400, unconnected UDP, sidecar port 5602
  • the paired 25 Mbps sidecar samples used continuous full-frame motion; the direct-AU repetitions used a fixed scene while CBR was already filling the requested bitrate

No recording path was enabled and no capture was written on the device.

@snokvist

Copy link
Copy Markdown
Collaborator

Heads-up: #114 (parity sync v0.65.2 → v0.67.1, 54 commits) is open and will conflict with this one.

This PR claims VERSION 0.66.0, which #114 already uses for the Star6E multi-slice release. After it lands the next free version is 0.67.2, and the next free contract version is 0.18.7 (#114 takes the contract to 0.18.6) — so VERSION, the HISTORY.md entry, and the contract bump will each need re-cutting.

Code overlap is real but small: #114 touches src/venc_api.c and src/venc_config.c (adding video0.sliceCount plumbing and the data.routes object), so the video0.maxIpProp field registration will land next to those rather than on top of them. maruko_controls.c/star6e_controls.c are untouched by #114.

Sorry for the churn — happy to help with the rebase if useful.

@snokvist

Copy link
Copy Markdown
Collaborator

#114 is merged, so this now shows as conflicting. Nine files overlap: VERSION, HISTORY.md, documentation/HTTP_API_CONTRACT.md, config/waybeam.default.json, include/venc_config.h, src/venc_config.c, src/venc_api.c, src/star6e_runtime.c, tests/test_venc_config.c.

Most of it is mechanical — the sync added video0.sliceCount plumbing along exactly the path a new video0.maxIpProp field takes, so your additions land next to the new ones rather than on top of them: a field in venc_config.h, a parse/default in venc_config.c, a config-file default, a capabilities/mutability entry in venc_api.c, and a test. Expect adjacent-line conflicts rather than semantic ones. maruko_controls.c / star6e_controls.c are untouched by the sync.

Bookkeeping to re-cut: VERSION0.67.2 (0.66.0 is now the Star6E multi-slice release), HISTORY.md entry to the top as ## [0.67.2], and the contract to 0.18.7 — four sites: the header near line 21, the example body, the divergence note, and the string in venc_api.c.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants