Skip to content

fix: deep dive systematic fixes + WASM TV Guide auto-advance#64

Merged
AndrewAltimit merged 2 commits intomainfrom
fix/deep-dive-systematic-improvements
Mar 10, 2026
Merged

fix: deep dive systematic fixes + WASM TV Guide auto-advance#64
AndrewAltimit merged 2 commits intomainfrom
fix/deep-dive-systematic-improvements

Conversation

@AndrewAltimit
Copy link
Owner

Summary

  • CommandSignal refactor: Extract signal variants from CommandOutput into a dedicated CommandSignal enum with convenience constructors, updated across 9+ files in 5 crates
  • Memory safety fixes: Integer overflow in AVCC NAL parsing (checked_add), saturating seek interpolation, bounded H.264 reinit packet budget (1500 total)
  • Robustness: Stall detection in streaming download throttle loop, exponential backoff for auth rate limiting (30s base, 2^excess multiplier, capped at 64x), FFI buffer size validation
  • WASM TV Guide auto-advance: Listen for <video> ended event and automatically tune to the next episode via deterministic schedule query -- the desktop build already had this but WASM was missing it entirely

Test plan

  • All workspace tests pass (cargo test --workspace)
  • Clippy clean (cargo clippy --workspace -- -D warnings)
  • WASM build passes (./scripts/build-wasm.sh)
  • PSP EBOOT build passes
  • New tests: auto_advance_skips_to_next_episode, schedule_at_episode_boundary_yields_next, seek_interpolation_saturates_on_large_offset, test_rate_limit_exponential_backoff, avcc_to_annex_b_huge_nal_length_no_overflow
  • Manual: deploy WASM build, tune to a TV Guide channel, verify next episode starts automatically when current one ends

Generated with Claude Code

Systematic improvements from deep dive audit across 7 crates:

- Extract CommandSignal enum from CommandOutput for type-safe signal handling
- Fix integer overflow in AVCC NAL parsing (checked_add)
- Add bounded packet budget across H.264 reinit attempts
- Fix saturating_add in streaming seek interpolation
- Add stall detection in download throttle loop to prevent deadlocks
- Exponential backoff for auth rate limiting (base * 2^excess, capped 64x)
- Add FFI buffer size validation before copy_nonoverlapping
- Document PSK auth TLS handshake ordering

WASM TV Guide auto-advance:
- Add 'ended' event listener on <video> element to detect episode completion
- Add auto_advance_episode() to WASM backend matching desktop behavior
- Query deterministic schedule for next episode, skip <5s remaining slots

Tests: auto-advance schedule query, episode boundary, seek saturation,
exponential backoff, AVCC overflow, FFI buffer validation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link

Gemini AI Code Review

Issues (if any)

  • [WARNING] crates/oasis-ffi/src/lib.rs:1071 - FFI function signature change
    • The addition of the buf_size parameter to oasis_video_next_frame breaks existing C ABI compatibility.
    • Ensure examples/ffi_demo.c and the UE5 C++ integration code are updated to pass the new buffer size argument, otherwise this will cause build failures or undefined behavior.

Previous Issues (for incremental reviews)

(none)

Suggestions (if any)

(none)

Notes

  • Excellent proactive memory safety and logic fixes: bounded retry limits for packet skipping, saturating arithmetic for media duration seeking, and robust integer overflow checks in NAL parsing.
  • Extracting CommandSignal from CommandOutput significantly improves semantic clarity in the terminal interpreter by explicitly separating side effects from display text.
  • The exponential backoff implementation for network authentication failures is clean and correctly bounds the multiplier to prevent potential overflow.

Reaction


Generated by Gemini AI (gemini-3.1-pro-preview). Supplementary to human reviews.

Automated fix by Claude in response to Gemini/Codex review.

Iteration: 1/5

Co-Authored-By: AI Review Agent <noreply@anthropic.com>
@AndrewAltimit
Copy link
Owner Author

Review Response Agent (Iteration 1)

Status: Changes committed, pushing...

Commit: e851873

Fixed Issues

  • [WARNING] docs/ffi-integration.md:219-220 - Updated C function signature for oasis_video_next_frame to include the new buf_size parameter that was added to the Rust FFI in crates/oasis-ffi/src/lib.rs:1074. The documentation was out of sync with the actual API, which would cause compilation errors for any C/C++ consumer referencing the documented signature.

Ignored Issues

  • Gemini mentioned updating examples/ffi_demo.c — verified it doesn't call oasis_video_next_frame, so no change needed there.
  • Gemini mentioned UE5 C++ integration code — that lives outside this repo (in the UE5 project), so cannot be updated here.

Deferred to Human

  • (none)

Notes

  • The review was largely positive, praising the memory safety fixes, CommandSignal extraction, and exponential backoff implementation. Only one actionable issue was found.

Automated summary of agent fixes.

@github-actions
Copy link

Gemini AI Incremental Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

(none)

Previous Issues (for incremental reviews)

  • [RESOLVED] crates/oasis-ffi/src/lib.rs:1071 - FFI function signature change

Suggestions (if any)

(none)

Notes

  • The documentation for the FFI integration has been successfully updated to reflect the new buf_size parameter.
  • Acknowledged that the UE5 integration code is external to this repository and examples/ffi_demo.c does not utilize oasis_video_next_frame, making the documentation update sufficient for this repository.

Reaction


Generated by Gemini AI (gemini-3.1-pro-preview). Supplementary to human reviews.

@AndrewAltimit
Copy link
Owner Author

Review Response Agent (Iteration 2)

Status: No changes needed

Warning: The agent's summary below claims fixes were applied, but no files were actually modified. These claimed fixes were NOT committed.

Fixed Issues

  • (none — the incremental review found zero new issues)

Ignored Issues

  • (none — no issues were raised)

Deferred to Human

  • (none)

Notes

  • The Gemini incremental review explicitly states "Issues: (none)" and confirms the previous FFI signature issue is "[RESOLVED]". There are no actionable items to fix.

The agent reviewed feedback but no file modifications were detected.

@AndrewAltimit AndrewAltimit merged commit 6e5623a into main Mar 10, 2026
9 checks passed
@AndrewAltimit AndrewAltimit deleted the fix/deep-dive-systematic-improvements branch March 10, 2026 10:32
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