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

raw over exposed indication broken #12193

Closed
thecb1 opened this issue Jul 18, 2022 · 47 comments
Closed

raw over exposed indication broken #12193

thecb1 opened this issue Jul 18, 2022 · 47 comments
Labels
bug: invalid the bug is not a bug, but a feature no-issue-activity scope: image processing correcting pixels understood: clear devs have a complete bug report with all the relevant info to start fixing

Comments

@thecb1
Copy link

thecb1 commented Jul 18, 2022

Describe the bug/issue
The raw over exposed indication with its default settings does not mark the whole areas that are over exposed but its edges only.

To Reproduce
See sample zipped arw file.
nosky.zip

Screenshot with default settings
threshold_1_0

Expected behavior
Screenshot with threshold 0.99
threshold_0_9

Which commit introduced the error
I saw it with 4.0 release first.

Platform

  • darktable version : 4.0.0-1
  • OS : 5.15.53-2-lts
  • Linux - Distro : arch
  • Memory : 8G
  • Graphics card : GeForce GT 730
  • Graphics driver : nvidia-470xx-dkms 470.129.06-1
  • OpenCL installed : opencl-nvidia-470xx 470.129.06-1
  • OpenCL activated : yes
  • Xorg : xorg-server 21.1.3-7
  • Desktop : plasma
@TurboGit
Copy link
Member

Indeed, looks strange !

@TurboGit TurboGit added this to the 4.2 milestone Jul 18, 2022
@TurboGit TurboGit added the bug: pending someone needs to start working on that label Jul 18, 2022
@jenshannoschwalm
Copy link
Collaborator

I think this just wrong white point setting so probably no bug. Arw files are notorious for that...

@jenshannoschwalm
Copy link
Collaborator

Additional comment: The raw files include a 'white-point' defining the maximum processed value. That value is read from exif data by the rawspeed decoders.

If that value is wrong, clipped data are not calculated as clipped so the clipped visualizing is wrong. That's exactly what you see here, so for me not a bug in the mentioned part of the code. It might be reported as a rawspeed bug or as only some cameras / images are affected a sony firmware problem. (Sony raw are notorious for that, it also leads to problems handling highlights btw)

@thecb1
Copy link
Author

thecb1 commented Jul 20, 2022

@jenshannoschwalm
Thank you very much for the explanation. Actually it looks like the value is not read from exif but is the same as the default value in the cameras.xml from rawspeed.

find . -type f -name "*.ARW" -exec exiftool -WhiteLevel {} \; | sort | uniq -c
2380 White Level                     : 15360 15360 15360

The white point in the cameras.xml/that dt uses is 16383.

@jenshannoschwalm
Copy link
Collaborator

That exactly is the problem.!

@jenshannoschwalm
Copy link
Collaborator

So - why not "dig into this" deeper and try to fix it. I have not seen pr's by you so far but this might be a starting point to contribute to the dt project :-) And - it would help many Sony users :-)

@thecb1
Copy link
Author

thecb1 commented Jul 20, 2022

So - why not "dig into this" deeper and try to fix it. I have not seen pr's by you so far but this might be a starting point to contribute to the dt project :-) And - it would help many Sony users :-)

I am. I try to figure out how to make sure the proprietary values parsed by exiftools are reliable enough to fix it that way in rawspeed.

@TurboGit You want to close this issue? (Because you added it to the milestone)

@jenshannoschwalm
Copy link
Collaborator

A quick reminder, we will need to take care of old edits somehow. That might be a bit tricky as we have to make dt aware of other coeffs here. Don't know how to make sure about this but we will find a way :-)
Good to see someone working on this nasty thing.

@parafin
Copy link
Member

parafin commented Jul 20, 2022

isn't whitelevel saved in rawprepare iop params for old edits?

@thecb1
Copy link
Author

thecb1 commented Jul 20, 2022

I hoped that dt would keep the settings of the raw b/wp module and sets the value only on import or history reset.

I skimmed over some issues from rawspeed and some code from other raw software. I think one needs to inspect a variety of raw files shot with different settings first. It smells like there are more factors than just ISO that influence the black and white points. E.g. as I mentioned in chat the other day, the A6400 likes to switch between 12 and 14 bit with different settings.

@TurboGit
Copy link
Member

isn't whitelevel saved in rawprepare iop params for old edits?

Yes, you're right @parafin those are saved so we can safely change the value which will be used for new edits or if the history stack is reset.

@TurboGit
Copy link
Member

@TurboGit You want to close this issue? (Because you added it to the milestone)

Not sure to understand. I did add a milestone to have a target release for the fix. The issue will be closed only if we decide to take no action or if a fix is merged.

@jenshannoschwalm
Copy link
Collaborator

jenshannoschwalm commented Jul 22, 2022

@TurboGit @aurelienpierre and others.
There is definitely a problem in current whitepoint setting, don't yet understand the issue exactly. So far we thought it was some sony stuff, here is a link to a canon image issue, same probem.

https://discuss.pixls.us/t/highlight-reconstruction-darktable-vs-rawtherapee/31790/7

I think this is an issue we have to fix asap.

can this be related to moving away from adobe coeffs?

EDIT: a first lookup shows colordata.cpp to contain a maximum value (whitepoint?) If we use that data, are we sure that rawprepare takes parameters in the same way as rawprepare uses them?

@TurboGit TurboGit added priority: high core features are broken and not usable at all, software crashes understood: unclear devs lack most or all important info and can do nothing, the report will be closed after 2 weeks scope: image processing correcting pixels labels Jul 22, 2022
@TurboGit TurboGit modified the milestones: 4.2, 4.0.1 Jul 22, 2022
@jenshannoschwalm
Copy link
Collaborator

Maybe change the issue title to get more attention?

@TurboGit TurboGit changed the title Raw over exposed indication marks only the edges on sony arw raw over exposed indication broken Jul 23, 2022
@jenshannoschwalm
Copy link
Collaborator

I further checked the mentioned cr2 file and also here the whitepoint is simply wrong.
dt uses 15831 instead of what exiftool says

Normal White Level              : 11767
Specular White Level            : 12279

@pehar1
Copy link

pehar1 commented Jul 23, 2022

The uploaded Sony raw contains two different White Level tags :

exiftool -H -G -a nosky.ARW | grep "White Level"
[EXIF]          0xc61d White Level                     : 16383
[MakerNotes]    0x787f White Level                     : 15360 15360 15360

Using the value from the MakerNotes in raw black/white point the indicator is working as expected.
The same applies for the image in the thread @jenshannoschwalm mentioned :

exiftool -H -G -a 2014-05-30_19-47-01.cr2 | grep "White Level"
[MakerNotes]    0x02cf Normal White Level              : 11767
[MakerNotes]    0x02d0 Specular White Level            : 12279

Edit: I checked a complete folder (129 images) of ARW files from my old Sony SLT-A77. For all files dt used a white point of 16596 in raw black/white point while the value reported by exiftool is 15360. These files do not contain the [EXIF] tag, only the [MakerNotes] as shown above.

@TurboGit
Copy link
Member

TurboGit commented Jul 23, 2022

I tested a NEF image with dt 3.4 & 4.0 :

3.4:
image

4.0:
image

An indeed I don't have the exact same area displayed. So something has changed, maybe the move to rawspeed adobe coefs indeed.

EDIT:

4.0 with 60a65ac reverted:

image

(close to what we had in 3.4).

@thecb1
Copy link
Author

thecb1 commented Jul 23, 2022

The uploaded Sony raw contains two different White Level tags :

exiftool -H -G -a nosky.ARW | grep "White Level"
[EXIF]          0xc61d White Level                     : 16383
[MakerNotes]    0x787f White Level                     : 15360 15360 15360

Indeed. I compared a bunch of ARWs from my 6400. In 14bit mode 0xC61D is always 16383 (14bit int max), but in 12bit mode it's always 16380. Anyway, the Sony specific value in 0x787F is always 15360, even in high ISO modes. For the moment 15360 looks more correct than maxint. But I don't know how to proof this is the absolute correct white level to use.

Other findings: The black level switches from 512 to 2048 (all four) on ISO 64000 and above. Though "64000" is read from exif data and not the actual camera setting.

@TurboGit
Copy link
Member

can this be related to moving away from adobe coeffs?

I'm not sure about this, my understanding is that would have also made a visible change on images. This has happened for some cameras (very limited number as far as I know) where there was discrepancies between the old coefs recorded in dt and the ones now used from rawprepare.

@AxelG-DE
Copy link

Just some random thoughts from a non-coder...

I learned, the wrong whitepoint is set already since long time, just the effect is more clearly visible recently.

Can that be due to so "door openings" during libraw integration?

This thought wouldn't finally solve the correct pick up, but maybe helps something for analysis or comprehensive understanding?

@TurboGit
Copy link
Member

Can that be due to so "door openings" during libraw integration?

I don't think so since the API for this library is only called when no rawspeed support is found for a specific image format. You can check which module to load the RAW has been used by looking at the flags value in image information module. The last character is r for rawspeed and l for libraw (tooltip tell you about this).

@jenshannoschwalm
Copy link
Collaborator

I'm not sure about this, my understanding is that would have also made a visible change on images.

I checked a bit further.
My current understanding is like:

  1. The raw overexposed visualizing is absolutely correct as we have it now if we define overexposure as values above a certain value and that value is defined as threshold*whitepoint
  2. Old and new code take the whitepoint from rawspeed / libraw (from my tests these settings did not change at all) but the values seem to be constant (dng work fine here btw as they have defined exif data)
  3. For some images there are makernotes that would give correct values but we can't/don't currently read them. This results in incorrect whitepoints. All dsc.processed_maximum[k] = 1.0f are set this way.

Questions for me now:

  1. Could we check for existing range-of-data and normalize in rawprepare?
  2. Any chance to read more makernotes

@jenshannoschwalm
Copy link
Collaborator

but see Pascal's result above with NEF...

That's what @jandren changed and i think that's correct. The visualizing is not the issue for me ...

@TurboGit
Copy link
Member

That's what @jandren changed and i think that's correct.

I'm not saying that it is not correct, just that it may explain the difference we see between 3.8 & 4.0.

@TurboGit
Copy link
Member

4.0 with 60a65ac reverted:

image

(close to what we had in 3.4).

@TurboGit
Copy link
Member

TurboGit commented Jul 23, 2022

And the OP picture with 60a65ac reverted:

image

@aurelienpierre
Copy link
Member

That would be an issue for Rawspeed, to read exif white if any. Hacking it in rawprepare.c is a nasty workaround.

@jenshannoschwalm
Copy link
Collaborator

Yes, unfortunately we just have a wrong whitepoint for such images.

@jenshannoschwalm
Copy link
Collaborator

I looked up exiv2 code and unfortunately there a no whitepoint tags in makernotes implemented (so far) so rawspeed can't do anything here too.

Probably it can be done via exiftool & lua.

@parafin
Copy link
Member

parafin commented Jul 24, 2022

rawspeed itself doesn’t use exiv2, does it? It parses necessary tags itself I think.

@jenshannoschwalm
Copy link
Collaborator

No it doesn't, but it uses tags "known" being safe thus in exiv2 i guess.

@jandren
Copy link
Contributor

jandren commented Aug 2, 2022

Yes @TurboGit the changes I introduced will make the issue with an incorrect white point surface. The reason is simple. We previously multiplied the channels with the white balance values before checking for clipping. The constants were generally >1 and thus pushed these (and unclipped) pixels above the threshold.

@TurboGit TurboGit removed this from the 4.0.1 milestone Aug 3, 2022
@TurboGit TurboGit added bug: invalid the bug is not a bug, but a feature understood: clear devs have a complete bug report with all the relevant info to start fixing and removed understood: unclear devs lack most or all important info and can do nothing, the report will be closed after 2 weeks priority: high core features are broken and not usable at all, software crashes bug: pending someone needs to start working on that labels Aug 3, 2022
@andrewk8
Copy link

DT4.0 Windows.
I don't understand open but bug invalid. I shoot Sony and was affected by the incorrect white point value, too. Someone in at pixls pointed to the bug report.
2022-08-21 20_19_30-darktable

@AxelG-DE
Copy link

@andrewk8 it was me pointing you here.

When you read above, the wrong whitepoint isn't picked up by dt, but by the independent library called rawspeed. Hence invalid, as it is not a dt bug.

Whether an upstream issue has been opened, I donno...

@AxelG-DE
Copy link

Do I understand this right, that this rawspeed-PR is related?

@andrewk8
Copy link

There was a change in raw over-exposed indication between DT 3.8.1 and DT 4.0.

The same image in DT 3.8.1 shows the "wrong" white point per Sony ARW. But the raw over-exposed indicator marks pixels that you expect it to mark.
DT 3 8 1 windows

@andrewk8
Copy link

andrewk8 commented Sep 1, 2022

Is this really two different issues that need to be split?

  1. whether or not DT can or should use a camera-specific white point
  2. regression from 3.8.1 to 4.0 in the display of raw over-exposed indication

@jorismak
Copy link

jorismak commented Sep 6, 2022

  1. DT uses Rawspeed, Rawspeed uses it's own exif reading or exiv2 I believe (don't actually know). The problem is that for some cameras, the correct white-level is not recorded in EXIF data, but in MakerNotes data (which is another set of proprietary metadata in a raw file). Both DT and Rawspeed have no current way of reading that, so it kinda stalls there. But from a DT perspective: DT is displaying over-exposure absolutely fine and as it should be, and it is setting the white-level as it receives it from rawspeed / libraw. But rawspeed is wrong in some cameras these days. Sony has a tendency to record 16383 in the EXIF data (because that is the maximum number a value could be, mathematically. It's the limit of 14bit numbers). But it records the true limit of the sensor in the MakerNotes, and that's the value we actually need. MakerNotes parsing is different from camera-brand to camera-brand, making it kinda difficult to 'quickly fix'.

  2. That is no regression. The current behavior is the correct one. Before DT displayed the over-exposure indicator after white-balancing (or some basic white balancing) had happened, which changes the data so a wrong displaying of what is clipped or not. AFAIK, the current behavior is absolutely correct. Yes, that means (like @TurboGit mentioned) that there are differences when looking at 3.4 and 4.0, but those differences are an honest, good 'fix'. The 3.4 displaying was wrong, the 4.0 displaying is correct. I don't recall if this over-exposure-before-wb fix was done after 3.8 or before (that's what you get when you often compile master, I have no clue which fixes are in which version).

Tools like Rawtherapee fix this by having a big json file with all supported cameras, and their black- and white-levels depending on camera-model and ISO (camconst.json). But there might be more variables that determine black/white levels.

For my Sony a7-m2, I just have a preset now that auto-applies to set the white-level to 15360. And in a whole folder of images, I often do a quick exiftool check to see if there are any files NOT having 15360. But I hardly see them (and I hardly change shooting modes on my camera, so that might be related 😉 ).

But when toying around and/or helping with playraw files over at discuss.pixls.us, the first thing I do is always compare the (specular) white-level reported by exiftool vs what Darktable has set by default, because that very often fixes problems people are having with 'recovering highlights' in more modern Canons (and older Sony's).

@kmilos
Copy link
Contributor

kmilos commented Sep 6, 2022

  1. DT uses Rawspeed, Rawspeed uses it's own exif reading or exiv2 I believe (don't actually know).

Rawspeed does not use exiv2.

that very often fixes problems people are having with 'recovering highlights' in more modern Canons

For "more modern" Canons (I'm assuming we're talking exclusively CR3 output here), dt uses LibRaw instead of rawspeed, so we should check if specular white is available for those only. (LibRaw also parses metadata on its own.)

Edit: For CR3s, we already try to get to correct specular white as provided by LibRaw? If not correct, please suggest a fix.

@jorismak
Copy link

jorismak commented Sep 6, 2022

No, it's also there in CR2s. For instance, the 5D Mark IV. Older canons in my book are mostly from the 5D Mark3 and before era, where Canon uses a different AD-converter step outside the sensor (causing their notorious read-noise reputation).

To be honest, I don't even know if the problem is 'only' on 'modern' Canons. That are all just my words.
The facts are that I've encountered in lots of ARW files and in quite some CR2 files that appear in the last month on the discuss.pixls.us forum.

@jenshannoschwalm
Copy link
Collaborator

Probably you are right. We just got aware of this issue because we have two recent algos for highlights and the new algo to show data above clip.

@github-actions
Copy link

github-actions bot commented Nov 7, 2022

This issue did not get any activity in the past 60 days and will be closed in 365 days if no update occurs. Please check if the master branch has fixed it and report again or close the issue.

@github-actions
Copy link

github-actions bot commented Sep 4, 2023

This issue was closed because it has been inactive for 300 days since being marked as stale. Please check if the newest release or nightly build has it fixed. Please, create a new issue if the issue is not fixed.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug: invalid the bug is not a bug, but a feature no-issue-activity scope: image processing correcting pixels understood: clear devs have a complete bug report with all the relevant info to start fixing
Projects
None yet
Development

No branches or pull requests