From 8b0648d1b7feaa5794ca78e350478c60ef30df9d Mon Sep 17 00:00:00 2001 From: Abas-Tim <8209950+Abas-Tim@users.noreply.github.com> Date: Sun, 6 Sep 2026 13:53:49 -0700 Subject: [PATCH 1/3] fix: run the metadata pass in rawspeed-cli 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, /../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. --- src/utilities/rawspeed-cli/main.cpp | 50 ++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/src/utilities/rawspeed-cli/main.cpp b/src/utilities/rawspeed-cli/main.cpp index 5f269debb..7193411c7 100644 --- a/src/utilities/rawspeed-cli/main.cpp +++ b/src/utilities/rawspeed-cli/main.cpp @@ -8,8 +8,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -23,6 +25,27 @@ using rawspeed::CFAColor; namespace { +std::string find_cameras_xml(const char* argv0) { +#ifdef RS_CAMERAS_XML_PATH + if (std::filesystem::exists(RS_CAMERAS_XML_PATH)) + return RS_CAMERAS_XML_PATH; +#endif + const std::string self(argv0); + const std::size_t lastslash = self.find_last_of(R"(/\)"); + const std::string bindir = lastslash == std::string::npos + ? std::string(".") + : self.substr(0, lastslash); + std::string camfile = bindir + "/../share/darktable/rawspeed/cameras.xml"; + if (std::filesystem::exists(camfile)) + return camfile; +#ifdef RAWSPEED_STANDALONE_BUILD + camfile = std::string(RAWSPEED_SOURCE_DIR "/data/cameras.xml"); + if (std::filesystem::exists(camfile)) + return camfile; +#endif + return {}; +} + struct DemosaicJob { rawspeed::Array2DRef mosaic; rawspeed::Array1DRef rgb; @@ -121,7 +144,11 @@ bool render(const rawspeed::RawImage& raw, std::vector& rgbOut, } const auto cfaSize = raw->cfa.getSize(); - if (cfaSize.x != 2 || cfaSize.y != 2) { + if (cfaSize.x < 2 || cfaSize.y < 2) { + raw->cfa.setCFA(rawspeed::iPoint2D(2, 2), CFAColor::RED, CFAColor::GREEN, + CFAColor::GREEN, CFAColor::BLUE); + } + if (raw->cfa.getSize().x != 2 || raw->cfa.getSize().y != 2) { std::fprintf(stderr, "rawspeed: unsupported CFA pattern\n"); return false; } @@ -230,6 +257,27 @@ int main(int argc_, char** argv_) { rawspeed::RawImage raw = decoder->decodeRaw(); + auto meta = std::make_unique(); +#ifdef HAVE_PUGIXML + const std::string camfile = find_cameras_xml(argv(0)); + if (!camfile.empty()) { + try { + meta = std::make_unique(camfile.c_str()); + } catch (const std::exception& e) { + std::fprintf(stderr, + "rawspeed: cameras.xml unusable ('%s'), continuing " + "without it\n", + e.what()); + } + } +#endif + try { + decoder->decodeMetaData(meta.get()); + } catch (const std::exception& e) { + std::fprintf(stderr, "rawspeed: metadata pass failed, continuing: %s\n", + e.what()); + } + std::vector rgb; uint32_t w = 0; uint32_t h = 0; From 89ed65885e7fa2176913bfb52d2621b732aaa4dd Mon Sep 17 00:00:00 2001 From: Abas-Tim <8209950+Abas-Tim@users.noreply.github.com> Date: Sun, 6 Sep 2026 13:53:56 -0700 Subject: [PATCH 2/3] ci: decode a real image in the rawspeed-cli smoke test 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. --- .github/workflows/rawspeed-cli.yml | 70 ++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/.github/workflows/rawspeed-cli.yml b/.github/workflows/rawspeed-cli.yml index 8bad2c3c4..93dfcd8c5 100644 --- a/.github/workflows/rawspeed-cli.yml +++ b/.github/workflows/rawspeed-cli.yml @@ -54,6 +54,7 @@ jobs: if ($null -eq $exe) { throw "rawspeed-cli.exe not found under build/" } + $output = (& $exe.FullName 2>&1 | Out-String) $exitCode = $LASTEXITCODE "exe: $($exe.FullName)" | Add-Content $env:GITHUB_STEP_SUMMARY @@ -63,6 +64,75 @@ jobs: throw "rawspeed-cli smoke test failed: exe=$($exe.FullName) exit=$exitCode output=[$output]" } Write-Output "smoke test ok: $($exe.FullName)" + + $w = 8 + $h = 8 + $pixels = [System.IO.MemoryStream]::new() + for ($y = 0; $y -lt $h; $y++) { + for ($x = 0; $x -lt $w; $x++) { + $b = [BitConverter]::GetBytes([uint16](($y * $w + $x) * 500 + 1000)) + $pixels.Write($b, 0, 2) + } + } + $pixels = $pixels.ToArray() + $make = [System.Text.Encoding]::ASCII.GetBytes("rawspeed`0") + $model = [System.Text.Encoding]::ASCII.GetBytes("test`0") + $ucm = [System.Text.Encoding]::ASCII.GetBytes("rawspeed test`0") + $entryCount = 16 + $extOffset = 8 + (2 + $entryCount * 12 + 4) + $makeOff = $extOffset + $modelOff = $makeOff + $make.Length + $ucmOff = $modelOff + $model.Length + $pixelOff = $ucmOff + $ucm.Length + $ms = [System.IO.MemoryStream]::new() + $bw = [System.IO.BinaryWriter]::new($ms) + $bw.Write([byte[]]@(0x49, 0x49, 0x2A, 0x00)) + $bw.Write([uint32]8) + $bw.Write([uint16]$entryCount) + foreach ($e in @( + @(254, 4, 1, 0), @(256, 4, 1, $w), @(257, 4, 1, $h), + @(258, 3, 1, 16), @(259, 3, 1, 1), @(262, 3, 1, 32803), + @(271, 2, $make.Length, $makeOff), @(272, 2, $model.Length, $modelOff), + @(273, 4, 1, $pixelOff), @(277, 3, 1, 1), @(278, 4, 1, $h), + @(279, 4, 1, $pixels.Length))) { + $bw.Write([uint16]$e[0]); $bw.Write([uint16]$e[1]) + $bw.Write([uint32]$e[2]); $bw.Write([uint32]$e[3]) + } + $bw.Write([uint16]33421); $bw.Write([uint16]3); $bw.Write([uint32]2) + $bw.Write([byte[]]@(2, 0, 2, 0)) + $bw.Write([uint16]33422); $bw.Write([uint16]1); $bw.Write([uint32]4) + $bw.Write([byte[]]@(0, 1, 1, 2)) + $bw.Write([uint16]50706); $bw.Write([uint16]1); $bw.Write([uint32]4) + $bw.Write([byte[]]@(1, 4, 0, 0)) + $bw.Write([uint16]50708); $bw.Write([uint16]2) + $bw.Write([uint32]$ucm.Length); $bw.Write([uint32]$ucmOff) + $bw.Write([uint32]0) + $bw.Write($make) + $bw.Write($model) + $bw.Write($ucm) + $bw.Write($pixels) + $bw.Flush() + $dng = Join-Path $env:RUNNER_TEMP "tiny.dng" + [System.IO.File]::WriteAllBytes($dng, $ms.ToArray()) + + $ppm = Join-Path $env:RUNNER_TEMP "tiny.ppm" + & $exe.FullName $dng $ppm 2>&1 | Out-String | Write-Output + $decodeExit = $LASTEXITCODE + if ($decodeExit -ne 0) { + throw "rawspeed-cli decode smoke test failed: exit=$decodeExit" + } + $ppmBytes = [System.IO.File]::ReadAllBytes($ppm) + $expected = "P6`n$w $h`n65535`n" + $expectedBytes = [System.Text.Encoding]::ASCII.GetBytes($expected) + if ($ppmBytes.Length -ne ($expectedBytes.Length + $w * $h * 6)) { + throw "unexpected PPM size: $($ppmBytes.Length)" + } + for ($i = 0; $i -lt $expectedBytes.Length; $i++) { + if ($ppmBytes[$i] -ne $expectedBytes[$i]) { + throw "unexpected PPM header" + } + } + Write-Output "decode smoke test ok: $dng -> $ppm ($($ppmBytes.Length) bytes)" exit 0 - name: Stage and checksum From d9656a8b275dd73609d31b97af2ad3bf135336f6 Mon Sep 17 00:00:00 2001 From: Abas-Tim <8209950+Abas-Tim@users.noreply.github.com> Date: Sun, 6 Sep 2026 13:53:56 -0700 Subject: [PATCH 3/3] docs: add rawspeed-cli v1.0.1 regression report 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. --- docs/rawspeed-cli-v1.0.1-regression-report.md | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 docs/rawspeed-cli-v1.0.1-regression-report.md diff --git a/docs/rawspeed-cli-v1.0.1-regression-report.md b/docs/rawspeed-cli-v1.0.1-regression-report.md new file mode 100644 index 000000000..e12b41c89 --- /dev/null +++ b/docs/rawspeed-cli-v1.0.1-regression-report.md @@ -0,0 +1,68 @@ +rawspeed-cli v1.0.1 regression report +===================================== + +Symptom +------- + +rawspeed-cli v1.0.1 fails on ARW/CR2/PEF inputs with:: + + rawspeed: unsupported CFA pattern + exit 1 + +v1.0.0 decoded the same files. + +Root cause +---------- + +``main.cpp`` only calls ``decoder->decodeRaw()`` and never +``decoder->decodeMetaData()``. For ARW/CR2/PEF the CFA pattern is set +in the metadata pass, not in the raw pass: + +- ``ArwDecoder::decodeMetaDataInternal`` (ArwDecoder.cpp:501) calls + ``mRaw->cfa.setCFA(2x2, R, G, G, B)`` before consulting the camera + database; the same holds for ``Cr2Decoder`` and ``PefDecoder``. +- ``render()`` then reads ``raw->cfa`` with its default size (0, 0) + and fails at the ``cfaSize != 2x2`` check. + +The old v1.0.0 CLI only worked because it unconditionally called +``raw->cfa.setCFA(2x2, R,G,G,B)`` in ``render()``. That call looked +like dead code and was removed during the clang-tidy cleanup - it was +load-bearing. + +Fix (v1.0.2) +------------ + +1. After ``decodeRaw()``, call ``decoder->decodeMetaData(&meta)``. + ``cameras.xml`` is resolved like ``rawspeed-identify`` does + (``RS_CAMERAS_XML_PATH``, ``/../share/darktable/rawspeed/ + cameras.xml``, then ``RAWSPEED_SOURCE_DIR/data/cameras.xml`` for + standalone builds). If it cannot be found or parsed, an empty + ``CameraMetaData{}`` is used: ``ArwDecoder`` (and Cr2/Pef) set the + RGGB CFA before consulting the database, and with + ``failOnUnknown = false`` an unknown camera only logs a warning and + returns early, so decoding works without ``cameras.xml``. A failing + metadata pass is logged and skipped rather than aborting the + decode. +2. ``render()`` is hardened: if the reported CFA size is smaller than + 2x2, it defaults to RGGB instead of failing. Larger CFA patterns + (e.g. X-Trans) still fail as unsupported. The existing 4-Bayer + pattern mapping is kept - it is correct once the CFA is real. +3. The publish workflow's smoke test now decodes a real image (a + synthetic 8x8 uncompressed CFA DNG generated in the runner) and + verifies the PPM output, instead of only checking the usage path + which cannot catch this class of regression. + +Verification +------------ + +- synthetic 8x8 CFA DNG decodes: exit 0, ``P6 8 8 65535``, 397 bytes; + 8-bit variant: 203 bytes +- usage path still exits 2 with usage text +- clang-format 18.1.8 idempotent, clang -Weverything -Werror clean, + clang-tidy (CI check set) clean, MSVC Release build green + +AI note +------- + +This report and the fix were prepared with AI assistance (opencode), +per AGENTS.md disclosure rules.