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

Noise tools embed incorrect 'maker' and 'model' in presets.json #8992

Closed
tduell opened this issue May 18, 2021 · 22 comments
Closed

Noise tools embed incorrect 'maker' and 'model' in presets.json #8992

tduell opened this issue May 18, 2021 · 22 comments
Labels
bug: pending someone needs to start working on that no-issue-activity scope: noise profile adding noise profiles for new cameras

Comments

@tduell
Copy link

tduell commented May 18, 2021


Describe the bug/issue
I've generated noise profile data (presets.json file) for the new Pentax K-3 Mark III camera, using the noise tools (darktable-gen-noiseprofile) and attempted to test the noise profiles on images from the camera, starting darktable with the command 'darktable --noiseprofiles /home/terry/Pentax-K-3/presets.json'.
darktable does not recognise that the noise data in presets.json is applicable to the images.
If the presets.json file is edited to reflect what is in the image exif data, darktable then recognises that the noise data in the edited presets.json file is applicable to images from the camera.

To Reproduce
Please provide detailed steps to reproduce the behaviour, for example:

  1. Produce appropriate images from the camera and run 'darktable-gen-noiseprofile' as appropriate for those images to produce 'presets.json' file.
  2. Start darktable using the presets.json file in 1. above, and import images from the camera.
  3. Attempt to apply 'denoise (profiled)' on an image.
  4. denoise (profiled) uses 'generic poissonian'

The following notes elaborate in more detail;
darktable does not recognise the presets.json data is applicable to PENTAX K-3 Mark III DNG image.
The first few lines of the presets.json file as generated by the tools.

"version": 0,
"noiseprofiles": [
{
"maker": "PENTAX",
"models": [
{
"comment": "k-3 mark iii contributed by terry",
"model": "K-3 Mark III",
"profiles": [

The first few lines of edited presets.json to reflect what is in the exif.
darktable recognises that the edited presets.json data is applicable to PENTAX K-3 Mark III DNG image.

"version": 0,
"noiseprofiles": [
{
"maker": "RICOH IMAGING COMPANY, LTD",
"models": [
{
"comment": "k-3 mark iii contributed by terry",
"model": "PENTAX K-3 Mark III ",
"profiles": [

An extract from the output of exiftool for a PENTAX K-3 Mark III DNG file.

File Type : DNG
File Type Extension : dng
MIME Type : image/x-adobe-dng
Exif Byte Order : Little-endian (Intel, II)
Make : RICOH IMAGING COMPANY, LTD.
Camera Model Name : PENTAX K-3 Mark III
Orientation : Horizontal (normal)
Software : PENTAX K-3 Mark III Ver. 1.01

Expected behavior
denoise (profiled) will show 'found match for ISO...'

Screenshots
'Selection_012.jpg' shows result with original presets.json file.
'Selection_013.jpg' shows result with edited presets.json file.

Platform
_Please fill as much information as possible in the list given below. Please state "unknown" where you do not know the answer and remove any sections that are not applicable _

  • darktable version : release-3.5.0-2257-gd48e51476
  • OS : Linux
  • Linux - Distro : Fedora 33
  • Memory : 16GB
  • Graphics card : Geforce GTX750Ti
  • Graphics driver : nouveau
  • OpenCL installed : yes
  • OpenCL activated : yes
  • Xorg : ?
  • Desktop : Cinnamon
  • GTK+ :
  • gcc :
  • cflags :
  • CMAKE_BUILD_TYPE :

Additional context
Please provide any additional information you think may be useful, for example:

  • Can you reproduce with another darktable version(s)? **yes with version x-y-z / Not tried

  • Can you reproduce with a RAW or Jpeg or both? RAW-file-format/Jpeg/both Can reproduce with other images from the subject camera.

  • Are the steps above reproducible with a fresh edit (i.e. after discarding history)? yes/no Not tried
    ![

  • If the issue is with the output image, attach an XMP file if (you'll have to change the extension to .txt)

  • Is the issue still present using an empty/new config-dir (e.g. start darktable with --configdir "/tmp")? yes/no
    Selection_012](https://user-images.githubusercontent.com/76498546/118600956-01343000-b7f5-11eb-8620-24c3075a3b58.jpg)
    Selection_013

@kmilos
Copy link
Contributor

kmilos commented Jun 14, 2021

Perhaps this can be closed as non-issue?

Some background: the way it works now is that rawspeed's "translated/unified/sanitized/normalized" values for make and model are used first (whether it's a DNG or a vendor raw format), and then the ones from the file as a fallback. So after implementing support in rawspeed, see now how it now works out for K-3 Mark III and GR III (try running the noise tools on master again).

For "proper" cameras, if it doesn't exits, one should add it first to cameras.xml with the normalized mapping in the field (either manually first for testing and generating the noise profile, but eventually it should get into rawspeed via a camera support issue). I guess for smartphones and other devices one currently doesn't bother...

@johnny-bit
Copy link
Member

Best case scenario should be: "Do not 2nd guess DNG data" and ideally in case there's no info in cameras.xml we should assume that the data in DNG is correct one. And the cameras.xml should only be used for outliers like vendor-only raw formats if I understand correctly... Dunno where the discrepancy for noise tools comes from, but i do know that wb tools will just bail on unknown camera...

Maybe this should be open to investigate possible code improvements for noise/wb tools when the data is in readable format (eg DNG) and there's no entry in cameras.xml?

@kmilos
Copy link
Contributor

kmilos commented Jun 14, 2021

"Do not 2nd guess DNG data" and ideally in case there's no info in cameras.xml we should assume that the data in DNG is correct one. And the cameras.xml should only be used for outliers like vendor-only raw formats if I understand correctly...

There seem to be a plethora of "DNG only" output Pentax/Ricoh and other vendor models that are present and "normalized" already, so we're not talking about outliers but intended operation to keep the vendor names (they keep putting different ones in there like "PENTAX RICOH IMAGING" vs "PENTAX" vs "RICOH IMAGING COMPANY, LTD." vs "PENTAX Corporation", or "LEICA CAMERA AG" vs "Leica Camera AG", or "SAMSUNG DIGITAL IMA" vs "samsung" vs "SAMSUNG TECHWIN Co.", or "LGE" vs "LG Mobile") and model schemes sane...

@johnny-bit
Copy link
Member

johnny-bit commented Jun 14, 2021 via email

@kmilos
Copy link
Contributor

kmilos commented Jun 14, 2021

So - what's your proposition here? Actually close as non-issue and keep in mind that some companies try to be fAnCy and make maintainer's life harder?

This particular case is IMHO a non-issue (or rather, premature noise profile merge before basic camera support was analyzed and implemented). Perhaps there needs to be a warning about this in the wiki?

Going forward, maybe a couple of ideas... Instead of creating a translation entry for each new model (talking about "DNG only" here that doesn't require any additional information usual for cameras.xml):

  1. Assume in the general case most vendors use a sane name (and model scheme) consistently, just capitalize it properly (e.g. using tolower()/toupper()), and use model as is
  2. For the ones that are not sane, maintain a per vendor (not per model) translation dictionary
  3. Use individual model mappings when truly needed

OTOH, I guess the current rawspeed "normalization" implementation is very neat/regular programmatically so there is no push to change it, but the burden remains on keeping cameras.xml up to date...

@johnny-bit
Copy link
Member

johnny-bit commented Jun 14, 2021 via email

@johnny-bit
Copy link
Member

However I think there MIGHT be and issue with denoise (profiled) module, since on exif data match it should take the data but according to the issue at hand - it doesn't.

@kmilos
Copy link
Contributor

kmilos commented Jun 14, 2021

Don't think so - AFAICT the Exif data is sanitized through cameras.xml first , or simply passed through if an entry doesn't exist there, and only then stored in dt_image_t.make and dt_image_t.model to be used by all other modules...

Part of the issue was perhaps that the attempted "RICOH IMAGING COMPANY, LTD." contained a comma and period which were not allowed/supported in vendor name matching of the denoise module when added by hand - only [a-zA-Z ] are supported precisely because we rely on some sane sanitization... Potentially this could be expanded to any ASCII character to support these manual hacking/testing cases?

@johnny-bit
Copy link
Member

OK, so:

noise tools correctly generate name for "unknown" camera: "PENTAX" as maker and "K-3 Mark III" as model

however due to exif data not going through cameras.xml "sanitization" process the module gets "RICOH IMAGING COMPANY, LTD."

this is problematic because we allow either "Pentax" or "Ricoh" in that case as maker in noiseprofiles.

Both problems for ricoh cameras were because exif reported "RICOH IMAGING COMPANY, LTD." instead of either Pentax or Ricoh...

so... how about we go with your suggestion from here:

For the ones that are not sane, maintain a per vendor (not per model) translation dictionary

just where to put good sanitization? in dt_image_refresh_makermodel or rather dt_rawspeed_lookup_makermodel? I think the later.

@kmilos
Copy link
Contributor

kmilos commented Jun 15, 2021

noise tools correctly generate name for "unknown" camera: "PENTAX" as maker and "K-3 Mark III" as model

however due to exif data not going through cameras.xml "sanitization" process the module gets "RICOH IMAGING COMPANY, LTD."

this is problematic because we allow either "Pentax" or "Ricoh" in that case as maker in noiseprofiles.

I think this is sums it up, with the exception of "correctly"... The potential bug here is how noise tools figures out "PENTAX" as make when that is not what Exif (or sanitized rawspeed) data contain?

Exif.Image.Make                               RICOH IMAGING COMPANY, LTD.     
Exif.Image.Model                              PENTAX K-3 Mark III

Both problems for ricoh cameras were because exif reported "RICOH IMAGING COMPANY, LTD." instead of either Pentax or Ricoh...

so... how about we go with your suggestion from here:

For the ones that are not sane, maintain a per vendor (not per model) translation dictionary

Unfortunately not going to work for this case since the mapping is not bijective: you choose either Pentax or Ricoh depending on the model, so has to be done on per model basis, like rawspeed already does...

just where to put good sanitization? in dt_image_refresh_makermodel or rather dt_rawspeed_lookup_makermodel? I think the later.

I don't think adding yet another layer of sanitization is the ideal approach, as it runs a danger of becoming out of sync w/ rawspeed.

@kmilos
Copy link
Contributor

kmilos commented Jun 15, 2021

In fact, noise tools are already out of sync w/ rawspeed (i.e. cameras.xml) as they try to figure things out independently, so there's the root cause of this bug report.

@kmilos
Copy link
Contributor

kmilos commented Jun 15, 2021

And to top it off, there is third attempt at this when extracting data for adobe_coeff and cameras.xml...

(Mind you even this scheme does not work out for Pentax cameras...)

Maybe a good first step would be to abstract this method and replace the one in tools/noise/subr.sh so at least there is a same starting point everywhere. I guess there will always be cases where we still need to tweak through cameras.xml.

@johnny-bit
Copy link
Member

johnny-bit commented Jun 15, 2021 via email

@kmilos
Copy link
Contributor

kmilos commented Jun 15, 2021

can you do pr for the tools?

Might give it a go at some point, no promises though...

Another thought: convert noise tools to Python and use cameras.xml just like you did for extracting white balance ;)

@johnny-bit
Copy link
Member

You can do that too :P

actually the order imo should be:

try cameras.xml -> if that's a no-go, then try to "guess".

@kmilos
Copy link
Contributor

kmilos commented Jun 16, 2021

The PR only slightly improves the situation in a sense that tools for noise profile, adobe_coeffs, and rawspeed (cameras.xml) should use the same "guessing" mechanism so they are in sync.

There is still a requirement for an entry in cameras.xml to exist in order for everything to fall in place, there is no fallback to "unfiltered" Exif make and model.

@github-actions
Copy link

This issue did not get any activity in the past 30 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.

@johnny-bit
Copy link
Member

Since the PR got merged... what would be the next steps? Syncronize guessing part across all tools (lens/exif/noise)?

@kmilos
Copy link
Contributor

kmilos commented Aug 9, 2021

Don't think it's that easy - lensfun relies on unfiltered Exif data, while the rest of darktable seems to use rawspeed sanitized data, so there is no one place to rule them all...

At best I guess we should just document and put warnings/gotchas for each tool & output file...

@johnny-bit
Copy link
Member

johnny-bit commented Aug 9, 2021 via email

@github-actions
Copy link

github-actions bot commented Oct 9, 2021

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

This issue did not get any activity in the past year and thus has been closed. Please check if the newest release or master branch 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 Mar 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug: pending someone needs to start working on that no-issue-activity scope: noise profile adding noise profiles for new cameras
Projects
None yet
Development

No branches or pull requests

3 participants