Skip to content

fix(video): clamp estimateJpegFrameBytes, reuse the shared JPEG prefix - #12543

Open
pacocartones wants to merge 2 commits into
diegosouzapw:release/v3.8.51from
pacocartones:fix/video-frame-estimate-clamp
Open

fix(video): clamp estimateJpegFrameBytes, reuse the shared JPEG prefix#12543
pacocartones wants to merge 2 commits into
diegosouzapw:release/v3.8.51from
pacocartones:fix/video-frame-estimate-clamp

Conversation

@pacocartones

Copy link
Copy Markdown
Contributor

Summary

  • estimateJpegFrameBytes (src/lib/guardrails/videoBridgeFrameContract.ts:24-27) subtracted the padding count from the floored estimate with no floor of its own, so the padding-only payloads the charset-only pattern admits (data:image/jpeg;base64,= and ==) came back as -1. Consequence-free in today's upper-bound usage, as feat(backend): video frame-contract follow-ups — clamp negative estimate, unify encode-site prefixes, broker-client 4th site #12323 notes, but a documented byte estimate never goes negative: the return is now Math.max(0, ...) (item 1).
  • The three encode sites named in the issue (videoBridgeContactSheet.ts:123, videoBridgeRuntime.ts:1000, videoBridgeDrilldownLifecycle.ts:173) still built their frame data URIs from a hardcoded data:image/jpeg;base64, template literal; they now import JPEG_FRAME_DATA_URI_PREFIX from the shared contract (item 2). Byte-for-byte identical output, since the constant is the same literal.
  • Deliberately out of scope: item 3 (videoBridgeBrokerClient.ts:104, the case-sensitive broker-output regex) and item 4 (structural base64 validation). Both are behaviour decisions per the issue text, so the issue stays open; this PR is Refs, not Closes. videoBridgeRuntime.ts gains a single import of the zero-dependency contract module, so no import cycle is introduced (videoBridgeContactSheet.ts already imported both).

Related Issues

Validation

  • Change type: other (backend guardrails / video frame contract)
  • Focused tests and category gates from the golden path: tests/unit/guardrails/videoBridgeFrameContract.test.ts + videoBridgeContactSheet.test.ts + videoBridgeRuntime.test.ts + videoBridgeDrilldownLifecycle.test.ts 38/38, node scripts/check/check-complexity-ratchets.mjs --base-ref origin/release/v3.8.51 OK (7/7 and 1/1 unchanged), npm run check:changelog-integrity OK, npm run typecheck:core exit 0, eslint exit 0 over the five touched .ts files
  • npm run lint
  • Reconciled with the current active release base release/v3.8.51; focused checks rerun afterward
  • Production-code changes include a new or updated automated test in this PR
  • SonarQube is temporarily opt-in while the private project has no quota; it is not a PR gate.

Tests Added Or Updated

  • tests/unit/guardrails/videoBridgeFrameContract.test.ts (2 new cases): padding-only payloads (=, ==, A=, A==) estimate >= 0, stay an upper bound of the real decode, and =/== estimate exactly 0; and a source-level check that the three encode sites no longer contain the data:image/jpeg;base64, literal and do reference JPEG_FRAME_DATA_URI_PREFIX. Red on the base (2 fail / 3 pass: "=" estimated -1, videoBridgeContactSheet.ts hardcodes the JPEG prefix), green with the change (5/5). The three existing cases are unchanged.

Coverage Notes

  • src/lib/guardrails/videoBridgeFrameContract.ts: the clamp is exercised directly by the new padding-only case; the existing padding-accounting case still pins the non-degenerate values.
  • src/lib/guardrails/videoBridgeContactSheet.ts, videoBridgeRuntime.ts, videoBridgeDrilldownLifecycle.ts: the prefix swap is behaviour-neutral and is covered by the existing videoBridgeContactSheet.test.ts, videoBridgeRuntime.test.ts and videoBridgeDrilldownLifecycle.test.ts suites (all green), plus the new source-level guard against the literal creeping back.
  • No touched file lost coverage.

Reviewer Notes

  • Two hunks in videoBridgeDrilldownLifecycle.ts (lines 214 and 366) are not Prettier-clean on the base; the pre-commit hook wanted to rewrap them. They were left untouched on purpose so this diff stays on the two lines the issue names. Happy to include the rewrap if you prefer the file fully formatted.
  • No migrations, feature flags or manual validation; the encoded output is identical before and after.

`estimateJpegFrameBytes` subtracted the padding count from the floored
estimate without a floor of its own, so padding-only payloads the
charset pattern admits (`data:image/jpeg;base64,=`, `==`) came back as
-1. A documented byte estimate never goes negative: clamp with
`Math.max(0, ...)`.

The three encode sites (contact sheet, runtime frame extraction,
drill-down lifecycle) still built their data URIs from a hardcoded
`data:image/jpeg;base64,` literal instead of the shared
`JPEG_FRAME_DATA_URI_PREFIX`; they now import the constant. The
broker-client validation site (item 3) and structural base64 validation
(item 4) are behaviour decisions left to the maintainer.

Tests: two new cases in videoBridgeFrameContract.test.ts — padding-only
payloads estimate 0 and stay an upper bound of the real decode, and a
source-level check that the three encode sites no longer hardcode the
prefix. Both fail on the base and pass with this change.

Refs diegosouzapw#12323
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.

1 participant