feat: path to a full rawshift-video crate - #68
Open
justin13888 wants to merge 13 commits into
Open
Conversation
gamut has declared video permanently out of scope ("gamut will not grow
video primitives"), and gamut-isobmff parses the HEIF still-image item
model rather than the movie model, so it cannot back MP4/MOV video. The
Upstream-First Policy read as though it governed video work too, which
would have blocked rawshift-video on issues gamut would decline.
Record the carve-out, and with it the decisions it implies:
- Video containers and codecs take third-party dependencies judged by the
PRINCIPLES.md maturity rule rather than the upstream-first rule.
- FFmpeg/libav is excluded in any form, on license (LGPL reach into an
MPL-2.0 library with no iOS relinking path) and portability grounds
(autotools + nasm cannot satisfy the wasm32 and mobile build lanes).
- Software H.264 joins software HEVC as never: both are patent-encumbered
independently of an implementation's own license, and the OpenH264
royalty grant covers Cisco's prebuilt binary rather than a from-source
or clean-slate build. Hardware decoders carry the OEM's licence.
- The hardware decode matrix gains H.264 on the sequence seam only, and
states that container parsing and metadata keep working on every target
with no backend at all.
Also fixes the safety boundaries for the crates this path introduces.
Refs #39
rawshift-core's module docs already claim ImageMetadata and its MetadataNamespace parts are "genuinely video-shared". rawshift-video is the first consumer of that claim, and it needs somewhere to put container-level facts: moov-tree values and the com.apple.quicktime.* udta/mdta keys iOS writes, and Matroska Tags/Info/TrackEntry elements. BREAKING CHANGE: MetadataNamespace is now #[non_exhaustive] and has two new variants. Adding a variant to an open enum is itself breaking, so both happen once, now, while the workspace is pre-1.0 — after which new namespaces are additive forever. Downstream matches need a `_` arm. No in-tree match on MetadataNamespace was exhaustive, so no other crate changes. Refs #39
The video counterpart to rawshift-image-core: errors, format and codec identity, the track/packet/frame model, container metadata, and the demux/decode contracts. No I/O, no platform code, no container or codec dependencies, #![forbid(unsafe_code)]. It has to be its own crate rather than a reuse of rawshift-image-core because CI enforces that no rawshift-image* crate appears in rawshift-video's dependency tree; image and video share only rawshift-core. Design points worth calling out: - Identity enums name more than rawshift can decode (AVI, MXF, ProRes), so "recognised but unsupported" is expressible and adding support later is additive. All are #[non_exhaustive]. - VideoError separates a capability gap from a broken file, and distinguishes "no decoder compiled in" (UnsupportedCodec) from "no backend on this machine" (HwDecoderUnavailable) — callers act on those differently. Backend error types are stringified at the leaf so none crosses a public boundary. - Colour is stored as raw CICP code points rather than typed enums, so a code point no library names yet survives instead of collapsing to "unspecified", with resolved_matrix() applying the universal SD-means-BT.601 / HD-means-BT.709 fallback. - VideoDecoder splits send_packet from receive_frame because coded and display order differ whenever a stream reorders, and because that is the shape every hardware decode API natively has. - DecoderRegistry makes supports()==false the only signal that falls through to the next backend, so a backend that accepts a track and then faults is reported rather than silently masked by a later one. This mirrors gamut-codec-abi's documented fallback contract. The track structs deliberately do not derive serde: they embed upstream gamut geometry and colour types that have no serde support, and skipping those fields would serialize a track that lost its resolution and pixel format. Refs #39
Add NAL framing for the H.264/HEVC families: length-prefix width from the avcC/hvcC record, an iterator over an access unit's NAL units, and is_random_access_point. This exists because the chosen demux backend exposes no sync-sample flag at all — not on its packet type, not on its track type; MP4's stss table is parsed only for its internal seeking. Reading IDR (H.264 NAL type 5) and IRAP (HEVC NAL types 16..=23) out of the bitstream is the only source available, and is the better one regardless: container sync tables are a well-known source of wrong keyframe flags. Framing and NAL headers only — no slice headers, no parameter sets. It lives in the vocabulary crate because both the demuxers and the decoders walk this framing. Truncated prefixes and payloads terminate the iterator cleanly rather than yielding a short NAL, since containers are untrusted input and a truncated tail is what a damaged file looks like. Refs #39
Wraps symphonia-format-isomp4 (MPL-2.0, pure Rust, no build script, so it builds on every target in docs/SUPPORT.md) and maps its tracks, packets and seeking onto rawshift's model. No symphonia type reaches the public API; its errors are flattened into VideoError::Container at the boundary. The backend leaves three gaps this crate fills: - Rotation. It does not expose the tkhd display matrix, so a focused header walk reads it. Without this most phone video presents sideways, since phones record in the sensor's orientation and correct in the container. - ftyp brands and mvhd timescale/times, needed for metadata and to tell MP4 from QuickTime. - Random access points, which come from the bitstream via rawshift-video-core rather than from a sync-sample table the backend does not expose. The header walk is deliberately not a demuxer and must not become one: it reads four small boxes, bounds-checks every field, and caps recursion depth and box size, because it parses untrusted bytes. Malformed structure returns what was recovered rather than failing the open, since these facts are supplementary and such a file should still demux. Two limitations are documented rather than papered over: ISOBMFF colour lives in a colr box the backend does not surface, so tracks report unspecified colour and resolve by picture height; and ProRes tracks are invisible because the backend does not recognise ap4h/apch/apcn sample entries. Adds constructors to the video-core track types, which are another crate. Refs #39
…bridge
Adds MKV/WebM support and extracts rawshift-video-symphonia, the one
translation of symphonia's tracks, packets, timestamps and seek
vocabulary into rawshift's model. Both container crates now share it
rather than carrying copies that drift; it follows the established
"focused support crate" pattern of rawshift-image-{metadata,ifd,ljpeg}.
No symphonia type leaves that crate, which is what lets the container
crates keep the same guarantee.
Fixes an end-of-stream bug found by running the demuxers against real
ffmpeg-generated files: the two readers disagree about how a stream ends.
The ISOBMFF reader returns Ok(None) while the Matroska reader raises an
UnexpectedEof I/O error, so every Matroska file ended in a spurious
"unexpected end of file". Both now map to rawshift's Ok(None), and only
UnexpectedEof does — a permission error or a malformed atom still fails.
Verified end to end against ffmpeg-generated fixtures via the new probe
examples: H.264 in MP4, MOV and MKV, and HEVC in MKV, all yield correct
tracks, dimensions, durations, config records and packet counts, with
random access points matching the encoder's GOP length.
Matroska states rotation in Video/Projection, which the backend does not
surface, so tracks report Rotation::None. That costs nothing in practice:
rotation is a phone-capture concern and phones write MP4 or QuickTime.
Refs #39
Replaces symphonia-format-isomp4 with mp4-atom plus a rawshift-owned
sample index, track model and demuxer.
The trigger was a hard blocker found by running the demuxer against real
files: symphonia-format-isomp4 caps the hvcC configuration record at 1 KB
("It should not exceed 1 kB"), and real HEVC records are 2-3 KB — the
plain x265 fixture here is 2440 bytes. The error aborts the open, so
every HEVC MP4 and MOV failed outright. That is XAVC HS and default
iPhone video, the two highest-priority formats on the roadmap, and it
could not be worked around from outside.
That reader is audio-first, and the same investigation found it hides
four more things video needs: the tkhd matrix (rotation), the colr box
(all colour signalling), the sync-sample table, and ProRes-shaped sample
entries. mp4-atom is a box codec that does no interpretation, so rawshift
reconstructs the sample index from the stbl tables itself — a few hundred
lines that remove all five limitations at once.
Also fixes timestamps. The edit list is now applied, so presentation
starts at zero; without it every timestamp carried the encoder's reorder
delay and disagreed with every other tool. rawshift's first PTS now
matches ffprobe's on the same file.
Verified against ffmpeg-generated fixtures: H.264 and HEVC in both MP4
and QuickTime now yield correct tracks, dimensions, durations, frame
counts, config records and timestamps, with random access points matching
the encoder's GOP length.
The sample-table walk is total against untrusted input: an inconsistent
or truncated table stops the walk and yields what was recovered, and no
byte range can point outside the file. Fragmented MP4 is rejected with a
clear error rather than presented as a track with no samples; camera
files are not fragmented.
BREAKING CHANGE: IsoBmffDemuxer is now generic over its source and no
longer requires Send + Sync + 'static, since samples are read on demand
rather than through a boxed media source.
Refs #39
Locks in the behaviours that synthetic-table unit tests cannot reach: that rawshift agrees with the rest of the world about a file ffmpeg wrote. Covers track and dimension recovery, complete sample delivery, decode ordering, composition offsets actually being applied, seeks landing on a resumable point, and rejection of non-ISOBMFF and truncated input. Two are regression tests for bugs found by running against real files rather than by reading code: HEVC opening despite a multi-kilobyte hvcC, and the first PTS matching ffprobe's once the edit list is applied. Fixtures are generated at test time and cached in a temp directory rather than committed, per TEST_FIXTURES.md. Generation stages to a unique name and renames into place: tests run in parallel and share the cache, so writing directly to the final path made it exist — and so look ready — while ffmpeg was still filling it, and other tests opened a half-written file. Verified that the suite passes with no ffmpeg on PATH, skipping rather than failing. Refs #39
Adds the API for decoding a coded video sequence alongside the existing still-frame one: VideoConfig, VideoPacket, VideoFrame, the HwVideoDecoder trait, video_decoder() and available_video_codecs(). The two seams stay separate rather than one being rewritten as the other. The still path is a shipping API that HEIC and AVIF depend on and that has device tests behind it; re-expressing it on the sequence path would risk regressions in the image crates for no user-visible gain. What they share is the VAAPI plumbing, not the policy. Decoding is send-packet / receive-frame rather than decode(packet) -> frame, because coded order and output order differ whenever a stream reorders, so one packet may make zero, one, or several frames available. It is also the shape every hardware decode API natively has. The decoder owns the picture buffer and emits display order: VAAPI is a slice-level API where the caller supplies the reference lists, so the buffer and the ordering cannot live above it, and pushing them onto callers would mean exposing picture order counts and buffer fullness across the API. video_decoder returns a Result rather than an Option, unlike decoder(): the caller supplies a real configuration record, so there is a specific reportable reason for failure that a consumer needs for its own errors. BREAKING CHANGE: HwCodec gains H264. The enum is documented as deliberately exhaustive, so this is that decision taken explicitly. H.264 is reachable only through the sequence seam — decoder(HwCodec::H264) returns None and available_codecs() never lists it — because no rawshift still format uses H.264 and offering an untested still path would misreport what the crate can do. Refs #39
Implements the sequence seam on the VAAPI backend: configuration records validated at open, H.264 profiles added to the runtime probe, and available_video_codecs() answering from what the driver actually exposes. Scope is random access points — every IRAP (HEVC) and IDR (H.264) access unit decodes; anything referencing other pictures is refused with a clear error rather than decoded into a plausible-looking wrong picture. That is a real capability rather than a stub: keyframe extraction, poster frames, timeline thumbnails and scrubbing all work, and All-Intra camera modes (Sony XAVC S-I, XAVC HS All-I) decode completely because every access unit in them is a random access point. Inter-frame decode is a purely internal change behind this unchanged API — it adds a decoded picture buffer, picture order counts, reference marking and reference lists, and drops the rejection. No part of the public seam moves, so it needs no second breaking release. Verified on an AMD RX 7900 XT (radeonsi, VA-API 1.23): a libx265 keyframe decodes to correct dimensions with non-blank pixels and its timestamp carried through, receive_frame reports "not ready" rather than failing, flush invents nothing, and reset discards pending output while leaving the session usable. Also extracts the device-test helpers into tests/common. Writing a second copy of the hvcC builder from memory put numOfArrays one byte off, which yields a record that parses but carries no parameter sets — the failure was "stream carries no SPS" on a stream that plainly had one. One copy, in one place, with a note saying why. Refs #39
Adds rawshift-video-hwdec, which binds the video decoder contract to rawshift-hwdec's sequence seam, and fills in rawshift-video: VideoFile, container detection, probe, the decoder registry, available_decoders, hw_decode_available, the prelude and the feature tree. One crate covers both codecs rather than two leaves. The image side splits per format because each wraps a different codec library; here both go through the same seam and the same backend and would differ only in which configuration record they carry, so the h264 and hevc features give a build the same narrowing without the duplicate crate. The feature tree separates container from codec, which the placeholder's tiers conflated. xavc-hs and xavc-s survive as device-oriented aliases so the documented roadmap names still resolve. A container feature with no codec feature is a valid metadata-and-timeline build, the same shape as heic/avif without hw on the image side. VideoFile is not generic over its reader — the containers box their sources internally, so carrying R would constrain callers for nothing — and its bound hides behind a sealed VideoSource trait, because the bounds are the Matroska backend's rather than rawshift's and should be relaxable without a break. Narrows what the VAAPI backend advertises. The probe finds this machine's driver decodes H.264, and the earlier code advertised it, but the crate has no H.264 picture-parameter path yet, so opening a session failed after discovery had promised it would work. available_video_codecs() now reports only HEVC, open() explains that H.264 is unimplemented rather than absent, and the device test proves the promise by opening a session for every codec discovery advertises. Verified end to end on an AMD RX 7900 XT: an HEVC MP4 goes from file to container to track to a decoded 128x96 NV12 frame through the public API alone, while H.264 and backend-less targets report a capability gap that callers can act on and keep full container, track and metadata access. Refs #39
rawshift-video has an implementation, so it rejoins the published set. The facade gains a `video` feature, release-plz gains the six video crates and the corrected publish order, and the justfile and README stop describing a parked crate. Video re-exports under `rawshift::video` rather than at the root: the two libraries share names — `prelude`, `Track`, and their error types — so flattening both would collide, and image stays at the root for source compatibility. The video-only CI job is rewritten for a crate that now ships. It keeps the assertion the workspace split exists for (no image crate in the video dependency tree) and adds three more: - No backend type may appear in a public signature. The crates wrap symphonia, mp4-atom and rawshift-hwdec rather than re-exporting them, so swapping a backend is not a breaking change for callers — this is what keeps that true, and it is not a property that survives on goodwill. - The facade's `video` feature pulls the crate in, and an image-only build still does not. - The containers build with no codec features at all, which is what every backend-less target gets. All four assertions were run locally before landing. Pre-existing and unchanged: `cargo publish --dry-run` cannot verify any workspace leaf, because the internal crates are not on crates.io yet and dry-run resolves them from the registry. It fails identically on master for rawshift-image-png. release-plz publishes in dependency order, which is the path that actually works. Closes #36's parking decision. Refs #39
Two denial-of-service vectors, both reachable from a malformed file and
both found by reviewing the diff rather than by a failing test:
- An stsz claiming `Identical { count: u32::MAX }` expanded to a
four-billion-entry vector — roughly 17 GB — during open, before a byte
of media was read.
- A sample size of u32::MAX allocated 4 GB per read_sample call.
The sample tables are attacker-controlled and a sample occupies at least
one byte, so the file length is the honest ceiling on both the sample
count and any single sample's extent. Samples that would run past the end
of the file are dropped at index time, since indexing them could only
produce a failing read later.
Also fixes a doc comment that an earlier unquoted heredoc had eaten.
Refs #39
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
1 participant
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.
Turns
rawshift-videofrom a zero-code placeholder into a working crate: containers, tracks, timelines, metadata, and hardware-backed frame decode, wired into therawshiftfacade behind a newvideofeature.Six crates:
rawshift-video-core(shared vocabulary),-isobmff,-matroska,-symphonia(internal bridge),-hwdec, and therawshift-videofacade.What works
Verified end to end against ffmpeg-generated files and, for decode, on an AMD RX 7900 XT (radeonsi, VA-API 1.23).
rawshift-hwdecUnsupportedAn HEVC MP4 goes from file to a decoded 128×96 NV12 frame through the public API alone. Everything except decode works on every target in
docs/SUPPORT.md— all eight verified compiling here, including iOS, Android, wasm32 and musl.Decisions worth reviewing
gamut does not cover video. Its README states it "will not grow video primitives", and
gamut-isobmffparses the HEIF still-image item model, not the movie model. The Upstream-First Policy read as though it governed video, which would have blocked this on issues gamut would decline.AGENTS.mdnow carves video out.FFmpeg is excluded, reversing the suggestion on #39. It is LGPL-2.1+ against rawshift's MPL-2.0 with no iOS relinking path, and needs autotools + nasm, which cannot satisfy the wasm32 lane and would need cross C toolchains for the mobile lanes. Recorded in
docs/SUPPORT.mdalongside the NVDEC exclusion it mirrors. Software H.264 joins software HEVC as never, for the patent-posture reasons already recorded there.The MP4 backend changed mid-PR. It started on
symphonia-format-isomp4, which caps thehvcCrecord at 1 KB — real HEVC records are 2–3 KB, so every HEVC MP4 and MOV failed to open, i.e. XAVC HS and iPhone video. It also hides thetkhdmatrix, thecolrbox, the sync-sample table and ProRes sample entries. rawshift now owns the sample index onmp4-atom, which removed all five at once. Matroska keeps symphonia, which has none of these problems.docs/SUPPORT.mdgains an H.264 column and a "video sequence" row. That document declares itself fixed, so although this touches neither the permanent target list nor the API list, it is flagged here for sign-off rather than slipped in.Breaking changes
HwCodecgainsH264— the enum is documented as deliberately exhaustive, so this is that decision taken explicitly. Reachable only through the sequence seam.MetadataNamespacebecomes#[non_exhaustive]and gainsQuicktime/Matroska. Both land together while the workspace is pre-1.0.Bugs found by running against real files
Four, none visible from reading code: the
tkhdmatrix offset was wrong (40/52, not 36/48); every Matroska file ended in a spurious EOF error because the two readers disagree about how a stream ends; thehvcCcap above; and timestamps were offset by the encoder's reorder delay until the edit list was applied — rawshift's first PTS now matches ffprobe's.Validation
cargo test --workspacegreen (31 suites);cargo clippy --workspace --all-targetsandcargo fmt --checkclean; MSRV 1.92.0 checks; all eight supported targets compile withvideoand withfull; the fourvideo-onlyCI assertions run locally, including a new one that no backend type appears in a public signature.Pre-existing, unchanged:
cargo publish --dry-runcannot verify any workspace leaf, because the internal crates are not yet on crates.io and dry-run resolves them from the registry. It fails identically onmasterforrawshift-image-png.Not included, deliberately
Inter-frame decode, and H.264's backend path. The public seam is final for both — adding them is an internal change needing no second breaking release.
Follow-ups filed: #69 (inter-frame decode), #70 (H.264 picture parameters), #71 (fragmented MP4), #72 (AVI and MXF), #73 (ProRes), #74 (video encode), #75 (report symphonia's hvcC cap upstream). VideoToolbox and MediaCodec sequence support ride on #28 and #30.
Closes #39