Skip to content

Conversation

@hjanuschka
Copy link
Collaborator

@hjanuschka hjanuschka commented Dec 23, 2025

Summary

This PR addresses the feedback from PR #586 about being able to:

  • Request output in different data types (u8/u16/f16) for benchmarking
  • Add tests to catch output format conversion bugs in the future

Changes

  1. CLI --data-type option (jxl_cli)

    • New flag: --data-type u8|u16|f16|f32 (default: f32)
    • Exercises the decoder's type conversion pipeline for benchmarking
    • Example: jxl_cli image.jxl -s --data-type u8
  2. Output format consistency tests (jxl/src/api/decoder.rs)

    • test_output_format_u8_matches_f32 - Verifies u8 output matches f32
    • test_output_format_u16_matches_f32 - Verifies u16 output matches f32
    • test_output_format_f16_matches_f32 - Verifies f16 output matches f32
    • Tests both RGB and BGRA formats to catch channel reordering bugs
    • Tests both simple and low-memory pipelines
    • Verified these tests would have caught the PR Fix rendering bugs introduced in PR #577 #586 bug by temporarily reverting the fix and confirming test failure

@hjanuschka hjanuschka closed this Dec 23, 2025
@hjanuschka hjanuschka force-pushed the cli-data-type-and-tests branch from 4a0c5c9 to 3d2d2f8 Compare December 23, 2025 00:10
@github-actions
Copy link

github-actions bot commented Dec 23, 2025

Benchmark @ 4a0a330

MULTI-FILE BENCHMARK RESULTS (4 files)
  CPU architecture: x86_64
  WARNING: System appears noisy: high system load (2.15). Results may be unreliable.
Statistics:
  Confidence:               99.0%
  Max relative error:        3.0%

Comparing: 4ac4460b (Base) vs 1e53d214 (PR)

File Base (MP/s) PR (MP/s) Δ%
bike.jxl 22.850 23.291 +1.93% ±2.0%
green_queen_modular_e3.jxl 8.096 8.056 -0.49% ±1.2%
green_queen_vardct_e3.jxl 19.954 19.873 -0.40% ±1.7%
sunset_logo.jxl 2.210 2.203 -0.32% ±0.7%

@hjanuschka hjanuschka reopened this Dec 23, 2025
@hjanuschka hjanuschka force-pushed the cli-data-type-and-tests branch 2 times, most recently from 6bf820e to d692e0a Compare December 23, 2025 08:38
Add CLI option to request specific output data types (u8, u16, f16, f32)
for benchmarking the decoder's conversion pipeline.

Add integration tests comparing u8/u16/f16 output against f32 reference
to catch conversion bugs like those fixed in PR libjxl#586. Tests verify both
RGB and BGRA formats across simple and low-memory pipelines.

Changes:
- jxl_cli: Add --data-type flag (default: f32)
- jxl_cli/src/dec: Add decode_frames_with_type() for typed output
- jxl/src/api/decoder.rs: Add test_output_format_{u8,u16,f16}_matches_f32
- jxl/Cargo.toml: Add half as dev-dependency for f16 tests
Returns TypedDecodeOutput that preserves the original output type, with
conversion to f32 happening in the caller (outside of timing). This
ensures benchmark timing accurately reflects just the decode time,
not post-decode conversion overhead.

- New enum TypedDecodeOutput that can hold u8/u16/f16/f32 output
- decode_frames_with_type returns TypedDecodeOutput + Duration
- Caller calls .to_f32() when needed for saving to encoders
- Added integration tests for output format consistency
- Add premultiply parameter to decode_with_format<T>
- Remove duplicate decode_image functions from premultiply tests
- All tests now use the shared decode_with_format helper

Removes ~140 lines of duplicated code.
@veluca93 veluca93 force-pushed the cli-data-type-and-tests branch from fe7cf60 to 4a0a330 Compare January 5, 2026 18:33
@veluca93 veluca93 enabled auto-merge (squash) January 5, 2026 18:33
@veluca93 veluca93 merged commit 969813a into libjxl:main Jan 5, 2026
17 checks passed
zond pushed a commit that referenced this pull request Jan 13, 2026
* Add --data-type CLI option and output format consistency tests

Add CLI option to request specific output data types (u8, u16, f16, f32)
for benchmarking the decoder's conversion pipeline.

Add integration tests comparing u8/u16/f16 output against f32 reference
to catch conversion bugs like those fixed in PR #586. Tests verify both
RGB and BGRA formats across simple and low-memory pipelines.

Changes:
- jxl_cli: Add --data-type flag (default: f32)
- jxl_cli/src/dec: Add decode_frames_with_type() for typed output
- jxl/src/api/decoder.rs: Add test_output_format_{u8,u16,f16}_matches_f32
- jxl/Cargo.toml: Add half as dev-dependency for f16 tests

* Add --data-type CLI option and output format tests

Returns TypedDecodeOutput that preserves the original output type, with
conversion to f32 happening in the caller (outside of timing). This
ensures benchmark timing accurately reflects just the decode time,
not post-decode conversion overhead.

- New enum TypedDecodeOutput that can hold u8/u16/f16/f32 output
- decode_frames_with_type returns TypedDecodeOutput + Duration
- Caller calls .to_f32() when needed for saving to encoders
- Added integration tests for output format consistency

* Consolidate duplicate decode_image helpers into decode_with_format

- Add premultiply parameter to decode_with_format<T>
- Remove duplicate decode_image functions from premultiply tests
- All tests now use the shared decode_with_format helper

Removes ~140 lines of duplicated code.
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