From bceda857a1c36f299e676d7b654b9ec808d58c75 Mon Sep 17 00:00:00 2001 From: Lilith River Date: Mon, 1 Jun 2026 15:16:06 -0600 Subject: [PATCH] feat(color): cross-codec color-emission policy (resolve_color_emit) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pure no_std color-carrier policy: resolve_color_emit(&SourceColor, &EncodeCapabilities, ColorPolicy) -> ColorPlan{cicp, icc} — no CMS, no codec deps. Replaces the over-built EmitFacts/EmitIntent/EmitPlan "scenario" model and the TranscodeEncoder trait, which a 5-codec dogfood + adversarial review + a full read of zenpixels/zenpipe showed (a) nothing in the pipeline produces the facts for (decode attaches no color to the buffer; the carrier is a flat Metadata), and (b) would have forced every codec to depend on every other. The grounded surface is the shipped #19 shape + the four surviving red-team fixes. - ColorPolicy { Compatibility, Balanced(default), Compact, Verbatim, Custom }; ColorPlan { cicp, icc: IccDisposition{KeepSource, SynthesizeFrom, Drop} }. - EncodeCapabilities: cicp_is_valid_carrier (standardized carrier incl. PNG cICP), cicp_safe_sole_carrier (JXL only). IccRetention +DropIfCicpRepresentable, +DropIfCicpSafeSoleCarrier. ColorFields::new makes Custom constructible. - No redundant SynthesizeFrom(sRGB). Lowers to zenpixels_convert:: finalize_for_output_with; SynthesizeFrom -> icc_profile_for_primaries const-fn table (no CMS, no silent drop; sRGB -> None). - helpers::set_exif_orientation closes the double-rotation hazard (pipeline-applied). 472 lib tests + 16 doctests pass, clippy + fmt clean. Design + rejected alternatives recorded in docs/color-emit-model.md. --- CHANGELOG.md | 23 ++ CLAUDE.md | 28 ++- docs/color-emit-model.md | 132 ++++++++++++ src/capabilities.rs | 38 ++++ src/color.rs | 445 +++++++++++++++++++++++++++++++++++++++ src/helpers/exif.rs | 135 ++++++++++++ src/helpers/mod.rs | 2 +- src/lib.rs | 6 + src/metadata.rs | 20 +- 9 files changed, 826 insertions(+), 3 deletions(-) create mode 100644 docs/color-emit-model.md create mode 100644 src/color.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index cc86b05..0f8ae24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,29 @@ All notable changes to zencodec are documented here. ## [Unreleased] +### Added + +- **Cross-codec color-emission policy** (`zencodec::color`) — + `resolve_color_emit(&SourceColor, &EncodeCapabilities, ColorPolicy) -> ColorPlan`, + a pure `no_std` decision of which color carriers (ICC vs CICP) to write for a + target, with no CMS and no codec dependencies. + - `ColorPolicy { Compatibility, Balanced (default), Compact, Verbatim, Custom(ColorFields) }`; + `ColorPlan { cicp: Option, icc: IccDisposition }`; + `IccDisposition { KeepSource, SynthesizeFrom(Cicp), Drop }`. Handles the + grayscale/CMYK terminal states and never emits a redundant `SynthesizeFrom(sRGB)`. + - `ColorFields::new` makes `ColorPolicy::Custom` constructible downstream. + - `EncodeCapabilities` gains `cicp_is_valid_carrier` (standardized carrier — + JXL/AVIF/HEIC `nclx`, PNG `cICP`) and `cicp_safe_sole_carrier` (safe CICP-only, + JXL) (+ `with_*`); `IccRetention` gains `DropIfCicpRepresentable`, + `DropIfCicpSafeSoleCarrier`. The plan lowers to `zenpixels_convert`'s + `finalize_for_output_with` (`icc_profile_for_primaries` materializes a + `SynthesizeFrom` from a `const fn` table — no CMS, never a silent drop). + - `helpers::set_exif_orientation` rewrites a blob's EXIF orientation tag inline + (offset-preserving) so a baked-upright pixel buffer and its embedded tag can't + disagree (the double-rotation hazard). Applied by the pipeline, not by the + color resolver. + - Design + rejected alternatives: `docs/color-emit-model.md`. + ## [0.1.21] - 2026-05-29 ### Added diff --git a/CLAUDE.md b/CLAUDE.md index e66d394..dc155eb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -66,4 +66,30 @@ Tiny, stable crate defining the common interface that all zen* codecs implement: ## Known Issues -(none) +Three bugs verified during the cross-codec color/metadata scenario-matrix +research (2026-06-01). The first is in this crate; the other two are recorded +here as cross-repo findings (do NOT edit those repos from here — flag to the +owner). Full design context: [`docs/color-emit-model.md`](docs/color-emit-model.md). + +1. **Double-rotation hazard (this crate, `src/metadata.rs`).** When a decoder + bakes orientation upright it sets `Metadata::orientation = Identity`, but the + EXIF blob still carries the `Orientation` tag (e.g. `6`). `Metadata::filtered` + keeps that tag, so the field says `Identity` while the blob says `Rotate90` — + they disagree, and a consumer that re-applies the EXIF tag rotates twice. The + test at `src/metadata.rs:816` currently locks in keeping the stale tag. The + byte-level fix now exists — `helpers::set_exif_orientation(blob, 1)` rewrites + the inline tag offset-preservingly. **Still TODO:** the pipeline (the layer + that bakes orientation) must actually call it on the emitted blob, and the + `metadata.rs:816` test should be updated to expect a rewritten tag, not a + stale one. This is a pipeline-applied fix, not a `Metadata::filtered` change. + +2. **AVIF descriptor-CICP override (zenavif, `src/codec.rs:824-831`).** + `apply_descriptor_color` overrides a metadata-set CICP unconditionally, + ignoring a CICP explicitly provided via `Metadata`. It should check for a + caller-supplied CICP before overriding from the pixel descriptor. + +3. **Missing signal-range conversion kernels (zenpixels-convert).** No + `Narrow <-> Full` range conversion kernels exist, so a range mismatch refuses + zero-copy but can relabel without rescaling — a black-crush risk. Needs + `ConvertStep::{Expand,Contract}NarrowToFull`. Until then, range must be + preserved verbatim, never relabeled. diff --git a/docs/color-emit-model.md b/docs/color-emit-model.md new file mode 100644 index 0000000..4a07c21 --- /dev/null +++ b/docs/color-emit-model.md @@ -0,0 +1,132 @@ +# Color emission model (grounded design) + +Status: **canonical.** This records the *minimal* shared color surface and — just +as importantly — the designs that were tried, dogfooded, adversarially reviewed, +and **rejected**, so they don't get rebuilt. Companion analysis: +[`cross-codec-color-metadata.md`](cross-codec-color-metadata.md). + +## Thesis + +The only thing that genuinely needs to be **shared** across codecs is a *pure +color-carrier policy*: given a source's color (`SourceColor`) and a target's +capabilities (`EncodeCapabilities`), decide which carriers to write (ICC vs +CICP). Everything else — pixel+metadata materialization, specialized +coefficient-domain transcodes, the decode→re-encode orchestration — already has +a home and must **not** be pulled into a grand "emit model" or a cross-codec +trait. + +This was reached the hard way: an over-built `EmitFacts`/`EmitIntent`/`EmitPlan` +"scenario" model + a `TranscodeEncoder` trait were dogfooded into 5 codecs and +adversarially reviewed; the review + a full read of zenpixels/zenpipe killed +them (see *Rejected designs*). The grounded surface is ~360 lines with **zero +codec dependencies**. + +## The shared surface — `zencodec::color` + +```rust +pub fn resolve_color_emit( + src: &SourceColor, // what the source file signalled (cicp / icc / channel_count) + target: &EncodeCapabilities, // which carriers the target format has + their quality + policy: ColorPolicy, +) -> ColorPlan; // { cicp: Option, icc: IccDisposition } + +pub enum ColorPolicy { Compatibility, Balanced /*default*/, Compact, Verbatim, Custom(ColorFields) } +pub enum IccDisposition { KeepSource, SynthesizeFrom(Cicp), Drop } +pub struct ColorFields { icc: IccRetention, cicp: CicpEmission } // ::new(icc, cicp) +pub enum CicpEmission { WhereValidCarrier /*default*/, WhereverSupported, Never } +``` + +Pure, `no_std`, **no CMS, no codec deps**. It emits a *plan*; the bytes are +materialized one layer up. `SourceColor` is the type the pipeline actually +produces (decode → `ImageInfo.source_color`; the bridge to encode is a flat +`Metadata`). The resolver also handles the grayscale/CMYK terminal states +(suppress CICP, keep ICC) and never emits a redundant `SynthesizeFrom(sRGB)`. + +### Capabilities (three flags drive it) + +- `cicp()` — has a CICP carrier slot at all. +- `cicp_is_valid_carrier()` — the carrier is standardized/honored, so CICP is + emitted by default (JXL enum, AVIF/HEIC `nclx`, **PNG `cICP`**). Distinct from + authority — PNG isn't the decode authority but is a valid carrier. +- `cicp_safe_sole_carrier()` — safe to ship CICP-only and drop the ICC (JXL only; + AVIF/HEIC/PNG keep the ICC alongside). + +## Lowering the plan (where the bytes happen) + +A codec or the pipeline lowers `ColorPlan` to bytes through **zenpixels-convert's +`finalize_for_output_with`** — which already converts pixels *and* emits matching +`OutputMetadata` atomically (pixels and embedded color cannot diverge): + +- `ColorPlan.cicp` → the format's native CICP carrier. +- `IccDisposition::KeepSource` → `OutputProfile::SameAsOrigin` (re-embed source ICC). +- `IccDisposition::SynthesizeFrom(cicp)` → `zenpixels_convert::icc_profile_for_primaries` + (a `const fn` table of bundled profiles — **no CMS, no allocation**; returns + `None` for BT.709/sRGB so the assumed default is never embedded). +- `IccDisposition::Drop` → no ICC. + +So "synthesize an ICC" can never silently lose color and never needs a CMS in the +codec — it's a table lookup. + +## Orientation (separate, tiny) + +The double-rotation hazard (a decoder bakes orientation upright but the embedded +EXIF blob still says `Rotate90`) is closed by +`helpers::set_exif_orientation(blob, value)` — an offset-preserving inline rewrite +of the 0x0112 tag. It's applied by the **pipeline**, which knows when it baked +orientation. It is *not* part of color policy and not a "unified plan". + +## Transcodes (pairwise, self-contained — not shared) + +Specialized lossless/coefficient transcodes are **not** a generic capability: + +- **JPEG → JPEG** (orient / recompress): entirely inside zenjpeg + (`zenjpeg::lossless`, `zenjpeg::recompress`). +- **JPEG → JXL** (lossless embed): inside jxl-encoder via jbrd (its own + `JpegData` parser — the JXL spec's recompression feature, **needs no zenjpeg**). + +These preserve metadata verbatim, so they don't even call the color resolver. +The set of real pairs is tiny and well-known. The **dispatch** belongs in +**zenpipe**, which already depends on every codec — a small finite table of known +pairs calling those functions directly, plus `resolve_color_emit` on the +decode→re-encode path. No codec ever learns about another. + +zenpipe already has the sketch: `try_lossless_jpeg` (in `lossless.rs`, currently +only called from tests) is the precedent to wire and generalize. **That's a later +piece**, tracked separately. + +## Rejected designs (do not rebuild) + +- **`EmitFacts { Fresh | Decoded | Passthrough }` + `PixelFidelity`** — nothing in + the pipeline produces a `ColorOrigin`/fidelity: decode attaches no color to the + buffer; provenance lives in `ImageInfo.source_color` and the carrier is a flat + `Metadata`. A codec `Encoder` only ever sees `with_metadata(Metadata)`, so it + could only ever build `Fresh` — the scenario machinery was dead code. The + `PixelDescriptor` already *is* the current gamut, so deriving `Reauthored` was + redundant. `resolve_color_emit(&SourceColor, …)` takes the type that flows. +- **`TranscodeEncoder` trait in zencodec** — a generic "output codec transcodes + from source-format X" trait forces every output codec to *ingest* every input + format (JXL←JPEG needs JPEG parsing; PNG←? needs zenpng; …) → **every codec + depends on every other codec**. The real pairs are ~2 and each self-contained. + zenpipe (deps-all) dispatches; no trait. +- **`EmitIntent` unifying color + metadata + orientation into one knob** — + aesthetic, not grounded. `MetadataPolicy` (#17) and `ColorPolicy` are fine + apart; orientation is a one-helper correctness fix, not a policy axis. +- **A resolver that produces final `Metadata` bytes** — a third metadata producer + alongside `Metadata::filtered` and `OutputMetadata`. Atomicity is already + `finalize_for_output_with`'s job. + +## What landed (the surviving red-team fixes) + +The 5-codec dogfood + adversarial review found real defects; the ones that +survived the grounding, all small and all on the `resolve_color_emit` shape: + +1. `ColorFields::new` / `CicpEmission` are constructible → `ColorPolicy::Custom` + is actually reachable downstream. +2. `cicp_is_valid_carrier` tier → PNG/WebP emit cICP under Balanced instead of + laundering wide-gamut color through a synthesized ICC. +3. No redundant `SynthesizeFrom(sRGB)` (the canned table returns `None` for sRGB). +4. `set_exif_orientation` for the double-rotation hazard. + +The `SynthesizeFrom`-silently-drops-color critical dissolves under lowering — +`icc_profile_for_primaries` always materializes a non-sRGB profile, never a CMS, +never a silent drop. diff --git a/src/capabilities.rs b/src/capabilities.rs index cc8cef4..6af9f3d 100644 --- a/src/capabilities.rs +++ b/src/capabilities.rs @@ -102,6 +102,9 @@ pub struct EncodeCapabilities { exif: bool, xmp: bool, cicp: bool, + // CICP carrier quality (distinct from `cicp` = "has a CICP carrier slot") + cicp_is_valid_carrier: bool, + cicp_safe_sole_carrier: bool, // Operation support stop: bool, animation: bool, @@ -143,6 +146,8 @@ impl EncodeCapabilities { exif: false, xmp: false, cicp: false, + cicp_is_valid_carrier: false, + cicp_safe_sole_carrier: false, stop: false, animation: false, push_rows: false, @@ -181,6 +186,27 @@ impl EncodeCapabilities { pub const fn cicp(&self) -> bool { self.cicp } + /// Whether this format has a standardized, real-world-honored CICP carrier — + /// so CICP can be emitted as a color signal by default. + /// + /// True for JXL codestream enum, AVIF/HEIC `nclx`, and PNG `cICP`. Distinct + /// from [`cicp`](Self::cicp) (= "has a CICP carrier slot at all") and from + /// [`cicp_safe_sole_carrier`](Self::cicp_safe_sole_carrier) (= "safe to ship + /// CICP *only* and drop the ICC"). Gates CICP emission under + /// [`CicpEmission::WhereValidCarrier`](crate::color::CicpEmission::WhereValidCarrier). + pub const fn cicp_is_valid_carrier(&self) -> bool { + self.cicp_is_valid_carrier + } + /// Whether it is safe in practice to ship CICP as the *sole* color carrier + /// and drop a redundant ICC profile for this format. + /// + /// Stricter than [`cicp_is_valid_carrier`](Self::cicp_is_valid_carrier): a + /// format can have a valid CICP carrier yet still need an ICC kept for + /// real-world tool compatibility. As of 2026 this is true only for JXL + /// (matches libjxl's `want_icc=false` default); AVIF/HEIC/PNG keep the ICC. + pub const fn cicp_safe_sole_carrier(&self) -> bool { + self.cicp_safe_sole_carrier + } /// Whether `with_stop` on encode jobs is respected (not a no-op). pub const fn stop(&self) -> bool { self.stop @@ -316,6 +342,18 @@ impl EncodeCapabilities { self.cicp = v; self } + /// Set whether this format has a standardized CICP carrier. + /// See [`cicp_is_valid_carrier`](Self::cicp_is_valid_carrier). + pub const fn with_cicp_is_valid_carrier(mut self, v: bool) -> Self { + self.cicp_is_valid_carrier = v; + self + } + /// Set whether CICP is safe as the sole color carrier (drop redundant ICC). + /// See [`cicp_safe_sole_carrier`](Self::cicp_safe_sole_carrier). + pub const fn with_cicp_safe_sole_carrier(mut self, v: bool) -> Self { + self.cicp_safe_sole_carrier = v; + self + } /// Set whether cooperative cancellation via [`Stop`](enough::Stop) is supported. pub const fn with_stop(mut self, v: bool) -> Self { self.stop = v; diff --git a/src/color.rs b/src/color.rs new file mode 100644 index 0000000..8afa1e3 --- /dev/null +++ b/src/color.rs @@ -0,0 +1,445 @@ +//! Color-signaling production policy: how an image's color *description* +//! (ICC profile vs CICP code points) is emitted when encoding or transcoding. +//! +//! This is orthogonal to which *pixels* are written. Containers differ in which +//! color carriers they have and in how reliably real-world decoders honor each +//! one, so emitting "the right" color description is a per-target decision. +//! +//! # The obvious knob: [`ColorPolicy`] +//! +//! Pick an intent — the same meaning whether encoding from pixels or transcoding +//! from another file: +//! +//! - [`Compatibility`](ColorPolicy::Compatibility) — always embed an ICC; add CICP where reliable. +//! - [`Balanced`](ColorPolicy::Balanced) (**default**) — emit CICP where the format has a +//! standardized CICP carrier, drop a redundant ICC only where CICP is safe as the sole carrier +//! (JXL today) or the ICC is plain sRGB. +//! - [`Compact`](ColorPolicy::Compact) — smallest: prefer CICP wherever the format carries it, drop the ICC. +//! - [`Verbatim`](ColorPolicy::Verbatim) — carry the source's signals unchanged. +//! - [`Custom`](ColorPolicy::Custom) — explicit [`ColorFields`] for power users. +//! +//! # The resolver: [`resolve_color_emit`] +//! +//! [`resolve_color_emit`] reconciles a [`SourceColor`] against a target's +//! [`EncodeCapabilities`] under a [`ColorPolicy`] and returns a [`ColorPlan`] — +//! a pure description of what to emit. This crate is `no_std` and carries no +//! CMS, so the plan only describes intent ([`IccDisposition::SynthesizeFrom`], +//! etc.); the bytes are materialized one layer up. +//! +//! # Lowering the plan +//! +//! A codec (or the pipeline) lowers a [`ColorPlan`] to the bytes it writes — for +//! the pixel-encode path, through `zenpixels_convert`'s atomic +//! `finalize_for_output_with` (which guarantees pixels and embedded color cannot +//! diverge): +//! +//! - [`ColorPlan::cicp`] → the format's native CICP carrier (JXL enum color, +//! AVIF/HEIC `nclx`, PNG `cICP`). +//! - [`IccDisposition::KeepSource`] → re-embed the source ICC bytes +//! (`OutputProfile::SameAsOrigin`). +//! - [`IccDisposition::SynthesizeFrom`]`(cicp)` → fetch a bundled profile via +//! `zenpixels_convert::icc_profile_for_primaries` (a `const fn` table — **no CMS**; +//! it returns `None` for BT.709/sRGB, so the assumed default is never embedded). +//! - [`IccDisposition::Drop`] → emit no ICC. +//! +//! Orientation/EXIF reconciliation is separate: when a pipeline bakes orientation +//! upright it rewrites the source EXIF orientation tag with +//! [`helpers::set_exif_orientation`](crate::helpers::set_exif_orientation) so the +//! tag and the pixels can't disagree (the double-rotation hazard). + +use zenpixels::icc; +use zenpixels::{Cicp, ColorModel}; + +use crate::capabilities::EncodeCapabilities; +use crate::info::SourceColor; +use crate::metadata::IccRetention; + +/// How color description is emitted on encode — the obvious, intent-named knob. +/// +/// See the [module docs](self) for the per-format behavior table. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +#[non_exhaustive] +pub enum ColorPolicy { + /// Widest compatibility: always embed an ICC profile (synthesizing one from + /// CICP when the source had none); add CICP where the format treats it as + /// authority. Largest color overhead. + Compatibility, + /// **Default.** Emit CICP where it is the format's authority and drop a + /// redundant ICC only where CICP is safe as the *sole* carrier + /// ([`cicp_safe_sole_carrier`](EncodeCapabilities::cicp_safe_sole_carrier) — + /// JXL today) or the ICC is a plain sRGB profile. Otherwise keep the ICC. + #[default] + Balanced, + /// Smallest color overhead: prefer CICP wherever the format can carry it at + /// all, and drop the ICC whenever CICP can describe the color. + Compact, + /// Carry the source's color signals through unchanged — derive and strip + /// nothing. For transcodes that must preserve exactly what was there. + Verbatim, + /// Explicit mechanism control. + Custom(ColorFields), +} + +/// Whether CICP is emitted, behind [`ColorPolicy::Custom`]. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +#[non_exhaustive] +pub enum CicpEmission { + /// Emit CICP where the format has a standardized, real-world-honored CICP + /// carrier ([`cicp_is_valid_carrier`](EncodeCapabilities::cicp_is_valid_carrier)): + /// JXL/AVIF/HEIC `nclx`, and PNG `cICP`. The default. Distinct from + /// "drop the ICC" — a valid carrier (PNG) still keeps the ICC alongside. + #[default] + WhereValidCarrier, + /// Emit CICP wherever the format has *any* carrier slot, even a non-standard + /// or emergent one. + WhereverSupported, + /// Never emit CICP (ICC-only output). + Never, +} + +/// Mechanism fields behind [`ColorPolicy::Custom`]. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[non_exhaustive] +pub struct ColorFields { + /// When to drop the ICC profile. + pub icc: IccRetention, + /// Whether to emit CICP. + pub cicp: CicpEmission, +} + +impl Default for ColorFields { + fn default() -> Self { + Self { + icc: IccRetention::DropIfCicpSafeSoleCarrier, + cicp: CicpEmission::WhereValidCarrier, + } + } +} + +impl ColorFields { + /// Construct explicit color-emission fields for [`ColorPolicy::Custom`]. + /// + /// `ColorFields` is `#[non_exhaustive]`, so downstream crates cannot build it + /// with a struct literal — use this constructor (or [`Default`]) so + /// [`ColorPolicy::Custom`] is actually reachable. + pub const fn new(icc: IccRetention, cicp: CicpEmission) -> Self { + Self { icc, cicp } + } +} + +impl ColorPolicy { + /// Resolve a preset to its mechanism fields. + pub const fn fields(&self) -> ColorFields { + match self { + Self::Compatibility => ColorFields { + icc: IccRetention::Keep, + cicp: CicpEmission::WhereValidCarrier, + }, + Self::Balanced => ColorFields { + icc: IccRetention::DropIfCicpSafeSoleCarrier, + cicp: CicpEmission::WhereValidCarrier, + }, + Self::Compact => ColorFields { + icc: IccRetention::DropIfCicpRepresentable, + cicp: CicpEmission::WhereverSupported, + }, + Self::Verbatim => ColorFields { + icc: IccRetention::Keep, + cicp: CicpEmission::WhereValidCarrier, + }, + Self::Custom(f) => *f, + } + } +} + +/// What to do with the ICC profile channel for one encode. +/// +/// The bytes are materialized by the codec adapter / CMS layer, not here. +#[derive(Clone, Debug, PartialEq, Eq)] +#[non_exhaustive] +pub enum IccDisposition { + /// Embed the source ICC bytes verbatim. + KeepSource, + /// Embed an ICC synthesized from this CICP (target has no CICP carrier, or + /// the policy wants an ICC alongside). The caller materializes the bytes. + SynthesizeFrom(Cicp), + /// Emit no ICC profile. + Drop, +} + +/// A resolved plan for emitting an image's color description on encode. +/// +/// Produced by [`resolve_color_emit`]. Deliberately minimal: it carries the +/// ICC/CICP decision, which is what current transcode needs. `#[non_exhaustive]` +/// so range/rendering-intent/HDR/gain-map dispositions and a warnings channel +/// can be added back additively when a consumer needs them. +#[derive(Clone, Debug, PartialEq)] +#[non_exhaustive] +pub struct ColorPlan { + /// CICP to write to the target's native carrier, if any. + pub cicp: Option, + /// Disposition of the ICC profile channel. + pub icc: IccDisposition, +} + +/// The CICP that describes this source's color as code points, if any: +/// the explicit CICP, else derived from the ICC (`cicp` tag, then corpus). +fn representable_cicp(src: &SourceColor) -> Option { + if let Some(c) = src.cicp { + return Some(c); + } + let icc_bytes = src.icc_profile.as_ref()?; + icc::extract_cicp(icc_bytes) + .or_else(|| icc::identify_common(icc_bytes).and_then(|id| id.to_cicp())) +} + +/// Reconcile a source's color description against a target's capabilities under +/// a [`ColorPolicy`], returning what to emit. +/// +/// Pure and `no_std`. Decides ICC vs CICP emission, including the grayscale / +/// CMYK terminal states (where CICP is inapplicable and the ICC must be kept). +pub fn resolve_color_emit( + src: &SourceColor, + target: &EncodeCapabilities, + policy: ColorPolicy, +) -> ColorPlan { + let fields = policy.fields(); + let src_has_icc = src.icc_profile.is_some(); + + // Grayscale / CMYK: CICP is RGB-centric and cannot describe these. Keep the + // ICC (the only valid color description) and suppress CICP — emitting an RGB + // CICP over gray/CMYK pixels would recolor them. + let model = src + .icc_profile + .as_deref() + .and_then(icc::profile_color_space); + let is_gray = matches!(model, Some(ColorModel::Gray)) || src.channel_count == Some(1); + let is_cmyk = matches!(model, Some(ColorModel::Cmyk)); + if is_gray || is_cmyk { + return ColorPlan { + cicp: None, + icc: if src_has_icc { + IccDisposition::KeepSource + } else { + IccDisposition::Drop + }, + }; + } + + let repr_cicp = representable_cicp(src); + let cicp_represents = repr_cicp.is_some(); + let has_carrier = target.cicp(); + let is_valid_carrier = target.cicp_is_valid_carrier(); + let sole_safe = target.cicp_safe_sole_carrier(); + let icc_is_srgb = src.icc_profile.as_deref().is_some_and(icc::is_common_srgb); + + // Whether to emit CICP. + let emit_cicp = match policy { + ColorPolicy::Verbatim => has_carrier && src.cicp.is_some(), + _ => match fields.cicp { + CicpEmission::Never => false, + CicpEmission::WhereValidCarrier => has_carrier && is_valid_carrier && cicp_represents, + CicpEmission::WhereverSupported => has_carrier && cicp_represents, + }, + }; + let cicp_out = if emit_cicp { + if policy == ColorPolicy::Verbatim { + src.cicp + } else { + repr_cicp + } + } else { + None + }; + + // Whether to drop the ICC. + let drop_by_rule = match fields.icc { + IccRetention::Drop => true, + IccRetention::Keep => false, + IccRetention::KeepNonSrgb => icc_is_srgb, + IccRetention::DropIfCicpRepresentable => emit_cicp && cicp_represents, + IccRetention::DropIfCicpSafeSoleCarrier => emit_cicp && sole_safe && cicp_represents, + }; + // Balanced additionally sheds a redundant sRGB ICC even where CICP isn't the + // sole carrier (the most common pure-weight case). + let drop_icc = match policy { + ColorPolicy::Balanced => drop_by_rule || (emit_cicp && icc_is_srgb), + _ => drop_by_rule, + }; + + // sRGB is the universally-assumed default: the canned-profile table has no + // sRGB ICC to synthesize (`zenpixels_convert::icc_profile_for_primaries` + // returns `None` for BT.709), so a `SynthesizeFrom(sRGB)` directive would + // lower to nothing. Don't emit it — drop instead. + let synth_worthwhile = cicp_represents && repr_cicp != Some(Cicp::SRGB); + + let icc = if src_has_icc { + if drop_icc { + IccDisposition::Drop + } else { + IccDisposition::KeepSource + } + } else if !emit_cicp && synth_worthwhile && policy != ColorPolicy::Verbatim { + // No source ICC and CICP isn't carrying the color (target is ICC-only): + // synthesize an ICC so the (non-default) color isn't lost. + IccDisposition::SynthesizeFrom(repr_cicp.expect("synth_worthwhile")) + } else if matches!(policy, ColorPolicy::Compatibility) && synth_worthwhile { + // Compatibility wants an ICC present alongside CICP (non-sRGB only). + IccDisposition::SynthesizeFrom(repr_cicp.expect("synth_worthwhile")) + } else { + IccDisposition::Drop + }; + + ColorPlan { + cicp: cicp_out, + icc, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use zenpixels::ColorAuthority; + + // Capability fixtures matching the 2026 reliability findings. + fn caps_jxl() -> EncodeCapabilities { + EncodeCapabilities::new() + .with_icc(true) + .with_cicp(true) + .with_cicp_is_valid_carrier(true) + .with_cicp_safe_sole_carrier(true) + } + fn caps_avif() -> EncodeCapabilities { + EncodeCapabilities::new() + .with_icc(true) + .with_cicp(true) + .with_cicp_is_valid_carrier(true) + .with_cicp_safe_sole_carrier(false) + } + fn caps_jpeg() -> EncodeCapabilities { + // No CICP carrier at all. + EncodeCapabilities::new().with_icc(true) + } + fn caps_png() -> EncodeCapabilities { + // PNG cICP: a standardized-but-emergent carrier — valid, not sole-safe. + EncodeCapabilities::new() + .with_icc(true) + .with_cicp(true) + .with_cicp_is_valid_carrier(true) + .with_cicp_safe_sole_carrier(false) + } + + fn src_cicp(c: Cicp) -> SourceColor { + SourceColor::default() + .with_cicp(c) + .with_color_authority(ColorAuthority::Cicp) + .with_channel_count(3) + } + + #[test] + fn jxl_balanced_strips_representable_icc() { + // JXL (sole-safe): CICP present + an ICC whose color CICP represents → + // emit CICP, drop the ICC (matches libjxl's want_icc=false default). + let src = SourceColor::default() + .with_cicp(Cicp::SRGB) + .with_icc_profile(alloc::vec![0u8; 132]) + .with_channel_count(3); + let plan = resolve_color_emit(&src, &caps_jxl(), ColorPolicy::Balanced); + assert_eq!(plan.cicp, Some(Cicp::SRGB)); + assert_eq!(plan.icc, IccDisposition::Drop); + } + + #[test] + fn avif_balanced_keeps_nonsrgb_icc_alongside_cicp() { + // AVIF (not sole-safe): a non-sRGB ICC is kept alongside CICP. (The + // redundant-sRGB drop needs a corpus-recognized profile and is covered + // by the conformance suite, which has a real sRGB profile via `cms`.) + let p3 = src_cicp(Cicp::DISPLAY_P3).with_icc_profile(alloc::vec![0u8; 132]); + let plan = resolve_color_emit(&p3, &caps_avif(), ColorPolicy::Balanced); + assert_eq!(plan.cicp, Some(Cicp::DISPLAY_P3)); + assert_eq!(plan.icc, IccDisposition::KeepSource); + } + + #[test] + fn jpeg_synthesizes_icc_from_cicp() { + // CICP-only source → JPEG (no CICP carrier): synthesize an ICC. + let src = src_cicp(Cicp::DISPLAY_P3); + let plan = resolve_color_emit(&src, &caps_jpeg(), ColorPolicy::Balanced); + assert_eq!(plan.cicp, None); + assert_eq!(plan.icc, IccDisposition::SynthesizeFrom(Cicp::DISPLAY_P3)); + } + + #[test] + fn compact_strips_icc_on_avif() { + // Compact drops the ICC wherever CICP represents the color, even on AVIF. + let p3 = src_cicp(Cicp::DISPLAY_P3).with_icc_profile(alloc::vec![0u8; 132]); + let plan = resolve_color_emit(&p3, &caps_avif(), ColorPolicy::Compact); + assert_eq!(plan.cicp, Some(Cicp::DISPLAY_P3)); + assert_eq!(plan.icc, IccDisposition::Drop); + } + + #[test] + fn compatibility_always_keeps_or_synthesizes_icc() { + // CICP-only source, AVIF, Compatibility → CICP emitted AND an ICC synthesized. + let src = src_cicp(Cicp::DISPLAY_P3); + let plan = resolve_color_emit(&src, &caps_avif(), ColorPolicy::Compatibility); + assert_eq!(plan.cicp, Some(Cicp::DISPLAY_P3)); + assert_eq!(plan.icc, IccDisposition::SynthesizeFrom(Cicp::DISPLAY_P3)); + } + + #[test] + fn grayscale_keeps_icc_suppresses_cicp() { + // A 1-channel source: CICP is inapplicable; keep ICC, suppress CICP. + let src = SourceColor::default() + .with_icc_profile(alloc::vec![0u8; 132]) + .with_channel_count(1); + let plan = resolve_color_emit(&src, &caps_avif(), ColorPolicy::Balanced); + assert_eq!(plan.cicp, None); + assert_eq!(plan.icc, IccDisposition::KeepSource); + } + + #[test] + fn verbatim_passes_source_through() { + // Verbatim keeps both, derives nothing. + let src = src_cicp(Cicp::DISPLAY_P3).with_icc_profile(alloc::vec![0u8; 132]); + let plan = resolve_color_emit(&src, &caps_avif(), ColorPolicy::Verbatim); + assert_eq!(plan.cicp, Some(Cicp::DISPLAY_P3)); + assert_eq!(plan.icc, IccDisposition::KeepSource); + } + + #[test] + fn default_policy_is_balanced() { + assert_eq!(ColorPolicy::default(), ColorPolicy::Balanced); + } + + #[test] + fn png_emits_cicp_keeps_icc_under_balanced() { + // PNG: standardized cICP carrier but not sole-safe → emit cICP AND keep + // iCCP. Regression for the missing valid-carrier tier — a non-authority + // carrier must still emit CICP under Balanced. + let p3 = src_cicp(Cicp::DISPLAY_P3).with_icc_profile(alloc::vec![0u8; 132]); + let plan = resolve_color_emit(&p3, &caps_png(), ColorPolicy::Balanced); + assert_eq!(plan.cicp, Some(Cicp::DISPLAY_P3)); + assert_eq!(plan.icc, IccDisposition::KeepSource); + } + + #[test] + fn srgb_only_source_does_not_synthesize_redundant_icc() { + // CICP-only sRGB → JPEG (no carrier): sRGB is the assumed default and the + // canned table has no sRGB profile → drop, never SynthesizeFrom(sRGB). + let src = src_cicp(Cicp::SRGB); + let plan = resolve_color_emit(&src, &caps_jpeg(), ColorPolicy::Balanced); + assert_eq!(plan.cicp, None); + assert_eq!(plan.icc, IccDisposition::Drop); + } + + #[test] + fn custom_policy_is_constructible() { + // ColorFields::new makes ColorPolicy::Custom reachable from downstream. + let policy = ColorPolicy::Custom(ColorFields::new(IccRetention::Keep, CicpEmission::Never)); + let p3 = src_cicp(Cicp::DISPLAY_P3).with_icc_profile(alloc::vec![0u8; 132]); + let plan = resolve_color_emit(&p3, &caps_avif(), policy); + assert_eq!(plan.cicp, None); // CicpEmission::Never + assert_eq!(plan.icc, IccDisposition::KeepSource); // IccRetention::Keep + } +} diff --git a/src/helpers/exif.rs b/src/helpers/exif.rs index c4de684..953e083 100644 --- a/src/helpers/exif.rs +++ b/src/helpers/exif.rs @@ -32,6 +32,95 @@ pub fn parse_exif_orientation(data: &[u8]) -> Option { crate::exif::Exif::parse(data)?.orientation() } +/// Rewrite the EXIF Orientation tag (0x0112) in a TIFF/EXIF blob to `value`, +/// returning a new blob. +/// +/// The orientation value is stored inline (SHORT or LONG), so this overwrites it +/// in place without recomputing any TIFF offsets. Accepts raw TIFF bytes or a +/// JPEG APP1 `Exif\0\0`-prefixed blob, both byte orders, fully bounds-checked. +/// +/// Returns `None` if the blob is malformed or carries no Orientation tag — the +/// caller should then leave the blob unchanged. This is the byte-level half of +/// closing the double-rotation hazard: when a decoder bakes orientation upright, +/// the structured field says `Identity` but the embedded blob still says e.g. +/// `Rotate90`; rewriting the tag to `1` keeps them in agreement. +pub fn set_exif_orientation(data: &[u8], value: Orientation) -> Option> { + // Optional "Exif\0\0" prefix; TIFF offsets are relative to the TIFF header. + let tiff_start = if data.len() >= 6 && &data[0..4] == b"Exif" && data[4] == 0 && data[5] == 0 { + 6 + } else { + 0 + }; + let tiff = data.get(tiff_start..)?; + if tiff.len() < 8 { + return None; + } + let be = match &tiff[0..2] { + b"II" => false, + b"MM" => true, + _ => return None, + }; + let r16 = |o: usize| -> Option { + let s = tiff.get(o..o + 2)?; + Some(if be { + u16::from_be_bytes([s[0], s[1]]) + } else { + u16::from_le_bytes([s[0], s[1]]) + }) + }; + let r32 = |o: usize| -> Option { + let s = tiff.get(o..o + 4)?; + Some(if be { + u32::from_be_bytes([s[0], s[1], s[2], s[3]]) + } else { + u32::from_le_bytes([s[0], s[1], s[2], s[3]]) + }) + }; + if r16(2)? != 42 { + return None; + } + let ifd = r32(4)? as usize; + let count = r16(ifd)? as usize; + if count > 4096 { + return None; // DoS cap on IFD entry count + } + let v = value as u8; + for i in 0..count { + let entry = ifd + 2 + i * 12; + if r16(entry)? != 0x0112 { + continue; + } + let type_id = r16(entry + 2)?; + // Value field is the last 4 bytes of the 12-byte entry, absolute in `data`. + let off = tiff_start + entry + 8; + let mut out = data.to_vec(); + match type_id { + 3 => { + let b = if be { + (v as u16).to_be_bytes() + } else { + (v as u16).to_le_bytes() + }; + *out.get_mut(off)? = b[0]; + *out.get_mut(off + 1)? = b[1]; + } + 4 => { + let b = if be { + (v as u32).to_be_bytes() + } else { + (v as u32).to_le_bytes() + }; + for (k, byte) in b.iter().enumerate() { + *out.get_mut(off + k)? = *byte; + } + } + _ => return None, + } + return Some(out); + } + None +} + #[cfg(test)] mod tests { use super::*; @@ -116,4 +205,50 @@ mod tests { assert_eq!(parse_exif_orientation(&tiff(false, 9, 3)), None); assert_eq!(parse_exif_orientation(&tiff(false, 0, 3)), None); } + + #[test] + fn set_orientation_roundtrips_all_orders_and_types() { + for be in [false, true] { + for &type_id in &[3u16, 4] { + // Start at Rotate90 (6), rewrite to Identity (1), read back. + let blob = tiff(be, 6, type_id); + assert_eq!(parse_exif_orientation(&blob), Some(Orientation::Rotate90)); + let rewritten = + set_exif_orientation(&blob, Orientation::Identity).expect("tag present"); + assert_eq!(rewritten.len(), blob.len()); // offsets unchanged + assert_eq!( + parse_exif_orientation(&rewritten), + Some(Orientation::Identity) + ); + } + } + } + + #[test] + fn set_orientation_with_exif_prefix() { + let mut blob = b"Exif\0\0".to_vec(); + blob.extend_from_slice(&tiff(false, 6, 3)); + let out = set_exif_orientation(&blob, Orientation::Rotate180).expect("tag present"); + assert_eq!(parse_exif_orientation(&out), Some(Orientation::Rotate180)); + } + + #[test] + fn set_orientation_absent_tag_or_garbage_is_none() { + // No 0x0112 entry: a minimal IFD with a different tag. + let mut v = b"II".to_vec(); + v.extend_from_slice(&42u16.to_le_bytes()); + v.extend_from_slice(&8u32.to_le_bytes()); + v.extend_from_slice(&1u16.to_le_bytes()); // 1 entry + v.extend_from_slice(&0x010Fu16.to_le_bytes()); // Make tag, not orientation + v.extend_from_slice(&3u16.to_le_bytes()); + v.extend_from_slice(&1u32.to_le_bytes()); + v.extend_from_slice(&[0, 0, 0, 0]); + v.extend_from_slice(&0u32.to_le_bytes()); + assert_eq!(set_exif_orientation(&v, Orientation::Identity), None); + assert_eq!( + set_exif_orientation(b"garbage", Orientation::Identity), + None + ); + assert_eq!(set_exif_orientation(&[], Orientation::Identity), None); + } } diff --git a/src/helpers/mod.rs b/src/helpers/mod.rs index ca7605b..8b3056a 100644 --- a/src/helpers/mod.rs +++ b/src/helpers/mod.rs @@ -19,7 +19,7 @@ use crate::traits::{AnimationFrameDecoder, Decode, DecodeJob}; mod exif; mod icc; -pub use exif::parse_exif_orientation; +pub use exif::{parse_exif_orientation, set_exif_orientation}; pub use icc::descriptor_for_decoded_pixels_v2; #[allow(deprecated)] pub use icc::{ diff --git a/src/lib.rs b/src/lib.rs index 83b97f3..6fcf3ce 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -38,6 +38,9 @@ extern crate alloc; whereat::define_at_crate_info!(); mod capabilities; +/// Cross-codec color-signaling emission policy (ICC vs CICP). See +/// `docs/color-emit-model.md`. +pub mod color; mod cost; mod detect; mod error; @@ -65,6 +68,9 @@ mod traits; // Public root: shared types used by both encode and decode // ========================================================================= +pub use color::{ + CicpEmission, ColorFields, ColorPlan, ColorPolicy, IccDisposition, resolve_color_emit, +}; pub use exif::{ByteOrder, Exif, ExifPolicy, Retention}; pub use extensions::Extensions; pub use format::{ImageFormat, ImageFormatDefinition, ImageFormatRegistry}; diff --git a/src/metadata.rs b/src/metadata.rs index 1d5786f..4d0306a 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -229,7 +229,12 @@ impl Metadata { // ICC — three-way; only KeepNonSrgb drops a redundant sRGB profile. out.icc_profile = match f.icc { IccRetention::Drop => None, - IccRetention::Keep => self.icc_profile.clone(), + // Target-blind retention keeps the profile; the CICP-conditional + // drop is resolved against a concrete target in + // `color::resolve_color_emit`, which `filtered` does not see. + IccRetention::Keep + | IccRetention::DropIfCicpRepresentable + | IccRetention::DropIfCicpSafeSoleCarrier => self.icc_profile.clone(), IccRetention::KeepNonSrgb => self .icc_profile .as_ref() @@ -285,6 +290,19 @@ pub enum IccRetention { KeepNonSrgb, /// Keep the profile as-is, even a redundant sRGB one (byte-faithful). Keep, + /// Drop the profile when it maps to a CICP expressible as code points + /// (sRGB / Display-P3 / BT.2020 / BT.2100…) — i.e. CICP fully describes the + /// color. **Target-aware**: only takes effect in + /// [`color::resolve_color_emit`](crate::color::resolve_color_emit), where the + /// target's CICP carrier is known. In the target-blind [`Metadata::filtered`] + /// path it conservatively keeps the profile. + DropIfCicpRepresentable, + /// Drop the profile only when the target format's CICP is safe as the sole + /// color carrier ([`EncodeCapabilities::cicp_safe_sole_carrier`](crate::encode::EncodeCapabilities::cicp_safe_sole_carrier) + /// — JXL today) and CICP represents the color. Like + /// [`DropIfCicpRepresentable`](Self::DropIfCicpRepresentable), this is + /// target-aware and keeps the profile in [`Metadata::filtered`]. + DropIfCicpSafeSoleCarrier, } /// Per-field metadata retention for [`MetadataPolicy::Custom`].