Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved calculation of DNG white balance from AsShotWhiteXY #515

Merged
merged 1 commit into from
Aug 24, 2023

Conversation

kmilos
Copy link
Contributor

@kmilos kmilos commented Aug 21, 2023

Attempt to address darktable-org/darktable#14805 @jenshannoschwalm @mcogoni

Not sure if an additional chromatic adaptation step is needed on the as shot XYZ values (relative to D65) before using the D65 matrix on them...

@kmilos kmilos requested a review from LebedevRI as a code owner August 21, 2023 10:25
mRaw->metadata.wbCoeffs[2] =
1 - mRaw->metadata.wbCoeffs[0] - mRaw->metadata.wbCoeffs[1];

const std::array<float, 3> d65_white = {{0.950456, 1, 1.088754}};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yeah, hardcoding D65 here clearly looks suspicious.

@codecov
Copy link

codecov bot commented Aug 22, 2023

Codecov Report

Patch coverage: 6.66% and project coverage change: -0.03% ⚠️

Comparison is base (060cb05) 59.12% compared to head (92ce5be) 59.10%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #515      +/-   ##
===========================================
- Coverage    59.12%   59.10%   -0.03%     
===========================================
  Files          232      232              
  Lines        13892    13898       +6     
  Branches      1937     1938       +1     
===========================================
  Hits          8214     8214              
- Misses        5545     5551       +6     
  Partials       133      133              
Flag Coverage Δ
benchmarks 8.18% <0.00%> (-0.01%) ⬇️
integration 47.51% <7.14%> (-0.02%) ⬇️
linux 57.02% <7.14%> (-0.03%) ⬇️
macOS 18.68% <0.00%> (-0.01%) ⬇️
rpu_u 47.51% <7.14%> (-0.02%) ⬇️
unittests 18.22% <0.00%> (-0.01%) ⬇️
windows ∅ <ø> (∅)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
src/librawspeed/decoders/DngDecoder.cpp 52.68% <6.66%> (-0.67%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@LebedevRI
Copy link
Member

Ok, i agree that this is an improvement, it very obviously fixes an existing CRW_8531.DNG sample.
This roughly matches what Translating White Balance xy Coordinates to Camera Neutral Coordinates says in the DNG spec, other than the rest of advanced steps mentioned there.

This "regresses" things in case where CalibrationIlluminant* is missing (e.g. Nikon LS-5000 sample),
or where we don't have a color matrix w/ CalibrationIlluminant = D65
(since that is what DngDecoder::parseColorMatrix() requires).

Thank you!

@LebedevRI LebedevRI merged commit 9031583 into darktable-org:develop Aug 24, 2023
44 of 47 checks passed
@kmilos kmilos deleted the kmilos/dng_asshotwhitexy branch August 24, 2023 06:17
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