Skip to content

gamut-jxl: a present ICC in with_metadata silently replaces the caller's ColorSpec #626

Description

@justin13888

What happens today

JxlEncoder::with_encoded_metadata (which with_metadata delegates to) routes a present ICC
profile through the encoder's colour dial:

// crates/gamut-jxl/src/encoder.rs
if let Some(icc) = &encoded.icc {
    self = self.with_color(ColorSpec::Icc(icc.clone()));
}

and with_color is a plain assignment (self.color = color;). So

let e = JxlEncoder::new()
    .with_color(ColorSpec::Srgb)     // the caller's explicit choice
    .with_metadata(&meta)?;          // meta.icc = Some(profile)
// e.color is now ColorSpec::Icc(profile); the Srgb choice is gone, silently.

In every other format wired to the facade the ICC carrier is a container payload (a JPEG APP2
run, a PNG iCCP chunk, a WebP ICCP chunk), so routing it to the raw setter overwrites only a
previous profile. In JPEG XL the profile is the codestream's colour encoding, so the same
routing overwrites a semantic choice the caller made through a different API.

The fork

Three defensible rules, and the crate has never chosen between them in writing:

  1. Last write wins (today's behaviour) — with_metadata is a setter like any other.
  2. Refuse the conflict — return Error::InvalidInput when self.color is not the default and
    the model carries a profile, so the caller orders the two calls deliberately.
  3. Metadata never touches the colour dial — the ICC field of an EncodedMetadata is ignored by
    JPEG XL, and a caller who wants the profile encoded says with_color(ColorSpec::Icc(..)).

Rule 2 is the only one that cannot lose information, but it makes an ordering of two independent
builder calls significant, which is its own surprise. Rule 3 makes the facade's round-trip lossy
for the one format where the profile matters most.

Whichever is chosen, the same question exists for any later format whose colour encoding and ICC
carrier are the same thing.

Interim

The behaviour is documented as it stands — in JxlEncoder::with_metadata's rustdoc and in
gamut-metadata's README ("Precedence, as it behaves today") — so a caller is not surprised by it.
Nothing is being changed until this is decided.

Evidence

Refs #420.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions