feat(jxl): expose modular-mode control on the encoder - #347
Merged
Conversation
`JxlEncoderFrameSettingId` declared only `EFFORT`, so `EFFORT` was the sole per-frame option any caller could reach. Add `MODULAR` (value 11), transcribed from the pinned libjxl v0.12.0 `jxl/encode.h`, so `gamut-jxl` can select the VarDCT/Modular coding tool. `JxlEncoderFrameSettingsSetOption` already takes an `i64`, so no new extern declaration is needed and the `tests/version.rs` symbol table (which covers extern fns only) is unchanged. Refs #339
Only `EFFORT` ever reached libjxl, so the public encoder offered no coding-tool control at all. Add `ModularMode` (`Auto`/`VarDct`/`Modular`, mapping libjxl's -1/0/1) and `JxlEncoder::with_modular`. `Auto` is the default and leaves the frame setting **unsent**, so a default encoder's bytes are byte-identical to before. Lossless + forced `VarDct` is rejected with a typed `InvalidInput` rather than silently ignored: libjxl's `QueueFrame` calls `cparams.SetLossless()`, which sets `modular_mode = true` unconditionally, so the request could not be honoured. The check runs in `dispatch_encode`, ahead of both the backend registry and the built-in tail, so every target and every backend sees one refusal. Forcing `Modular` on lossless is allowed — it agrees with libjxl. The knob is codestream-level, so it reaches pushed backends through `JxlEncodeRequest`. `EncodeConfig` has no field for it, so the codec-abi adapter declines a pinned mode (as it already does for colour and orientation) instead of dropping it — unlike effort, the coding tool reshapes the stream. `recompress_jpeg` re-packs the JPEG's own coefficients and has no coding-tool choice to make, so it ignores the setting, as it already does the other inapplicable knobs. Refs #339
Exposes `JxlEncoder::with_modular` alongside `--jxl-effort` / `--jxl-distance` / `--jxl-container`, so the coding tool can be pinned from the binary — the parity the image-evaluation sweep needs to compare gamut-jxl against libjxl-encode's `modular-on` variant on equal terms. `auto` is the default; `vardct` on the lossless default surfaces the codec's typed refusal through `CliError::Codec`. Refs #339
Adds the implemented-surface bullet (including the lossless+VarDCT refusal, the codec-abi decline and the jbrd no-op), the coding-tool test-regime bullet, and `with_modular` in the README encode example. Narrows the deferred "progressive encode control" entry into a single frame-settings entry: the modular *tuning* knobs and the remaining coding-tool toggles are now additive behind the seam this opened, rather than an FFI change each time. Refs #339
Collaborator
Author
|
Local gate results (the two the PR body left pending):
Full local set, all green: |
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
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.
Summary
gamut-jxlreached libjxl with exactly oneJxlEncoderFrameSettingId—EFFORT— so the publicencoder offered effort, distance/lossless, container, colour spec, orientation, bit depth and
metadata, and no coding-tool control at all.
This adds modular-mode control:
gamut-jxl-sysdeclaresJxlEncoderFrameSettingId::MODULAR(value 11), transcribed from thepinned libjxl v0.12.0
jxl/encode.h.JxlEncoderFrameSettingsSetOptionalready takes ani64,so no new extern declaration and no
tests/version.rssymbol-table change.gamut-jxlgainsModularMode(Auto/VarDct/Modular, mapping libjxl's-1/0/1)and
JxlEncoder::with_modular, shaped like the existingEffort/Orientationknobs.gamut-cligains--jxl-modular auto|vardct|modularongamut convert.The motivation is parity with the
modular-onvariant thatimage-evaluation already sweeps, so
gamut-jxlcan be compared against
libjxl-encodeon equal terms — not rate-distortion (forcing modulargenerally loses above distance ~0.5, and libjxl already picks modular for lossless). Opening this
seam also makes the rest of the frame-setting surface additive rather than an FFI change each time.
Three decisions worth reviewing
Autois inert, not "auto = -1". When the mode isAutothe option is not sent at all, so adefault encoder's bytes are byte-identical to before this PR.
auto_modular_is_byte_identical_to_the_untouched_defaultpins that.
Lossless + forced
VarDctis a typed refusal. libjxl'sQueueFramecallscparams.SetLossless()(
lib/jxl/encode.cc:471-473), which setsmodular_mode = trueunconditionally(
lib/jxl/enc_params.h:144-150) — so the frame setting is overridden and the request cannot behonoured. Rather than silently emit a Modular stream, this returns
InvalidInput("JXL: lossless encoding cannot force VarDCT mode"). The check lives indispatch_encode, ahead of both the backend registry and the built-in tail, so every target andevery backend sees one refusal. Forcing
Modularon lossless is allowed — it agrees with libjxl.The codec-abi adapter declines a pinned mode rather than dropping it.
EncodeConfigcarries onlya codec id and a
0..=100quality. Effort is deliberately not conveyed (a free speed/density choicewith no effect on decoded pixels), but the coding tool reshapes the codestream, so
is_conveyablenow declines it exactly as it already declines non-default colour and orientation. The knob does
reach typed Rust backends, through a new
JxlEncodeRequest::modular()(its constructor ispub(crate), so this is additive).recompress_jpeg(jbrd) re-packs the JPEG's own DCT coefficients and has no coding-tool choice tomake, so it ignores the setting — as it already does the other inapplicable knobs. Documented rather
than rejected, matching that path's existing behaviour.
Validation
mise run test— pass (2505 passed, 0 failed)mise run lint— pass (clippy,-D warnings,--all-targets --all-features)mise run fmt-check— passmise run check-commits— pass (no errors in 4 commits)mise run coverage/mise run mutants— running locally; see the comment below for results.gamut convert in.png auto.jxl --jxl-distance 1.0→ 2228 bytesgamut convert in.png mod.jxl --jxl-distance 1.0 --jxl-modular modular→ 1608 bytes, bytes differgamut convert in.png bad.jxl --jxl-modular vardct→error: invalid input: JXL: lossless encoding cannot force VarDCT mode [origin: gamut-jxl]gamut convert mod.jxl back.png→ decodes cleanly through jxl-rsNew tests:
ModularModevalue round-trip / range / default;with_modularchaining, equalityparticipation and the lossless+VarDCT refusal;
modular_setting_changes_the_stream(forced Modulardiffers from forced VarDCT and from libjxl's own choice, all three decodable by both jxl-rs and the
libjxl oracle);
auto_modular_is_byte_identical_to_the_untouched_default;forced_modular_lossy_stays_within_psnr; and, on the seam, that a pushed backend seesreq.modular()while a codec-abi backend declines a pinned mode and falls through to the built-in tail.
Since no decoder in the workspace reports whether a stream is VarDCT or Modular (
JxlInfocarries nosuch flag, and the oracle helper returns pixels only), the plumbing is pinned on stream bytes
differing plus decodability — the same shape as the existing
effort_setting_changes_the_stream.Risks and rollout
Additive and semver-minor.
Autois the default and provably byte-identical to the previous output,so no existing stream changes.
JxlEncodeRequestgained a field, but its constructor ispub(crate)— external
JxlCodestreamEncoderimplementors are unaffected.Out of scope
The sibling modular tuning enumerants (
MODULAR_COLOR_SPACE25,MODULAR_GROUP_SIZE26,MODULAR_PREDICTOR27,MODULAR_NB_PREV_CHANNELS29) and the float-valuedMODULAR_MA_TREE_LEARNING_PERCENT(28, which would additionally needJxlEncoderFrameSettingsSetFloatOptiondeclared plus atests/version.rsrow). The issue asked forthe minimal enumerant addition; these are now additive behind the seam this opens, and
STATUS.mdrecords them.
Pre-existing issues noticed, deliberately not fixed here
Spotted while reading the crate; unrelated to this change, so left for a separate PR:
crates/gamut-jxl/src/lib.rs:98— the last doc line ends mid-sentence with an unterminatedbacktick:
//! The crate is `#![deny(unsafe_code)]. The crate has no#![deny(unsafe_code)]/#![forbid(unsafe_code)]attribute at all (unlike every other workspacecrate), which makes
ffi.rs:9's#![allow(unsafe_code)]a no-op.crates/gamut-jxl/README.md:129— also ends mid-sentence ("On the decode side,").Issue
Closes #339