Skip to content

write_access_units_to_mp4 silently drops the reorder tail of any B-frame stream #255

Closed

Description

@kstonekuan

What happens

write_access_units_to_mp4 remuxes raw Annex B into MP4 with -c:v copy. When the stream carries B-frames, the trailing reorder-buffer frames cannot be decoded from the muxed file: the container holds every sample, and the decoder emits fewer.

Measured in #250, layer by layer, with plain ffmpeg and no HFlow code:

  1. Source chunk decodes 11939/11939, zero errors. The exact episode slice (-ss 0 -to 10.1 -c copy) decodes 303/303.
  2. The converter's H.264 output is 303 access units, and decoding the raw Annex B directly yields 303/303.
  3. Through write_access_units_to_mp4's exact command, the container holds 303 samples and the decoder emits 301. Decoded PTS are contiguous 0..300, so the loss is precisely the reorder tail.
  4. Re-encoding the same slice with bframes=0 decodes 303/303 through the identical remux.

-muxdelay 0 -muxpreload 0, -fflags +genpts -avoid_negative_ts disabled, -max_interleave_delta 0, and a two-step raw to MP4 to MP4 remux were all tried. Every one reproduces 301.

Why this is still open after #251

#251 set bframes=0 in the LeRobot converter, so HFlow's own converter stops producing streams that hit this. That fixes the corpus in front of us and nothing else. write_access_units_to_mp4 is reached by any path that hands it H.264, and B-frames are libx264's default and standard on hardware encoders, so a third-party converter or a direct recording still loses its tail silently.

There is a related fact worth holding alongside: docs/FORMAT.md:152 item 4 requires no B-frames in canonical video, and hflow doctor does not check picture coding types (documented in #236). So a B-frame stream is already non-conforming, already undetected, and now also silently lossy. Three gaps in a line.

The decision this needs

Two defensible shapes, and the choice is a design call rather than a patch:

  1. Remux losslessly for arbitrary B-frame input. Preserves whatever the caller hands over. No flag combination found so far does it, so this likely means not using -c copy from a raw demuxer, which is a real change to a path on the canonical write route.
  2. Refuse a stream whose reorder depth cannot be represented, and say so. Consistent with how the transform treats every other canonical violation: it refuses rather than silently transforming. Cheap, and it converts a silent loss into an error naming the cause.

(2) fits the project's existing posture. (1) is more useful to someone whose recorder they do not control. They are not exclusive: refuse now, and lift the refusal if a lossless remux is found.

Whichever is chosen, the measurement half is separate work and is filed alongside this: nothing in the evidence currently reports the shortfall.

Definition of done

Whichever shape is chosen: a B-frame stream through write_access_units_to_mp4 either round-trips every frame, or fails with an error naming the reorder depth and the frames at risk. A test covers a B-frame source end to end and asserts the chosen outcome. If a refusal is added, docs/FORMAT.md says so, and the transform's own refusal list gains it.

Validation

uv sync --locked --all-extras
uv run ruff check --fix
uv run ruff format
uv run ty check
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: it decides behaviour on the canonical write path, and option 1 may not be reachable at all.

Investigated by @ayam04 on #250, which carries the full matrix and every flag combination already ruled out. Start there rather than re-deriving it.

Activity

  1. added
    bugSomething isn't working
    help wantedExtra attention is needed
    advancedNeeds codebase familiarity; not a starter issue
    on Aug 29, 2026
  2. Sagar-024 commented on Sep 1, 2026

    @Sagar-024
    Contributor

    Taking this one. Going with option 2, refusing fits how the transform already treats everything else it can't represent. The error will name the reorder depth and which frames get dropped so nobody has to decode-and-count to find out.

    For detection I'll read the bitstream directly (B slices and reorder depth from the NALs) instead of trying more muxer flag combos, #250 already burned through those. That part is the fiddly one, the rest is a normal guard plus tests.

    Keeping the measurement half out of this PR, that's its own thing per the issue.

  3. added a commit that references this issue on Sep 1, 2026
    57a2e39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    advancedNeeds codebase familiarity; not a starter issuebugSomething isn't workinghelp wantedExtra attention is needed

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions