feat(isobmff)!: model and write top-level boxes so a C2PA uuid box has somewhere to live - #498
Open
justin13888 wants to merge 14 commits into
Open
feat(isobmff)!: model and write top-level boxes so a C2PA uuid box has somewhere to live#498justin13888 wants to merge 14 commits into
justin13888 wants to merge 14 commits into
Conversation
`IsoBmffImage::new(major_brand, compatible_brands, primary_item_id, items)` plus `with_minor_version` and `with_groups` build a model without a struct literal, so consumers stop depending on the exact field set ahead of the type becoming `#[non_exhaustive]` when it gains a top-level box list (#443). Refs #443
`IsoBmffImage` gains `top_level_boxes: Vec<TopLevelBox>`, in file order, holding every top-level box of the primary stream the model does not otherwise own — anything but `ftyp`, `meta` and `mdat` — with the `uuid` user type split off the payload exactly as `RawBox` does. This is where a C2PA `ContentProvenanceBox` lives; until now `read` discarded it and `write` had nowhere to emit one. Each box carries a `TopLevelPosition`: `AfterFtyp` boxes are written between `ftyp` and `meta`, so before the first `mdat` (C2PA 2.4 §A.5.3); `Trailing` boxes after `mdat`. `read` assigns the position from where it met the box (after `mdat` → `Trailing`, otherwise `AfterFtyp`), so a file this crate wrote re-serialises byte-identically, and a foreign box found between `meta` and `mdat` is written back before `meta` — the one reordering, documented. `moov`/`trak` stay `Unsupported` on both sides; `write` also rejects a box typed `ftyp`/`meta`/`mdat` or whose `user_type` does not pair with the `uuid` type. A `uuid` inside `meta` is never promoted to the top level. `walk_segments` is unchanged and still tiles a file carrying such boxes. The struct is now `#[non_exhaustive]`; `IsoBmffImage::new(..)` with `with_minor_version`/`with_groups`/`with_top_level_boxes` replaces the literal, so the next field is a minor release. `TopLevelBox::new`/`::uuid` /`with_position` build the boxes. BREAKING CHANGE: `IsoBmffImage` is `#[non_exhaustive]` and gains `top_level_boxes`; struct literals outside the crate no longer compile — construct it with `IsoBmffImage::new(..)` and the `with_*` builders. Refs #443
Remux a libavif corpus file through gamut-isobmff with a top-level `ContentProvenanceBox` (a `uuid` box with the C2PA 2.4 §A.5.1 user type, placed after `ftyp` per §A.5.3) added to `top_level_boxes`, and check that libavif decodes the result to exactly the original's pixels — the oracle for the placement being invisible to a conforming reader. Refs #443
…load `write` promised to reject a box at or beyond 4 GiB, but the top-level check measured only the payload: a trailing payload within 24 bytes of 4 GiB passed and `end_box` wrote a wrapped size field. Bound `payload.len().saturating_add(TOP_LEVEL_HEADER_MAX)` (8-byte header + the 16-byte uuid user type, applied to every box type) against `u32` instead — a method call, not an arithmetic operator, so the check has no mutant that only a 4 GiB allocation could kill. Refs #443
Every other enum in the crate is `#[non_exhaustive]`, and this release already pays a major bump to make `IsoBmffImage` so; adding the obvious future position (between `meta` and `mdat`) or a field to `TopLevelBox` must not cost a second one. `TopLevelPosition` keeps `#[repr(u8)]` with permanent append-only discriminants; tests build values through `TopLevelBox::new`/`::uuid`/`with_position`. Also correct the placement claim in the docs: C2PA 2.4 §A.5.3 requires only after `ftyp` and before the first `mdat`/any `moov`, so a box a foreign file put between `meta` and `mdat` was lawfully placed, and the round-trip moves it to another lawful position rather than fixing an unlawful one. STATUS qualifies the byte-identical claim to files this crate wrote and records the two accepted costs of retaining boxes (an owned copy of e.g. a motion-photo `mpvd`; an `iloc` extent into a retained box is carried twice). Refs #443
…uilder `IsoBmffImage::with_minor_version` had no caller; a round-trip with a non-zero minor version covers it and pins the field's carriage. Refs #443
Where the uuid box lands is pinned exact-byte in gamut-isobmff's `tests/top_level.rs`; the libavif test asserts only that the decoded pixels are unchanged. Refs #443
…_level_box `write` partitions `top_level_boxes` by position — every AfterFtyp box lands before meta, every Trailing box after mdat — so the file cannot record the model's order across the two groups, and `read` handed an interleaved list `[free Trailing, uuid AfterFtyp]` back re-grouped as `[uuid, free]`: the documented `read(&write(&img)) == img` contract was false for a model `write` accepted. Realistic trigger: appending a C2PA uuid box to a parsed model that already carries a trailing box. `validate` now rejects an AfterFtyp box that follows a Trailing one with `InvalidInput` (the writer's existing cannot-round-trip policy), and `IsoBmffImage::push_top_level_box` inserts a box at the end of its position group so a caller appending to a parsed model never builds an interleaved list. Both are pinned: the interleaved model is refused with the named error, and pushing an AfterFtyp box onto `[Trailing]` yields `[AfterFtyp, Trailing]` and round-trips byte-identically. The writer, reader, model and crate docs state the grouping rule. Refs #443
Every AfterFtyp push in the suite landed at index 0, so the "after the last AfterFtyp box" arm was never exercised; push onto `[a, t]` and `[a, a, t, t]` and pin `[a, new, t]` / `[a, a, new, t, t]`. The test now asserts list order alone — the grouped-list round-trip is already pinned by `boxes_keep_model_order_within_each_position` and `tests/roundtrip.rs`. Refs #443
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
IsoBmffImagegainstop_level_boxes: Vec<TopLevelBox>— every top-level box of the primary stream the model does not otherwise own (anything butftyp/meta/mdat), in file order, with theuuiduser type split off the payload exactly asRawBoxdoes. This is where a C2PAContentProvenanceBoxlives; until nowreadsilently discarded such boxes (reader.rs_ => {}) andwritehad nowhere to emit one.TopLevelPosition:AfterFtypboxes are written betweenftypandmeta(so before the firstmdat, C2PA 2.4 §A.5.3);Trailingboxes aftermdat.readassigns the position from where it met the box (aftermdat→Trailing, otherwiseAfterFtyp), so a file this crate wrote re-serialises byte-identically; a foreign box betweenmetaandmdatis written back beforemeta— a move between two lawful §A.5.3 positions, documented inTopLevelPositionand STATUS.md.writerejects a top-level box typedftyp/meta/mdat(InvalidInput),moov/trak(Unsupported, as on read), one whoseuser_typedoes not pair with theuuidtype, one whose complete box (header included) does not fit the 32-bit size field, or atop_level_boxeslist that interleaves positions (anAfterFtypbox after aTrailingone — the file cannot record that order, soreadcould not reproduce it);IsoBmffImage::push_top_level_boxappends at the end of a box's position group so a parsed model never becomes interleaved (review round 1, decision 14/4a). Auuidinsidemetais never promoted.walk_segmentsis unchanged and still tiles a file carrying such boxes.IsoBmffImageis now#[non_exhaustive];IsoBmffImage::new(major_brand, compatible_brands, primary_item_id, items)pluswith_minor_version/with_groups/with_top_level_boxesreplace the struct literal, so the next field is a minor release.TopLevelBoxandTopLevelPositionare#[non_exhaustive]too;TopLevelBox::new/::uuid/with_positionbuild the boxes. Every literal site in the workspace (gamut-avif encoder/tests/example, gamut-heic tests/example, gamut-cliisobmff build, the isobmff bench and test fixtures) moved to the constructor in commits that each leave the workspace compiling.uuidbox lands right afterftyp, beforemeta/mdat(tests/top_level.rs); (b)write(&read(&f)) == ffor a file carrying boxes at both positions; (c)walk_segmentstiles the written file with noTrailer/unclassified run; (d) libavif decodes an AVIF carrying the C2PA box to identical pixels (gamut-avif/tests/remux_roundtrip.rs).No human approved this plan: this is an unattended run, and the
## Decisions takenrecord below is what a human reads afterwards.Validation
Every workspace-wide gate ran inside
systemd-run --user --scope -p MemoryMax=16G -p MemorySwapMax=0withCARGO_BUILD_JOBS=2 CMAKE_BUILD_PARALLEL_LEVEL=2andulimit -v 12000000, in the lane's worktree, logs under a lane-unique directory.Round 3 (head
c8b87cf, after the review-round-2 Lows, test/docs only):push_top_level_box_appends_within_the_position_groupnow pushes onto[a, t]→[a, new, t], then[a, a, t]+ t →[a, a, t, new], then[a, a, t, t]+ a →[a, a, new, t, t]— the "after the last AfterFtyp box" arm is reached (replacing it with index 0 fails) — and asserts list order only; the grouped-list round-trip stays pinned byboxes_keep_model_order_within_each_positionandtests/roundtrip.rs.reader.rsreaddocs now linkIsoBmffImage::top_level_boxesinstead of the deadself#top-level-boxesanchor (readeris private).cargo test -p gamut-isobmff --all-features— pass, 13 test binaries.RUSTDOCFLAGS=-D warnings cargo doc -p gamut-isobmff --no-deps— fails on one pre-existing link only:reader.rs:59self#motion-photo-tolerance(present on master before this PR); the link this PR added is fixed. Out of scope here — see Unresolved review notes.mise run check-commits— "no errors in 14 commits";mise run check-tests— conforms;__CARGO_TEST_ROOT=<worktree> mise run fmt-check— pass.Round 2 (head
2e7f2ef, after the review-round-1 repair):writeaccepted[free Trailing, uuid AfterFtyp]andreadreturned[uuid AfterFtyp, free Trailing]; mechanism:write_top_levelpartitions by position, so the bytes never encode cross-group model order.cargo test -p gamut-isobmff --all-features— pass (13 test binaries; newwriter::interleaved_top_level_positions_are_rejected,top_level::push_top_level_box_appends_within_the_position_group;boxes_keep_model_order_within_each_positionfixture regrouped, same claim).cargo test -p gamut-avif --all-features --test remux_roundtrip— pass 2/2 (the oracle test now appends viapush_top_level_box).cargo clippy -p gamut-isobmff --all-targets --all-features -- -D warnings— pass.mise run lint— exit 0.mise run test— exit 0, 203 test binaries green.mise run mutants-diff --budget 16(diff vs origin/master) — 26 mutants tested in 3m: 20 caught, 6 unviable, 0 missed, 0 timeouts.mise run check-commits— "no errors in 12 commits";mise run check-tests— conforms;__CARGO_TEST_ROOT=<worktree> mise run fmt-check— pass (incl. fmt-tooling-check).Round 1 (head
d3be07a):cargo test -p gamut-isobmff -p gamut-avif --all-features— pass.mise run lint— exit 0.mise run test— exit 0, 203 test binaries.mise run check-commits— 10 commits clean.mise run check-tests— conforms.__CARGO_TEST_ROOT=<worktree> mise run fmt-check— pass (without the prefix the task fails in this nested worktree before formatting anything:cargo metadata"current package believes it's in a workspace when it's not" for everytooling/*path-dep —unavailablethere; per-membercargo +nightly fmt --check -p <32 members>also passed).mise run mutants-crate gamut-isobmff --shard 1/2 --budget 16and--shard 0/2— 171 + 171 mutants: 318 caught, 24 unviable, 0 missed, 0 timeouts (the whole crate).mise run coverage(no newsrc/module; CI Coverage gate is the floor),check-release-deps/check-ffi-*(no Cargo.toml or C-surface change).Risks and rollout
gamut-isobmff(feat(isobmff)!:with aBREAKING CHANGEfooter): struct literals ofIsoBmffImageoutside the crate no longer compile. release-plz will bump the major; gamut-avif/gamut-heic/gamut-cli are migrated in this PR. The constructor makes this the last literal break.readnow retainsfree/skip/vendor/uuidtop-level boxes that it previously dropped, sowriteof a parsed foreign file carries them (and moves one found betweenmetaandmdatto beforemeta). A libavif corpus file remuxes to identical pixels with and without an addeduuidbox (oracle test).gamut-avif/gamut-heiccontainers account bytes throughwalk_segments, which is untouched. A retained box is an owned copy (a motion-photompvdincluded), bounded by the input size — recorded in STATUS.box_purpose, the merkle offset or JUMBF —gamut-heic'sc2pamodule keeps that lens (gamut-heic: locate the C2PA manifest store and report its byte range #429).Issue
Closes #443
Decisions taken
Appended by this lane (same shape):
Unresolved review notes
From the independent review pass before the PR opened; each declined with its evidence, none blocking.
readnow copies every retained top-level box, including a motion-photompvd(whole video). Declined: record decision 4 retains every box the model does not own, and an opt-inread_with(..)or anmpvdcarve-out is a public-API fork the record did not open. Bounded by input size; documented under STATUS "Top-level boxes" as an accepted cost. A follow-up could add a read option if a consumer measures the cost.C2PA_UUIDis spelled in gamut-heic and in three test/doc sites here; own it in gamut-isobmff. Declined:references/c2pa/README.md's clause map assigns the §A.5.1 user type to gamut-heic and only §A.5.3 placement to gamut-isobmff, and decision 2 rejected a C2PA-specific surface in the container crate; the test copies cannot depend on gamut-heic (dev-dep topology).ilocextent addressing bytes inside a retained box is carried twice on write. Documented in STATUS as an accepted cost (no known encoder does this; the file decodes identically).gamut isobmff inspectdoes not printtop_level_boxes. Out of this PR's manifest (gamut-cli is touched for constructor fallout only); a six-line follow-up in gamut-cli.reader.rsreaddocs linkself#motion-photo-tolerance, butreaderis a private module, soRUSTDOCFLAGS=-D warnings cargo doc -p gamut-isobmfffails on master and on this branch alike. Not touched here (it predates gamut-isobmff: model and write top-level boxes, so a C2PA uuid box has somewhere to live #443 and is not in the manifest); a one-line follow-up.writer.rs,TOP_LEVEL_HEADER_MAXcheck) has no test that reaches it. Reaching it needs a ~4 GiBVecfixture, which is not portable across CI runners; the pre-existing item-payload and file-offset 4 GiB bounds in the same writer share exactly this status, so this matches crate convention. The check is a method call (u32::try_from(..).is_err()), so it contributes no mutation site; its body-replacement mutant (validate_top_level -> Ok(())) is caught by the type/user-type rejection tests.