Skip to content

Fix Sequence Header film-grain rewriting with padded AV1 OBUs - #44

Open
rampageX wants to merge 1 commit into
rust-av:mainfrom
rampageX:fix/sequence-header-film-grain-pr
Open

Fix Sequence Header film-grain rewriting with padded AV1 OBUs#44
rampageX wants to merge 1 commit into
rust-av:mainfrom
rampageX:fix/sequence-header-film-grain-pr

Conversation

@rampageX

Copy link
Copy Markdown

Fix Sequence Header film-grain flag rewriting with padded AV1 OBUs

Summary

This fixes grav1synth apply corrupting some AV1 streams when
film_grain_params_present is not located in the final byte of the declared
Sequence Header OBU payload.

Affected streams can fail with:

obu_forbidden_bit out of range: 1, but must be in [0,0]
Failed to read temporal unit from packet
Error applying bitstream filters to an output packet

Reproduction

One reproducible NVENC AV1 stream used a 3840x1606 Sequence Header.

Before rewriting:

obu_size = 15
film_grain_params_present = 0

After the old write path:

obu_size = 16
film_grain_params_present = 0

The next OBU was then parsed at the wrong byte boundary and its
obu_forbidden_bit became 1.

The clean AV1 stream validates before grav1synth apply, so corruption occurs
during Sequence Header rewriting.

Root cause

There are two related assumptions in the existing write path:

  1. sequence.rs uses obu_out.last_mut() when toggling
    film_grain_params_present.

    This assumes the flag is always located in the last byte of the declared
    Sequence Header payload. That is not true when trailing/alignment bytes are
    present.

  2. obu.rs adjusts the Sequence Header obu_size by comparing the number of
    bytes written with the number of syntax bytes consumed.

    The syntax parser can stop before trailing/alignment bytes, even though the
    complete declared payload is mirrored into the output. This can falsely make
    the Sequence Header appear to have grown.

Fix

  • Compute the real payload-relative byte/bit position of
    film_grain_params_present using TraceCtx::pos().
  • Modify that exact byte instead of obu_out.last_mut().
  • Treat Sequence Header rewriting as length-preserving and keep its declared
    obu_size unchanged.

Validation

  • Existing regression suite passes except for the pre-existing
    film_grain_params_key_frame_forces_update_and_uses_420_zero_luma_shortcut
    test, whose fixture writes grain_scaling_minus_8 = 2 but expects
    scaling_shift == 8 instead of 10.
  • The previously failing AV1 stream is the primary reproduction case for this
    fix.

Scope

The change is limited to:

src/parser/sequence.rs
src/parser/obu.rs

No container, encoder, preset, or grain-model behavior is changed.

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