Fix rawspeed-cli ARW/CR2/PEF decoding (missing metadata pass) - #3
Merged
Merged
Conversation
ARW/CR2/PEF decoders set the CFA pattern in their metadata pass
(ArwDecoder.cpp:501 and the Cr2/Pef equivalents), not in decodeRaw.
Without calling decoder->decodeMetaData() the CFA stays at its
default 0x0 size and render() fails with 'unsupported CFA pattern'.
- resolve cameras.xml like rawspeed-identify does (RS_CAMERAS_XML_PATH,
<bindir>/../share/darktable/rawspeed/cameras.xml, then
RAWSPEED_SOURCE_DIR/data/cameras.xml) and fall back to an empty
CameraMetaData{}: the decoders set RGGB before consulting the
database and unknown cameras only warn with failOnUnknown=false
- skip a failing metadata pass with a warning instead of aborting
- harden render(): default to RGGB when the reported CFA is smaller
than 2x2; larger patterns still fail as unsupported
Found via real-world ARW/CR2/PEF failures of the v1.0.1 release;
the v1.0.0 CLI only worked because it unconditionally forced an RGGB
CFA in render(), a load-bearing call that was removed as apparent
dead code.
AI assistance disclosure: prepared with AI assistance (opencode), per
AGENTS.md disclosure rules.
The usage-only smoke test cannot catch decode-path regressions such as the v1.0.1 metadata regression (every decode failed while the usage check passed). Generate a synthetic 8x8 uncompressed RGGB CFA DNG in the runner, decode it with the freshly built rawspeed-cli and verify the PPM header and size. The usage check is kept as well. AI assistance disclosure: prepared with AI assistance (opencode), per AGENTS.md disclosure rules.
Documents the missing decodeMetaData() call that broke ARW/CR2/PEF decoding in v1.0.1, why the v1.0.0 forced-CFA call was load-bearing, and the fix shipped in v1.0.2. AI assistance disclosure: prepared with AI assistance (opencode), per AGENTS.md disclosure rules.
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
Fixes the v1.0.1 regression: ARW/CR2/PEF decoding failed with
rawspeed: unsupported CFA pattern(exit 1).Root cause:
main.cppnever calleddecoder->decodeMetaData();those decoders set the CFA in their metadata pass
(
ArwDecoder.cpp:501, same for Cr2/Pef), sorender()saw a 0x0 CFAand failed. The v1.0.0 CLI only worked because it unconditionally
forced an RGGB CFA in
render()— a load-bearing call that theclang-tidy cleanup removed as apparent dead code. Full details in
docs/rawspeed-cli-v1.0.1-regression-report.md.Changes:
decoder->decodeMetaData(&meta)afterdecodeRaw();cameras.xmlis resolved likerawspeed-identifydoes(
RS_CAMERAS_XML_PATH→<bindir>/../share/darktable/rawspeed/cameras.xml→
RAWSPEED_SOURCE_DIR/data/cameras.xml), falling back to an emptyCameraMetaData{}(the decoders set RGGB before consulting the DB,and unknown cameras only warn with
failOnUnknown = false)the decode
render()hardening: CFA smaller than 2x2 now defaults to RGGBinstead of failing; larger patterns (e.g. X-Trans) still fail as
unsupported; the 4-Bayer pattern mapping is unchanged
uncompressed RGGB CFA DNG generated in the runner, verifying the
PPM header and size (the usage-only check cannot catch this class
of regression)
docs/Verification
P6 8 8 65535, 397 bytes (8-bit: 203)-Weverything -Werrorclean;clang-tidy (CI check set) clean; MSVC Release build green; exact CI
smoke-test script executed locally against the fresh build
Notes
rawspeed-cli-v1.0.1; v1.0.2 will betagged after merge