Skip to content

feat(bin): add ICC color profile handling for still images#31

Open
Shathe wants to merge 2 commits into
rust-av:mainfrom
Shathe:feat/icc-color-profile-handling
Open

feat(bin): add ICC color profile handling for still images#31
Shathe wants to merge 2 commits into
rust-av:mainfrom
Shathe:feat/icc-color-profile-handling

Conversation

@Shathe

@Shathe Shathe commented May 28, 2026

Copy link
Copy Markdown

Summary

  • Read embedded ICC profiles from PNG (iCCP chunk) and JPEG (APP2 markers) and convert pixels to sRGB via lcms2 before computing the metric. This matches the C++ libjxl ssimulacra2 binary's behavior on tagged inputs (Display P3, Adobe RGB, etc).
  • Add --no-icc flag to disable conversion (reproduces previous behavior).
  • Handle alpha channels via dual-background compositing (0.1 and 0.9), taking the worse score. Matches the C++ reference.
  • Fall back to image crate (assuming sRGB) for formats where ICC extraction isn't supported (WebP, HDR, EXR, etc).

New dependencies

  • lcms2 = "6.1" — ICC color management (bundles liblcms2 source, no system dep needed)
  • png = "0.17" — PNG decoding with ICC profile access
  • jpeg-decoder = "0.3" — JPEG decoding with ICC profile access

Test plan

  • cargo clippy -- -D warnings clean
  • 7 integration tests covering: untagged regression, sRGB-tagged matches untagged, Display P3 differs from sRGB, Adobe RGB differs from sRGB, --no-icc bypasses conversion, reference score pinning for Display P3 and Adobe RGB
  • Test fixtures committed (64x64 PNGs with embedded ICC profiles)

@Shathe

Shathe commented Jun 1, 2026

Copy link
Copy Markdown
Author

Heads up on the two red checks: I dug into both and they're pre-existing on main, not caused by this PR.

msrv job runs on Rust 1.65.0, but the binary that got merged in eb58170 already requires newer than that. ssimulacra2_bin declares rust-version = "1.74" and depends on image 0.25.6 (whose own MSRV is 1.70), so the bin can't build on 1.65 regardless of the lock file. On top of that the tracked Cargo.lock.MSRV predates the binary merge, so cargo check --locked fails with "lock file needs to be updated" before it even compiles. This PR's lcms2 addition just changes the error text, the job was already red. I confirmed by running the exact CI step on eb58170 and it fails the same way.

build job dies on yeslogic-fontconfig-sys because the runner has no system fontconfig/pkg-config. That comes in transitively through plotters/criterion (the bench deps), nothing to do with this change, and it's failing on main too.

Happy to push a fix for either in this PR if you'd like (bump the msrv job to match the bin's 1.74, regenerate the MSRV lock, add a libfontconfig1-dev install step or feature-gate the plot deps). Just let me know if you'd rather own the CI side yourself.

@shssoichiro

Copy link
Copy Markdown
Member

Sorry for the delay on reviewing this. Looks like I finally got the CI job passing after cleaning up a year of decay.

@shssoichiro

Copy link
Copy Markdown
Member

I'm guessing this needs a rebase. I swear Github used to have a button so I could do it, but I don't seem to have it available.

Shathe added 2 commits June 21, 2026 10:48
Read embedded ICC profiles from PNG (iCCP chunk) and JPEG (APP2 markers)
and convert to sRGB via lcms2 before computing the metric, matching the
C++ libjxl ssimulacra2 binary behavior. Adds --no-icc flag to bypass
conversion. Also handles alpha via dual-background compositing (0.1/0.9).
@FreezyLemon FreezyLemon force-pushed the feat/icc-color-profile-handling branch from b832ecd to c631ef3 Compare June 21, 2026 08:49
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