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

dcraw: prefer raw crop and mask from dng metadata #7016

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

sgotti
Copy link
Contributor

@sgotti sgotti commented Mar 29, 2024

Prefer raw crop and mask from dng metadata instead of camconst.json entries.

The unique exception, to keep backward compatibility, is from in camera Pentax DNGs. In such case, the preference goes to a camconst entry, if it exists.

This is based on discussion (see last comments) in #6826

It uses the same logic already implemented for RT_whitelevel_from_constant and RT_blacklevel_from_constant

To sum up, from my understanding of the code and after reading some old discussions/issues:

How to test

To test this PR use, for example, a Fuji XT3 file and convert it to dng with ADC. Change camconst to remove the source size filter from raw_crop:

--- a/rtengine/camconst.json
+++ b/rtengine/camconst.json
@@ -1596,10 +1596,7 @@ Camera constants:
     { // Quality A, samples provided by Daniel Catalina (#5839) and pi99y (#5860)
         "make_model": [ "FUJIFILM X-T3", "FUJIFILM X-PRO3" ],
         "dcraw_matrix": [ 13426,-6334,-1177,-4244,12136,2371,-580,1303,5980 ], // DNG_v11, standard_v2 d65
-        "raw_crop" : [
-           { "frame" : [6384, 4182], "crop": [ 0, 5, 6252, 4176] },
-           { "frame" : [6384, 3348], "crop": [624, 0, 5004, 3348] }
-        ],
+        "raw_crop": [ 0, 5, 6252, 4176],
         "white": [ 16170, 16275, 16170 ] // typical safe-margins with LENR
         // negligible aperture scaling effect
     },

Without this PR crop data is taken from camconst and the file is wrongly demosaiced, with this PR the crop data is taken from DNG metadata and correctly demosaiced.

@Lawrence37 @kmilos @Entropy512

Prefer raw crop and mask from dng metadata instead of camconst.json
entries.

The unique exception, to keep backward compatibility, is from in camera
Pentax DNGs. In such case, the preference goes to a camconst entry, if
it exists.
@Lawrence37
Copy link
Collaborator

We should review camconst.json for any raw crops for dng-only cameras, aside from Pentax since it's already handled by your code.

@sgotti
Copy link
Contributor Author

sgotti commented Mar 31, 2024

We should review camconst.json for any raw crops for dng-only cameras, aside from Pentax since it's already handled by your code.

I know only of leica SL cameras. The SL, SL2 and the just released SL3 creates only dng files. Not sure about other Leica models.

Since this will be just a cleanup (without side effects), do you prefer to do this in this PR or open an issue when this will be merged and do it in another PR?

Do you mean to check if the dng raw crop is different than the one in the camconst? In such case what should be done?

@Lawrence37
Copy link
Collaborator

If there's a raw crop for a dng-only camera, that means the crop is meant to override the values in the dng metadata. The camconst crop should take precedence just like it is with Pentax dngs.

@sgotti
Copy link
Contributor Author

sgotti commented Apr 3, 2024

@Lawrence37 @kmilos

For Leica SL cameras, their unique raw format is dng. I analyzed some samples for the three SL models and probably the same should also apply for the Q models

For example:

Leica SL2-S

There'a a relative raw crop defined in camconst.json [ 0, 2, 0, -4 ] . Please note that the the DNG Active Area is used as an initial raw_crop, so the entries in camconst.json, if relative like in this case, are doing an additional subtraction from the dng ActiveArea.

See also issue #6390.

So for a normal image:

  • raw dimensions: 6048 x 4048
  • dng active area/row crop: 0 0 6024 4048
  • camconst raw crop: 0 2 6024 4042

for a high resolution image:

  • raw dimensions: 12096 x 8082
  • dng active area/row crop: 0 0 12034 8082
  • camconst raw crop: 0 2 12034 8076

Something similar also for the other SL models.

With the raw crop provided by the dng I don't see any garbage pixels so I don't see where they came from when the entries were added.

So looks like the raw crop is not required in camconst and the ones in camconst are greater/too conservative since, when relative, they are substracting from something already cropped (dng ActiveArea) and not the whole raw size.

Also darktable/rawspeed doesn't have any crop but uses the dng values.

What should we do?

  • Remove the entries from camconst also it will change a bit current edits (is this considered a backward compatibility issue?)
  • keep the entries in camconst, document they aren't need but kept for bw compat, and perhaps remove them in 6.0?

@kmilos
Copy link
Contributor

kmilos commented Apr 3, 2024

I analyzed some samples for the three SL models and probably the same should also apply for the Q models

M series as well I think...

On RPU also CL and X2 samples are DNGs, and also X-U and TL2 at DPR.

@sgotti
Copy link
Contributor Author

sgotti commented Apr 3, 2024

M series as well I think...

On RPU also CL and X2 samples are DNGs, and also X-U and TL2 at DPR.

Yeah but many of these are not in camconst

$ grep -i leica rtengine/camconst.json | grep make_model
        "make_model" : [ "LEICA C-LUX", "LEICA CAM-DC25" ],
        "make_model" : "LEICA D-LUX 7",
        "make_model": "LEICA Q (Typ 116)",
    //    "make_model": "LEICA M8",
        "make_model": "LEICA Q2",
        "make_model": "LEICA SL (Typ 601)",
        "make_model": "Leica SL2",
        "make_model": "Leica SL2-S",
        "make_model" : ["LEICA V-LUX 5","Panasonic DC-FZ1000M2"],
        "make_model": [ "Panasonic DMC-FZ1000", "Leica V-LUX (Typ 114)" ],
        "make_model": [ "Panasonic DMC-LF1", "Leica C (Typ 112)" ],
        "make_model": [ "Panasonic DMC-LX100", "Leica D-LUX (Typ 109)" ],

Leica m8 commented out since it was causing issues and with just dng metadata it works (#6237 and #6498).

But we can't know if they are supported or just not tested by anyone

@Lawrence37 probably a list of supported cameras, also without any overrides should be kept. Perhaps camconst.json is the right place (just add an entry without any overrides or also add a field reporting it as unsupported) like done by rawspeed with cameras.xml. If it was not already discussed I'd like to open a dedicated issue.

@kmilos
Copy link
Contributor

kmilos commented Apr 3, 2024

Perhaps camconst.json is the right place (just add an entry without any overrides or also add a field reporting it as unsupported) like done by rawspeed with cameras.xml.

One caveat - dt/RawSpeed doesn't explicitly test nor mark as supported/unsupported DNG based models (except for Pentax maybe because of the dual PEF output), and many samples are missing on RPU indeed (Roman generally doesn't even want them unless they are to test some new functionality). DNGs are self-contained and should "just work" (assuming the implementation of various compression modes) w/o any additional metadata supplied or override. If that's not the case, it is probably an invalid/uncompliant file. Checking and listing all the possible devices outputting DNG is a fool's errand IMHO...

The presence of DNG based models in RawSpeed's cameras.xml is there just to normalize and prettify the Make/Model strings (and we pretty much draw the line at "real" camera bodies like Leica, Pentax/Ricoh, and Sigma; trying to cover smartphones and the myriad of other devices is not worth it IMHO...).

In other words, IMHO you should only use your camconst.json to exceptionally override some known problematic DNGs, not to track valid ones.

@Lawrence37
Copy link
Collaborator

As far as Leica dngs go, these models have raw crops in camconst.json.

  • LEICA Q (Typ 116)
  • LEICA Q2
  • LEICA SL (Typ 601)
  • Leica SL2
  • Leica SL2-S

For the Leica SL2-S, we know the original raw crop was [ 0, 2, 6024, 4042 ] with a comment explaining it:

2 rows at top and 4 rows at bottom are black

It is strange that the dng ActiveArea says otherwise, so I would like to confirm if there really are black rows.

The other models should be reviewed too, or just take the easy option of allowing Leica dng raw crops to be overridden by the camconst entries.

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.

None yet

3 participants