Skip to content

RafDecoder, RawDecoder: don't use an unset blackLevelSeparate - #1001

Open
alepacheco wants to merge 2 commits into
darktable-org:developfrom
alepacheco:fix-black-level-optional
Open

alepacheco wants to merge 2 commits into
darktable-org:developfrom
alepacheco:fix-black-level-optional

Conversation

@alepacheco

Copy link
Copy Markdown

RawImageData::blackLevelSeparate is an Optional that stays empty until something assigns it. Two places use it without assigning it first:

  • RafDecoder::decodeMetaDataInternal(): the FUJI_BLACKLEVEL block fills it only when the tag has 4 or 36 entries, then averages it unconditionally. A RAF from a camera listed in cameras.xml whose tag has any other count reaches Optional::operator-> with no value: an assertion failure in a debug build, undefined behaviour in a release build. It now throws, as ArwDecoder and NefDecoder do for their black level tags.
  • RawDecoder::setMetaData(): the final_cfa_black hint writes all four values through it, but it is only set if the decoder or the camera's sensor entry already set it. No camera in cameras.xml uses the hint today; it now sets the 2x2 view first, as the sensor branch above does.

Testing: ctest passes. rstest -c on raw.pixls.us samples (X100, X30, X-A5, X-Pro1, X-T3, GFX 50S, HS50EXR, S5Pro) gives identical hashes before and after.

Only an entry count of 4 or 36 sets blackLevelSeparate, but the
averaging after it dereferences blackLevelSeparate unconditionally,
so any other count dereferenced an empty Optional. Throw instead,
like ArwDecoder and NefDecoder do for their black level tags.
The final_cfa_black hint writes all four values through
blackLevelSeparate, which is unset unless the decoder or the
camera's sensor entry already set it, so the hint could
dereference an empty Optional. No camera in data/cameras.xml
uses the hint today.
@alepacheco
alepacheco requested a review from LebedevRI as a code owner October 29, 2026 09:50
@LebedevRI

Copy link
Copy Markdown
Member

Reproducer?

@alepacheco

Copy link
Copy Markdown
Author

I don't have a camera file that hits the first one; the condition is in the source (lines at c48d5a6):

RafDecoder::decodeMetaDataInternal(): blackLevelSeparate is assigned only in the count == 4 and count == 36 branches (L312-L333), but L337 dereferences it whenever a FUJI_BLACKLEVEL entry exists (L309). The count comes from the file and nothing checks it before this, so a RAF of a camera listed in cameras.xml (past getCamera() at L298) with any other count reaches Optional::operator-> empty: the has_value() assertion in a debug build, UB in a release build (where invariant() is an assumption).

RawDecoder::setMetaData(): this one reproduces with the tree's own data. PefDecoder calls setMetaData() (L129) before it sets blackLevelSeparate, and no camera has black_colors. Add

<Hints>
	<Hint name="final_cfa_black" value="10,20,30,20"/>
</Hints>

to the PENTAX K-5 entry in data/cameras.xml (after BlackAreas) and run rstest -c with a Debug build on a K-5 PEF from raw.pixls.us. On develop:

rstest: src/librawspeed/adt/Optional.h:61: T *rawspeed::Optional<rawspeed::Array2DRef<int>>::operator->() [T = rawspeed::Array2DRef<int>]: Assertion `has_value()' failed.
Aborted

With this PR it decodes and writes the hash.

This branch has not been deployed

No deployments
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