The two slice-header walks still duplicate their loop and their Exp-Golomb decoder

[kstonekuan]

Current behavior

count_h264_pictures (src/hflow/video.py:347) and scan_picture_coding_types (src/hflow/video.py:419) walk the same NALs the same way. After #354 the shared body is twelve identical lines in each:

nal_offsets_and_types = _annex_b_nal_offsets_and_types(stream)
for nal_index, (nal_start_offset, nal_type) in enumerate(nal_offsets_and_types):
    if nal_type not in _SLICE_HEADER_NAL_TYPES:
        continue
    nal_end_offset = (...)
    nal_header_offset = _nal_header_offset(stream, nal_start_offset)
    slice_payload = stream[nal_header_offset + 1 : nal_end_offset]
    rbsp = _unescape_ebsp_head(slice_payload, _SLICE_HEADER_HEAD_BYTES)
    # ... decode, and on failure re-unescape the whole payload and decode again

They then diverge into two Exp-Golomb decoders that must agree:

• _decode_first_mb_in_slice (:289) raises, with two distinct messages that hflow doctor and the AUD repair path depend on.
• _decode_unsigned_exp_golomb_at (:319) plus _slice_header_fields (:380) return None, and the scan turns that into one message.

The head-then-fallback logic #354 introduced is written out in both, so the duplication grew rather than shrank.

count_h264_pictures(stream) is exactly scan_picture_coding_types(stream).picture_count. Verified across sixteen inputs: real encodes at 320x240 and 1280x720, bframes=0 and bframes=3, multi-slice, synthetic streams with emulation-prevention triples straddling the head boundary, and first_mb_in_slice from 0 to 139263. The two agree on every one.

Why this is worth doing

Two decoders over the same bitstream field, kept in step by nothing but care. They already differ in failure style (raise versus None), which is the part that makes them look like separate concerns when they are not. A future change to the head budget, the fallback, or the NAL filter has to be made twice and can be made inconsistently, and the inconsistency would be silent: both still return a number, just different ones.

This was DoD 4 on #346 and did not land with #354, which is on me for putting a "while in there" item into a definition of done. It is its own change.

What to build

One walk that yields (first_mb_in_slice, slice_type) per slice NAL, with the two public functions as thin consumers. count_h264_pictures keeps its two specific messages, so the shared piece has to let a caller choose how a short or truncated header is reported: returning an explicit outcome the callers translate is one way, passing the messages in is another. Say which you picked and why.

Definition of done

1. One NAL walk and one Exp-Golomb decoder serve both functions.
2. count_h264_pictures raises byte-identical messages for a header with no complete first_mb_in_slice and for a truncated one. The existing tests cover both; they must pass untouched.
3. scan_picture_coding_types raises its existing single message for the same inputs.
4. The head-then-fallback path exists once.
5. Results are unchanged on canonical, B-frame, multi-slice, and escape-heavy streams. Adding a case to tests/test_video.py that asserts scan_picture_coding_types(s).picture_count == count_h264_pictures(s) pins the equivalence directly; perf(video): unescape only the slice-header head, not the whole NAL #354 added one, extend it rather than duplicating it.
6. No measurable regression against the perf(video): unescape only the slice-header head, not the whole NAL #354 timings. hflow doctor and the remux both sit on this path.

Validation

uv sync --locked --all-extras
uv run ruff check --fix
uv run ruff format
uv run ty check
uv run pytest -q tests/test_video.py tests/test_video_malformed_guards.py \
  tests/test_processing_regressions.py
uv run pytest -q

Run plain uv run pytest -q from the repo root: pytest tests alone misses packages/hflow-server/tests.

Notes

Not a good first issue: the error-message compatibility is the whole difficulty, and getting it wrong changes hflow doctor output on malformed input.

[Sagar-024]

Taking this , follow-up to my #354 merge. Will collapse the two
walks and unify on the offset-aware decoder, translating the two
raising messages to the call sites that need them.