Skip to content

feat: migrate color / cfa / pixel_format / frame primitives to videoframe 0.2#5

Merged
uqio merged 11 commits into
mainfrom
feat/migrate-to-videoframe
May 15, 2026
Merged

feat: migrate color / cfa / pixel_format / frame primitives to videoframe 0.2#5
uqio merged 11 commits into
mainfrom
feat/migrate-to-videoframe

Conversation

@uqio
Copy link
Copy Markdown
Collaborator

@uqio uqio commented May 11, 2026

Summary

Wires mediadecode to consume duplicate type definitions from the
new `videoframe` crate
(crates.io). videoframe is
the lowest-layer shared vocabulary crate; mediadecode keeps its
decoder/codec abstractions and consumes the pixel-format / color
types via re-exports.

Changes

  • Cargo.toml: adds `videoframe = { version = "0.2", default-features = false, features = ["frame"] }`
    at the workspace root; the `mediadecode` core member consumes it
    via `videoframe = { workspace = true }`.

  • Replaces `mediadecode/src/{color,cfa,pixel_format}.rs` with
    thin re-exports from `videoframe::{color,cfa,pixel_format}` (~725
    LoC of duplicated definitions deleted).

  • `mediadecode/src/frame.rs`: removes the local `Dimensions`,
    `Rect`, `Plane` definitions in favor of
    `videoframe::frame::{Dimensions, Rect, Plane}` re-exports.
    Decoder-output types (`VideoFrame<P, E, D>`, `AudioFrame<S, C, E, D>`,
    `SubtitleFrame<E, D>`) stay in mediadecode — they carry timestamp +
    backend-extras / sample-format / channel-layout, which is mediadecode's
    domain.

PixelFormat::Unknown shape change

videoframe's `PixelFormat::Unknown(u32)` is a tuple variant
(lossless wire round-trip), unlike the prior unit-variant `Unknown`.
Updated 17 sites in `mediadecode-ffmpeg` and `mediadecode-webcodecs`:

  • Boundary mapping fallback: `_ => PixelFormat::Unknown` →
    `_ => PixelFormat::Unknown(raw as u32)` (preserves the raw FFmpeg /
    WebCodecs identifier through the cast).
  • Test assertions: `assert_eq!(x, PixelFormat::Unknown)` →
    `assert!(matches!(x, PixelFormat::Unknown(_)))`.
  • Default-frame placeholders / function-argument bare-`Unknown` sites:
    → `PixelFormat::Unknown(0)`.

Tests

  • `mediadecode` core: 47 + 71 tests pass (workspace lib + mediadecode-ffmpeg lib)
  • `cargo fmt --check`: clean
  • `cargo clippy --workspace --all-features -- -D warnings`: clean
  • `cargo doc`: clean

Breaking changes (pre-publish, acceptable)

  • `PixelFormat::Unknown` is now `Unknown(u32)` — downstream callers
    matching on it must use `Unknown(_)` or `Unknown(raw)`.
  • Existing import paths (`mediadecode::color::ColorMatrix`,
    `mediadecode::PixelFormat`, etc.) continue to resolve via the
    re-exports.

🤖 Generated with Claude Code

@codecov
Copy link
Copy Markdown

codecov Bot commented May 11, 2026

Codecov Report

❌ Patch coverage is 24.17582% with 69 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
mediadecode-ffmpeg/src/error.rs 0.00% 32 Missing ⚠️
mediadecode-ffmpeg/src/decoder.rs 0.00% 14 Missing ⚠️
mediadecode/src/frame.rs 60.00% 14 Missing ⚠️
mediadecode-ffmpeg/src/video.rs 0.00% 8 Missing ⚠️
mediadecode-ffmpeg/src/boundary.rs 50.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

uqio added 3 commits May 12, 2026 00:27
Wires mediadecode to consume duplicate type definitions from the new
videoframe crate via a workspace path dep:

- `mediadecode::color::*`                     → `videoframe::color`
- `mediadecode::cfa::BayerPattern`            → `videoframe::cfa::BayerPattern`
- `mediadecode::frame::{Dimensions, Rect, Plane}` → `videoframe::frame`

Decoder-output types (`VideoFrame<P, E, D>`, `AudioFrame<S, C, E, D>`,
`SubtitleFrame<E, D>`) remain in `mediadecode::frame` — they carry
timestamp + backend-extras / sample format / channel layout, which is
mediadecode's domain (`videoframe` stays the pure pixel-data layer).

Existing import paths resolve via the re-exports so downstream
callers using `mediadecode::color::ColorMatrix` etc. continue to
work — no source-level break.

Note: `pixel_format::PixelFormat` is NOT migrated in this commit.
videoframe's variant is `Unknown(u32)` (lossless round-trip) while
mediadecode uses `Unknown = 0` (unit variant); the two types are
structurally incompatible and require changes to all
`_ => PixelFormat::Unknown` fallback arms in mediadecode-ffmpeg /
mediadecode-webcodecs. Tracked as a follow-up.
…tives)

Wires mediadecode to consume duplicate type definitions from the new
`videoframe` crate via a workspace path dep:

- `mediadecode::color::*`             → `videoframe::color`
- `mediadecode::cfa::BayerPattern`     → `videoframe::cfa::BayerPattern`
- `mediadecode::pixel_format::PixelFormat` → `videoframe::pixel_format`
- `mediadecode::frame::{Dimensions, Rect, Plane}` → `videoframe::frame`

Decoder-output types (`VideoFrame<P, E, D>`, `AudioFrame<S, C, E, D>`,
`SubtitleFrame<E, D>`) stay in `mediadecode::frame` — they carry
timestamp + backend-extras / sample format / channel layout, which is
mediadecode's domain. `videoframe` stays the pure pixel-data layer.

videoframe's `PixelFormat::Unknown(u32)` is a tuple variant
(lossless wire round-trip), unlike the prior unit-variant
`Unknown`. Updated downstream callers in `mediadecode-ffmpeg` and
`mediadecode-webcodecs`:

- Boundary mapping fallback: `_ => PixelFormat::Unknown` →
  `_ => PixelFormat::Unknown(raw as u32)` (preserves the raw FFmpeg /
  WebCodecs identifier through the cast).
- Test assertions: `assert_eq!(x, PixelFormat::Unknown)` →
  `assert!(matches!(x, PixelFormat::Unknown(_)))` (wildcard match
  on the tuple variant).
- Default-frame placeholders / function-argument bare-Unknown sites:
  → `PixelFormat::Unknown(0)`.

Existing `mediadecode::color::ColorMatrix` / `mediadecode::PixelFormat`
import paths continue to resolve via the re-exports — no source-level
break beyond the Unknown variant shape.
…pace layout)

videoframe restructured into a Cargo workspace; the actual lib crate
moved from the repo root to videoframe/videoframe/. Update path dep
to match.
@uqio uqio force-pushed the feat/migrate-to-videoframe branch from 54757cf to e9fbf9e Compare May 11, 2026 12:29
videoframe v0.1.0 was published to crates.io. Switch the workspace
dep spec from the local path (`../videoframe/videoframe`) to the
registry version. Caret semver allows future 0.1.x patches without
further edits.
@uqio uqio force-pushed the feat/migrate-to-videoframe branch from e9fbf9e to eec6dbb Compare May 11, 2026 12:31
videoframe 0.2.0 reorganized the public surface: the `cfa` module
was removed; `BayerPattern` now lives at `videoframe::frame::bayer`
and is re-exported via `frame::*`. Update mediadecode's re-export
in `cfa.rs` to use the new path.

The 0.2.0 release also tightened error shapes (newtype-tuple
variants with thiserror::Error on each payload, ref/ref_mut accessors
on every *FrameError) and the unicode-glyph normalization that
landed alongside, but nothing mediadecode consumes directly changed
in a way that needs further code adjustments — the workspace builds
clean and all 47 + 71 lib tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@uqio uqio changed the title feat: migrate color / cfa / pixel_format / frame primitives to videoframe 0.1 feat: migrate color / cfa / pixel_format / frame primitives to videoframe 0.2 May 13, 2026
uqio and others added 6 commits May 15, 2026 14:56
mediadecode and mediadecode-ffmpeg both move to 0.2.0 alongside the
videoframe migration; mediadecode-webcodecs stays pre-release at
0.0.0.

CHANGELOGs:

- mediadecode/CHANGELOG.md — write the [0.2.0] section covering the
  PixelFormat::Unknown shape change (unit -> Unknown(u32)), the
  re-export pivot for color/cfa/frame primitives onto videoframe,
  and the explicit "decoder-output types stay in mediadecode"
  invariant. ~270-variant PixelFormat (videoframe-sourced, FFmpeg
  n8.1 coverage) called out so consumers know the previously-closed
  enum subset is now a strict superset.
- mediadecode-ffmpeg/CHANGELOG.md — add [0.2.0] noting the bump to
  mediadecode 0.2 + the boundary mapping update preserving the raw
  AVPixelFormat identifier via Unknown(raw as u32).
- mediadecode-webcodecs/CHANGELOG.md — same boundary preservation
  note; crate stays scaffolded pre-publish.

Cargo.toml:

- mediadecode-ffmpeg 0.1.0 -> 0.2.0 (type aliases shift with the
  PixelFormat shape change, so the version must move with mediadecode).

READMEs:

- root README crates table — add mediadecode-webcodecs row, add a
  note about videoframe sourcing the pixel/color vocabulary.
- mediadecode README — fix stale `version = "0.0.0"` dep example to
  `"0.2"`; rewrite the "Pixel and sample formats" bullet to describe
  the ~270-variant videoframe-sourced PixelFormat with Unknown(u32).
- mediadecode-webcodecs README — note unpublished status; fix the
  `mediadecode-webcodecs = "0.1"` example to `"0.0"` (crate is at
  0.0.0 and not yet on crates.io).

Verified locally: cargo build --workspace clean; cargo test
--workspace --lib reports 47 + 71 lib tests passing; cargo fmt
--check clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Issue #4 reported 4 findings (0 critical/high, 1 medium, 3 low):

1. MEDIUM — `plane_count` not validated against array capacity
   in `VideoFrame::new` / `AudioFrame::new`. Out-of-range values
   would panic later inside `planes()` / `samples()`.
   Already addressed in 902ec1c (pre-existing) with `assert!`
   on both constructors. Confirmed; no further code change.

2. LOW — no `Debug` on `mediadecode_ffmpeg::Frame`. Already
   addressed in 902ec1c via a manual `impl core::fmt::Debug`
   showing dimensions / pixel format. Confirmed; no further
   code change.

3. LOW — missing `#[must_use]` on consuming `with_*` builders.
   Added to all 59 `with_NAME(mut self, ...) -> Self` methods
   across the workspace (mediadecode core: frame/packet/subtitle/
   channel; mediadecode-ffmpeg: decoder/extras; mediadecode-
   webcodecs: extras). Setters that return `&mut Self` are not
   must-use candidates and were left alone.

4. LOW — webcodecs crate has no native-target tests. Added
   `mediadecode-webcodecs/tests/native_stub.rs` with two tests
   verifying the empty-stub behavior under `#[cfg(not(target_arch
   = "wasm32"))]`. Tests pass; clean compile gate on native hosts.

CHANGELOGs updated with cross-references to issue #4.

Verified: cargo build --workspace, cargo test --workspace --lib
(47 + 71 + 5 + 2 = 125 lib/native tests pass; one pre-existing
integration test in mediadecode-ffmpeg/tests/audio_pcm_fixtures.rs
fails on this worktree because the referenced fixture file isn't
checked in — unrelated to this work). cargo fmt --check + cargo
clippy --workspace --all-features -- -D warnings clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Panic-free counterparts to the existing `new` constructors,
returning `Result<Self, FrameError>` instead of asserting on
out-of-range `plane_count`. The existing `new` methods stay
`const fn` and keep their panicking behavior for statically-
known call sites (consts, tests with literal arrays); `try_new`
is for runtime-checked callers like the FFmpeg / WebCodecs
adapters that map decoder output into frames.

`try_new` itself is not `const fn` — returning `Result<Self, _>`
would require dropping the moved generic-typed `planes` /
`pixel_format` / `extra` on the error branch, which the const
evaluator can't prove safe for arbitrary `P` / `E` / `S` / `C` /
`D`. Rationale documented inline on each method.

New types:
- `mediadecode::frame::FrameError` — `non_exhaustive` enum with
  `TooManyVideoPlanes { plane_count }` and
  `TooManyAudioPlanes { plane_count }` variants. Derives
  `IsVariant` and `thiserror::Error` per the crate's existing
  convention.

New tests (4 added; total: 51 lib tests):
- `video_frame_try_new_returns_err_for_too_many_planes`
- `video_frame_try_new_accepts_valid_plane_count`
- `audio_frame_try_new_returns_err_for_too_many_planes`
- `audio_frame_try_new_accepts_valid_plane_count` (boundary at 8)

SubtitleFrame::try_new was intentionally not added — its
constructor has no validation step and would either return
`Result<Self, Infallible>` (lying about possible failure) or
require a separate stub variant. The crate's own README scopes
`try_*` to "where construction can fail", so this is consistent.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirrors the convention used in `videoframe`: struct-style variants
are split into a payload struct + a newtype-tuple enum variant.
Each payload struct carries private fields with `pub const fn`
getters and (where useful) `into_*` consumers, derives
`thiserror::Error` with the moved `#[error("...")]` message, and
the enum variant uses `#[error(transparent)]` to delegate.

Affected variants (all originally struct-style):

- `mediadecode::frame::FrameError::TooManyVideoPlanes`
- `mediadecode::frame::FrameError::TooManyAudioPlanes`
- `mediadecode_ffmpeg::Error::HwDeviceInitFailed`
- `mediadecode_ffmpeg::Error::AllBackendsFailed`
- `mediadecode_ffmpeg::Error::FallbackFailed`

Pure tuple variants (Ffmpeg, NoCodec, BackendUnsupportedByCodec)
unchanged.

Callers destructuring `Err(Error::AllBackendsFailed { attempts, .. })`
must switch to `Err(Error::AllBackendsFailed(p))` then call
`p.attempts()` / `p.unconsumed_packets()` / `p.into_parts()`.
Owning-move paths preserved via `into_parts()` /
`into_unconsumed_packets()` so non-seekable callers can still
relinquish the `Vec<Packet>` without cloning.

The manual `Debug` impl on `Error` previously printed `[N packets]`
instead of dumping per-packet bytes; this Debug logic now lives
on the payload structs (`AllBackendsFailed` and `FallbackFailed`
have hand-rolled Debug for the same reason - `ffmpeg::Packet:
!Debug`). The enum itself can derive Debug normally now.

Construction sites in `decoder.rs` / `video.rs` updated (~25).
Destructuring sites in tests / examples / benches / internal
matches updated (~18). README and rustdoc examples updated to
show the new pattern.

CHANGELOG entries reflect the BREAKING change for 0.2.0.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds `#[from]` to each newtype-tuple payload variant in
`FrameError` and `mediadecode_ffmpeg::Error`, so inner helpers
returning the payload type as their error can be `?`-propagated
into the enum directly. `Ffmpeg(#[from] ffmpeg_next::Error)`
already had this; the other five payload-wrapping variants now
match the pattern.

Affected variants:

- `mediadecode::frame::FrameError::TooManyVideoPlanes`
- `mediadecode::frame::FrameError::TooManyAudioPlanes`
- `mediadecode_ffmpeg::Error::HwDeviceInitFailed`
- `mediadecode_ffmpeg::Error::AllBackendsFailed`
- `mediadecode_ffmpeg::Error::FallbackFailed`

Auto-generated impls:

- `impl From<TooManyVideoPlanes> for FrameError`
- `impl From<TooManyAudioPlanes> for FrameError`
- `impl From<HwDeviceInitFailed> for Error`
- `impl From<AllBackendsFailed> for Error`
- `impl From<FallbackFailed> for Error`

Each payload type is unique to its variant, so no conflicting
`From` impls are produced. The simple primitive / enum variants
(`NoCodec(u32)`, `BackendUnsupportedByCodec(Backend)`) are
intentionally left without `#[from]` — `u32` would conflict
broadly and `Backend` isn't naturally a "result of a failed
operation" (it's deliberately constructed at sites that want to
reject a requested backend).

CHANGELOGs updated to mention the new `From` impls so consumers
know they exist.

Verified: cargo build --workspace clean; cargo test --workspace
--lib (51 + 71 + 2 pass); cargo clippy --workspace --all-features
--all-targets -- -D warnings clean; cargo fmt --check clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The macOS test job started failing with
`ld: warning: search path '/opt/homebrew/Cellar/ffmpeg/8.1_1/lib'
not found; ld: library 'avutil' not found`. Root cause:
`ffmpeg-sys-next`'s `build.rs` reads `pkg-config` once and bakes
the absolute brew-installed library path into its linker hints.
The cached output points at the brew FFmpeg path that was current
when the cache was first populated; when brew bumps FFmpeg to a
new patch version on the runner, that path no longer exists and
the linker fails as soon as a downstream test/example binary
gets linked.

Two fixes per affected job (build / test / coverage):

1. **Cache-key prefix bump** (`ffmpeg-{build,test,coverage}-` →
   `-v2-`). One-shot invalidation that retires the stuck caches
   carrying the stale brew path.

2. **`cargo clean -p ffmpeg-sys-next -p ffmpeg-next -p mediadecode-ffmpeg`**
   after rust install but before the build/test/tarpaulin step
   in the test + coverage jobs. Forces re-execution of the
   ffmpeg-sys-next build script on every run so the linker
   hints always match the currently-installed brew FFmpeg.
   Skipped on clippy (no link step) and build (rlib only).

The clean cost is one rebuild of the FFmpeg binding stack per
run — small relative to the full dependency build that was
already happening, and the only reliable answer short of pinning
a specific brew FFmpeg version (which has its own brittleness).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Migrates mediadecode's pixel-vocabulary primitives (PixelFormat, color enums, BayerPattern, Dimensions/Rect/Plane) to re-exports from the new videoframe 0.2 crate so that colconv, mediadecode, and scenesdetect share one canonical definition. Adapts the FFmpeg and WebCodecs adapters to the new PixelFormat::Unknown(u32) tuple variant, and reshapes Error/FrameError variants into newtype-tuple form wrapping payload structs (with accessors and into_* methods to preserve owning-move ergonomics for non-seekable callers replaying rescued packets).

Changes:

  • Replace mediadecode/src/{color,cfa,pixel_format}.rs and the structural bits of frame.rs with thin re-exports from videoframe::{color,cfa,pixel_format,frame}; bump mediadecode to 0.2.0 and mediadecode-ffmpeg to 0.2.0.
  • Update 17 boundary / test / default-frame sites in mediadecode-ffmpeg and mediadecode-webcodecs to the new PixelFormat::Unknown(u32) shape (Unknown(raw as u32) at adapter fallbacks; matches!(_, Unknown(_)) in tests).
  • Reshape Error::{HwDeviceInitFailed, AllBackendsFailed, FallbackFailed} into newtype-tuple variants over payload structs with accessor + into_parts methods; add a parallel FrameError + VideoFrame::try_new / AudioFrame::try_new panic-free constructors; add #[must_use] on every consuming with_* builder.

Reviewed changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated no comments.

Show a summary per file
File Description
Cargo.toml Adds videoframe = "0.2" workspace dep, bumps mediadecode to 0.2.
mediadecode/Cargo.toml Bumps to 0.2.0, adds videoframe dep.
mediadecode/src/{color,cfa,pixel_format}.rs Replace local definitions with videoframe::* re-exports.
mediadecode/src/frame.rs Re-export structural primitives; add FrameError + try_new constructors; add #[must_use] on builders.
mediadecode/src/{packet,channel}.rs Add #[must_use] on consuming builders.
mediadecode/{README,CHANGELOG}.md Document the videoframe migration.
mediadecode-ffmpeg/src/error.rs Reshape error variants into newtype-tuple form over payload structs; move hand-written Debug onto payloads.
mediadecode-ffmpeg/src/{decoder,video,boundary,frame,extras}.rs Adapt to new Error shape and PixelFormat::Unknown(u32); add #[must_use] on builders.
mediadecode-ffmpeg/{tests,examples,benches}/* Update destructuring to new Error::AllBackendsFailed(p) shape.
mediadecode-ffmpeg/{README,CHANGELOG}.md, Cargo.toml Document migration; bump to 0.2.0.
mediadecode-webcodecs/src/{boundary,extras}.rs Adapt Unknown fallback to Unknown(0); add #[must_use].
mediadecode-webcodecs/tests/native_stub.rs New non-wasm stub-linkage test.
mediadecode-webcodecs/{README,CHANGELOG}.md Document scaffolded status and migration.
.github/workflows/ci-ffmpeg.yml Bump cache key prefix to -v2- and add cargo clean -p ffmpeg-sys-next step to work around stale brew-FFmpeg path baked into the sys crate.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@uqio uqio merged commit 29cbb95 into main May 15, 2026
58 checks passed
@uqio uqio deleted the feat/migrate-to-videoframe branch May 15, 2026 05:16
uqio added a commit that referenced this pull request May 15, 2026
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.

2 participants