Beep6581 / RawTherapee

A powerful cross-platform raw photo processing program
https://rawtherapee.com
GNU General Public License v3.0
2.7k stars 308 forks source link

Canon EOS 100D dual ISO DNG regression #5658

Closed madarexxx closed 1 year ago

madarexxx commented 4 years ago

Hi :) I am using Rawtherapee for a while and it is the great example of free software -- developing, bringing new ideas and possibilities to users.

Today I have met with the great regression --- Rawtherapee refused to open Dual ISO DNG file, created by Canon SL1 and converted to DNG by:

cr2hdr: a post processing tool for Dual ISO images
Last update: 185da25 on 2018-02-04 22:52:42 UTC by Dan:

I am running:

I have tested both 5.8 AppImage release from official site and 1:5.6-git20200218-1794~ubuntu18.04.1 version from PPA https://launchpad.net/~dhor/+ppa-packages and have got completely same behavior.

Following screenshots are demonstrating RT reaction on DNG file with a slightly modified Auto-Matched Curve Low ISO and Neutral profiles. The last one is Darktable, which opened DNG correctly.

Screenshot at 2020-02-19 23-16-18 Screenshot at 2020-02-19 23-16-40 Screenshot at 2020-02-19 23-16-53

Here is the DNG file I'm trying to process https://www.dropbox.com/s/gs5s0i4wug24maa/IMG_5431.DNG?dl=0

Thanks in advance :)

Beep6581 commented 4 years ago

@madarexxx which version of RawTherapee opened it correctly?

Beep6581 commented 4 years ago

The file you uploaded is not the same as the file in your screenshots, but the problem looks the same.

Beep6581 commented 4 years ago

Confirmed that it looks correct in 5.7.

Thanatomanic commented 4 years ago

Bisected it to this seemingly unrelated commit https://github.com/Beep6581/RawTherapee/commit/81f9ab73152c8896fd59c37e3adb1b8e3d563563

heckflosse commented 4 years ago

It also fails in ART, where I stole this from ;-)

heckflosse commented 4 years ago

@Thanatomanic

Bisected it to this seemingly unrelated commit

It turned out that the bisecting did not show the culprit. I thought about the reason why the bisecting did not show the culprit and I know the reason: If bisecting shows a commit which fixes a former uninitialized variable, the human input to bisecting is not reliable.

I talked with @agriggio about this issue and the fix for RT could be

diff --git a/rtengine/dcraw.cc b/rtengine/dcraw.cc
index 0858bdaaa..2b9f194ce 100644
--- a/rtengine/dcraw.cc
+++ b/rtengine/dcraw.cc
@@ -6123,7 +6123,8 @@ get2_256:
     {
       fseek(ifp, offsetChannelBlackLevel, SEEK_SET);
       FORC4
-          bls += (cblack/*imCanon.ChannelBlackLevel*/[c ^ (c >> 1)] = get2());
+          bls += (RT_canon_levels_data.cblack/*imCanon.ChannelBlackLevel*/[c ^ (c >> 1)] = get2());
+      RT_canon_levels_data.black_ok = true;
       imCanon.AverageBlackLevel = bls / 4;
       // RT_blacklevel_from_constant = ThreeValBool::F;
     }
@@ -6135,7 +6136,8 @@ get2_256:
       imCanon.SpecularWhiteLevel = get2();
       // FORC4
       //   imgdata.color.linear_max[c] = imCanon.SpecularWhiteLevel;
-      maximum = imCanon.SpecularWhiteLevel;
+      RT_canon_levels_data.white = imCanon.SpecularWhiteLevel;
+      RT_canon_levels_data.white_ok = true;
       // RT_whitelevel_from_constant = ThreeValBool::F;
     }

@@ -6143,7 +6145,8 @@ get2_256:
     {
         fseek(ifp, offsetChannelBlackLevel2, SEEK_SET);
         FORC4
-            bls += (cblack/*imCanon.ChannelBlackLevel*/[c ^ (c >> 1)] = get2());
+            bls += (RT_canon_levels_data.cblack/*imCanon.ChannelBlackLevel*/[c ^ (c >> 1)] = get2());
+        RT_canon_levels_data.black_ok = true;
         imCanon.AverageBlackLevel = bls / 4;
         // RT_blacklevel_from_constant = ThreeValBool::F;
     }
@@ -9867,7 +9870,8 @@ void CLASS identify()
     filters = 0;
     tiff_samples = colors = 3;
     load_raw = &CLASS canon_sraw_load_raw;
-    FORC4 cblack[c] = 0; // ALB
+    //FORC4 cblack[c] = 0; // ALB
+    RT_canon_levels_data.black_ok = RT_canon_levels_data.white_ok = false;
   } else if (!strcmp(model,"PowerShot 600")) {
     height = 613;
     width  = 854;
@@ -10531,6 +10535,14 @@ bw:   colors = 1;
     }
   }
 dng_skip:
+  if (!dng_version && is_raw) {
+      if (RT_canon_levels_data.black_ok) {
+          FORC4 cblack[c] = RT_canon_levels_data.cblack[c];
+      }
+      if (RT_canon_levels_data.white_ok) {
+          maximum = RT_canon_levels_data.white;
+      }
+  }
   if ((use_camera_matrix & (use_camera_wb || dng_version))
         && cmatrix[0][0] > 0.125
         && strncmp(RT_software.c_str(), "Adobe DNG Converter", 19) != 0
diff --git a/rtengine/dcraw.h b/rtengine/dcraw.h
index 89c1fcaff..b7439eeb5 100644
--- a/rtengine/dcraw.h
+++ b/rtengine/dcraw.h
@@ -192,9 +192,16 @@ public:
         unsigned int crx_track_selected;
         short CR3_CTMDtag;
     };
+    struct CanonLevelsData {
+        unsigned cblack[4];
+        unsigned white;
+        bool black_ok;
+        bool white_ok;
+        CanonLevelsData(): cblack{0}, white{0}, black_ok(false), white_ok(false) {}
+    };
 protected:
     CanonCR3Data RT_canon_CR3_data;
-
+    CanonLevelsData RT_canon_levels_data;
     float cam_mul[4], pre_mul[4], cmatrix[3][4], rgb_cam[3][4];

     void (DCraw::*write_thumb)();
Thanatomanic commented 1 year ago

Fixed after a long time by committing the changes proposed by @heckflosse in #6505 Thank you @GiMo84 for your PR to get this fixed.