Hebbian-Robotics / Hebbian-Robotics/hflow
The fixed-GOP promise is stamped into provenance as "actually used" but is never measured on pass-through video
- Dominant language
- Python
- Stars
- 269
- Forks
- 150
- Avg merge
- 8h 18m
- Merged PRs (30d)
- 246
Description
## Current behavior
FORMAT.md item 5 promises Fixed GOP (`keyint = min-keyint`, `scenecut=0`): "keyframes land exactly every `gop_frames` messages, so seek cost is uniform and predictable" ([docs/FORMAT.md:79](https://github.com/Hebbian-Robotics/hflow/blob/main/docs/FORMAT.md#L79)), and the presets section calls GOP length "effectively a training hyperparameter" ([docs/FORMAT.md:84](https://github.com/Hebbian-Robotics/hflow/blob/main/docs/FORMAT.md#L84)). The provenance record stamps `gop_seconds`, defined at [docs/FORMAT.md:129](https://github.com/Hebbian-Robotics/hflow/blob/main/docs/FORMAT.md#L129) as "The keyframe interval actually used."
On the re-encode path the promise is real: x264 gets `keyint=min-keyint=gop_frames`, `scenecut=0`, and `_enforce_encode_guarantees` refuses any keyframe that lands off the grid (`expected {keyframe_expected} (gop_frames={gop_frames})`, [src/hflow/video.py:683](https://github.com/Hebbian-Robotics/hflow/blob/main/src/hflow/video.py#L683)).
On the pass-through path nothing measures it:
- `_validate_passthrough_video_payload` ([src/hflow/transform.py:479](https://github.com/Hebbian-Robotics/hflow/blob/main/src/hflow/transform.py#L479)) checks one access unit per message, SPS/PPS on keyframes, first-message keyframe, and B-freedom. Cadence is not checked.
- `_check_video_payload` ([src/hflow/doctor.py:96](https://github.com/Hebbian-Robotics/hflow/blob/main/src/hflow/doctor.py#L96)) checks the same two keyframe properties (SPS/PPS at :174, first message at :180) and never the interval between them.
The provenance record is written once per episode, identically for both paths ([src/hflow/transform.py:895-896](https://github.com/Hebbian-Robotics/hflow/blob/main/src/hflow/transform.py#L895-L896)), so a pass-through episode is stamped with the configured interval no matter what its stream contains.
## Controlled result
A 100-message pass-through source, conforming by every existing rule (AUD-first, one access unit per message, SPS/PPS on every keyframe, first message is a keyframe, zero B-frames), with keyframes deliberately placed off the grid. The transform accepts it, and every video check in the doctor passes (the only finding is the optional `missing-episode-record` warning):
```
doctor conforming: True
provenance stamp: gop_preset='vla' gop_seconds='1'
keyframe positions: [0, 7, 8, 90, 91]
actual spacing: [7, 1, 82, 1]
promised spacing: every 30 messages (gop_seconds=1.0 at 30 fps -> gop_frames=30)
```
The file carries a provenance record asserting a 1.0 s keyframe interval ("actually used") while its real spacing is 7, 1, 82, 1, and `hflow doctor` reports it as conforming. Consumers following the `vla` read pattern ("a keyframe seek per sample") see a 12x swing in seek cost between neighboring windows, and the record that would tell them says 1.0 s.
## Why now
#373 closed this family for B-frames: a stated FORMAT.md constraint is now checked in doctor and asserted by the encoder. Fixed GOP is the remaining constraint with no check anywhere, and the only one whose provenance field asserts a measurement. The per-message NAL walk that #354/#358 made cheap already yields `is_keyframe` per access unit, so measuring cadence rides an existing pass rather than adding a new one.
## What to build
Open call, not assumed:
1. **Refuse**: `SourceNotConforming` (as B-frames are) when pass-through keyframe spacing deviates from the stamped interval. Risk: pass-through cannot be re-gridded without transcoding (v1 refuses), so this may reject real recordings.
2. **Measure and stamp**: stamp the measured interval, keeping configured and measured distinguishable. Provenance semantics change, so per the rule stated on #365, cache/provenance identity must move with it.
3. **Doctor finding**: a new code (for example `video-keyframe-cadence`) reporting measured spacing against the stamped interval, leaving refusal policy to callers.
Any combination (measure always, refuse on gross deviation) is also plausible.
## Definition of done
1. Pass-through keyframe cadence is measured from the access units the transform and doctor already walk.
2. Provenance never asserts an unmeasured interval: either the stamp reports the measurement or the deviation is reported.
3. Cadence deviations surface with a documented finding-code row.
4. The re-encode path is unchanged: the encoder still enforces and asserts the grid.
5. Existing conforming fixtures are unaffected; the fixture above lands as a test.
## Non-goals
- Transcoding pass-through video
- Changing the GOP presets or BENCHMARKS.md
- New doctor severity levels
## Validation
```bash
uv run ruff check --fix
uv run ruff format
uv run ty check
uv run pytest -q tests/test_transform.py tests/test_doctor.py tests/test_video.py
uv run pytest -q
```
Contributor guide
Research direction
Read _validate_passthrough_video_payload in src/hflow/transform.py, _check_video_payload in src/hflow/doctor.py, and the provenance write at src/hflow/transform.py:895-896. Run the listed transform, doctor, and video tests, then add the off-grid fixture from the issue to establish the current behavior. Resolve whether cadence is refused, measured into provenance, or reported as a doctor finding, while preserving the re-encode path and existing fixtures.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- data-engineering
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100