fix: resolve 4 audit bugs (PTS, WlOutput, first_frame, DRM minor)
This commit is contained in:
110
docs/superpowers/specs/2026-04-06-fix-audit-bugs-design.md
Normal file
110
docs/superpowers/specs/2026-04-06-fix-audit-bugs-design.md
Normal file
@@ -0,0 +1,110 @@
|
||||
# Fix Audit Bugs - Design Spec
|
||||
|
||||
**Date:** 2026-04-06
|
||||
**Scope:** Critical + High severity bugs from code audit
|
||||
**Approach:** Minimal fix principle - change only what is necessary
|
||||
|
||||
## Bugs to Fix
|
||||
|
||||
| # | Severity | Bug | File |
|
||||
|---|----------|-----|------|
|
||||
| 1 | CRITICAL | PTS timestamps in microseconds but time_base is 1/fps - output unplayable | state.rs |
|
||||
| 2 | HIGH | WlOutput dispatch attributes events to wrong output (multi-monitor) | state.rs |
|
||||
| 3 | HIGH | First captured frame always silently dropped | state.rs |
|
||||
| 4 | HIGH | DRM render node minor number extraction wrong | state.rs |
|
||||
|
||||
Bugs #5-11 (Medium/Low) deferred to future work.
|
||||
|
||||
---
|
||||
|
||||
## Bug 1: PTS Timestamp Unit Mismatch
|
||||
|
||||
### Root Cause
|
||||
|
||||
on_copy_complete sets PTS in microseconds. But the encoder time_base is 1/fps
|
||||
(frame-number units). Filter graph passes PTS through unchanged. At 30fps, frame
|
||||
2 has PTS=33333, which the muxer reads as 33333/30 = 1111 seconds.
|
||||
|
||||
### Fix
|
||||
|
||||
Convert PTS from microseconds to frame numbers at the capture boundary.
|
||||
|
||||
In state.rs on_copy_complete (line 373):
|
||||
```rust
|
||||
// Before:
|
||||
let pts = (tv_sec as i64) * 1_000_000 + (tv_usec as i64);
|
||||
|
||||
// After:
|
||||
let fps = self.args.fps as i64;
|
||||
let pts = (tv_sec as i64) * fps + (tv_usec as i64) * fps / 1_000_000;
|
||||
surface.set_pts(Some(pts));
|
||||
```
|
||||
|
||||
PTS becomes an integer frame counter. No other files change.
|
||||
|
||||
---
|
||||
|
||||
## Bug 2: WlOutput Multi-Monitor Index Error
|
||||
|
||||
### Root Cause
|
||||
|
||||
Dispatch for WlOutput uses () user data. The handler uses
|
||||
outputs.len().saturating_sub(1) as index - always the last output. With
|
||||
multiple outputs, events for output 0 are written to output N-1.
|
||||
|
||||
### Fix
|
||||
|
||||
Change WlOutput dispatch user data from () to OutputId(global_name).
|
||||
|
||||
Changes in state.rs:
|
||||
|
||||
1. Registry dispatch - bind with OutputId(name) instead of ()
|
||||
2. Change impl Dispatch for WlOutput from user data () to OutputId
|
||||
3. Use output_names lookup to find correct index from data.0
|
||||
|
||||
---
|
||||
|
||||
## Bug 3: First Frame Silently Dropped
|
||||
|
||||
### Root Cause
|
||||
|
||||
FpsLimit::on_new_frame returns None on first call (buffers it). In
|
||||
on_copy_complete, should_encode is false for the first captured frame, so
|
||||
the surface is dropped without encoding.
|
||||
|
||||
### Fix
|
||||
|
||||
Add first_frame: bool field to State (initial true). On first call to
|
||||
on_copy_complete, force should_encode = true and set first_frame = false.
|
||||
|
||||
---
|
||||
|
||||
## Bug 4: DRM Minor Number Extraction Wrong
|
||||
|
||||
### Root Cause
|
||||
|
||||
(dev_t as u32) and 0xFFFFF extracts raw bits instead of the Linux minor
|
||||
number. For renderD128 (major=226, minor=128): dev_t=57984, code produces
|
||||
/dev/dri/renderD57984.
|
||||
|
||||
### Fix
|
||||
|
||||
Use Linux standard minor extraction:
|
||||
```rust
|
||||
let dev = u64::from_ne_bytes(dev_bytes);
|
||||
let minor = ((dev & 0xFF) | ((dev >> 12) & 0xFFFFFF00)) as u32;
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Impact Summary
|
||||
|
||||
All changes in state.rs only. ~24 lines changed total.
|
||||
|
||||
## Out of Scope (Deferred)
|
||||
|
||||
- Bug 5: Silent hang without xdg_output_manager
|
||||
- Bug 6: Intermediate state on encoder failure
|
||||
- Bug 7: Fragile capture-start in EverythingButFmt
|
||||
- Bugs 8-11: Low severity
|
||||
- Tail frame preservation
|
||||
Reference in New Issue
Block a user