Skip to content

fix(video): load the H.264 bootstrap cache without statx - #10

Merged
Producdevity merged 3 commits into
Producdevity:masterfrom
alexander-clawthorne:fix/h264-bootstrap-statx
Oct 17, 2026
Merged

Producdevity merged 3 commits into
Producdevity:masterfrom
alexander-clawthorne:fix/h264-bootstrap-statx

Conversation

@alexander-clawthorne

@alexander-clawthorne alexander-clawthorne commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Targets master because the bug is there: loadBootstrap has used getEndPos() since the early commits, and the GeForce NOW branch carries the same code. The branch name follows the fix/<kebab-case> pattern of your fix/gfn-atomics-* branch, and the commit uses the conventional prefixes from #2-#4. The change is independent of #8 and #9.

What does this change?

loadBootstrap in src/media/video/video_pipeline.zig sized the H.264 parameter-set cache with file.getEndPos(). That goes through File.stat(), which uses statx, and statx only exists from Linux 4.11. On the 4.9 kernels H700 devices ship, the call fails, catch return 0 swallows the error, and the cache is never loaded, even though persistBootstrap writes it after every session. This is the limitation src/platform/linux/stat_compat.zig already documents.

The fix reads the file into a buffer one byte larger than bootstrap_capacity and uses the byte count as the size. Empty and oversized files are rejected exactly as before, and no stat call is involved. Read errors are now logged in debug mode, like the existing open errors.

How did you test it?

On an Anbernic RG35XX-H (H700, muOS, kernel 4.9.170):

  1. Proving the bug. A small static aarch64 probe on the device, against the real cache file:

    getEndPos: error.Unexpected
    raw fstat: 42 bytes
    

    Debug logs from earlier streaming sessions never contain H.264 bootstrap loaded, although h264-parameter-sets.bin (42 bytes, starting 00 00 00 01 67) was present.

  2. With the fix (built on the GeForce NOW branch, where I can stream), one GFN session:

    H.264 bootstrap loaded (42 bytes)
    ...
    Video output active: 1024x768 format=23 decoder=cedar-h616
    ...
    H.264 bootstrap updated
    H.264 bootstrap loaded (40 bytes)
    

    The whole cycle works: the cache is loaded at startup, rewritten after the session, and loaded again. The stream showed 0 decode errors, 0 corruption and 1 keyframe in the first second.

  3. On master, tools/zig.sh build test and tools/zig.sh build fmt-check pass.

    I could not run build release on master in my environment. Without the pkg-config isolation from 3724080 (on feat/geforce-now), host headers leak into the SDL2 dependency build (L_tmpnam errors in stdio.h). That is unrelated to this change. On feat/geforce-now, the same change builds for release and has 0 __aarch64_ outline-atomics imports.

Not measured: how much this speeds up the first picture. Without the cache the decoder waits for the server's parameter sets. In one session the first second presented 55 of 57 decoded frames, against 39 of 57 in an earlier session without the cache. That is a single comparison, not a benchmark. Xbox Cloud uses the same pipeline, but I have not tested it.

video_pipeline.zig has no host unit tests and needs FFmpeg and SDL headers, so I didn't add new build wiring for a one-function fix. I'm happy to add a test if you'd prefer a particular setup.

AI disclosure

Per CONTRIBUTING: this was found and written with AI assistance (Claude). I reviewed the diff and verified the bug and the fix on hardware myself.

🤖 Generated with Claude Code


Summary by cubic

Fixes H.264 bootstrap cache loading on kernels older than 4.11, where File.getEndPos() failed because it relies on statx. The cache is now sized by reading the file directly, so saved SPS/PPS data loads correctly on H700 devices running kernel 4.9.

  • Extracts cache loading into bootstrap_cache.zig, covered by unit tests, including one that blocks statx via seccomp to prove the fix on kernel 4.9.
  • Continues rejecting empty and oversized caches; oversized files and read failures are logged in debug mode.

Written for commit 85bacb1. Summary will update on new commits.

View guided diff

Summary by CodeRabbit

  • Bug Fixes
    • Bootstrap cache loading now ignores empty or invalid cache data and stops when a cache cannot be read. In debug mode, oversized caches are distinguished from other unreadable caches.
    • Cache files larger than the supported capacity are rejected, while files within the limit can be read without relying on file-size metadata.

loadBootstrap sized the cache with File.getEndPos(), which goes through
File.stat() and statx. Kernel 4.9 has no statx, so on H700 devices the
call failed, the error was swallowed and the saved SPS/PPS were never
loaded, although they were written after every session. Read the file
into a buffer one byte larger than the limit instead, which also detects
oversized files, and log read failures in debug mode.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: bfc2dd74-6304-432a-9219-429eb2959246
📥 Commits

Reviewing files that changed from the base of the PR and between e856b29 and 85bacb1.

📒 Files selected for processing: 3
  • build.zig
  • src/media/video/bootstrap_cache.zig
  • src/media/video/video_pipeline.zig

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

The video pipeline now uses a dedicated cache reader with a 4104-byte capacity. The reader detects oversized files without calling stat, and the host test step runs its tests.

Changes

Bootstrap Cache Loading

Layer / File(s) Summary
Bounded cache reader and validation
src/media/video/bootstrap_cache.zig
The new reader reads into a buffer one byte larger than capacity and returns error.FileTooBig when the data exceeds capacity. Tests cover valid parameter sets, empty and malformed data, size boundaries, file errors, and operation when Linux statx is unavailable.
Pipeline integration and test wiring
src/media/video/video_pipeline.zig, build.zig
loadBootstrap uses the shared capacity and reader. It ignores empty data and depacketizer-rejected data, and distinguishes oversized files from other read errors in debug logging. The host test step runs the cache reader tests with h264_depacketizer imported.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: producdevity

Merge Risk: ⚪ Minimal · up to 85bac

The cache-loading change is ready to merge after normal checks; no material issue was identified.

### Architecture Summary

Architecture risk: 🔵 Low · up to e856b

The change affects 1 system.

Changed systems: src

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — src (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in src/media/video/video_pipeline.zig: loadBootstrap replaces getEndPos() and the exact-size readNoEof read with a bootstrap_capacity + 1 buffer and readAll. Read failures now produce a debug message and return; zero-byte and over-capacity files are ignored.
Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: loading the H.264 bootstrap cache without using statx.
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (3 skipped: 3 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 1 file

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread src/media/video/video_pipeline.zig
alexander-clawthorne and others added 2 commits October 4, 2026 18:24
An oversized cache was rejected without a message, so in debug logs it looked the same as a missing one. Log it like the other skipped-cache cases.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Producdevity

Copy link
Copy Markdown
Owner

Thanks for tracking this down and testing it on muOS.

I added regression tests, trimmed the comments a bit (claude loves to write novels in comments 😂)

@Producdevity
Producdevity merged commit cdee258 into Producdevity:master Oct 17, 2026
4 checks passed
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