Beep6581 / RawTherapee

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

dcraw update tracker #1611

Closed Beep6581 closed 9 years ago

Beep6581 commented 9 years ago

Originally reported on Google Code with ID 1627

Umbrella issue for dcraw update requests

Reported by entertheyoni on 2012-11-19 17:30:07

Beep6581 commented 9 years ago
Hmm .. some serious changes for Panasonic Black levels .. now they are at 15 while the
old value was 143 !!.

I cannot find any +128 offset anywhere in the code but it would be very good if Dcraw
now reads Panasonic Black levels from the exif data.

Reported by iliasgiarimis on 2014-05-20 01:58:56

Beep6581 commented 9 years ago
Please advise what to do about tagging.

Reported by entertheyoni on 2014-05-20 08:23:13

Beep6581 commented 9 years ago
I 'll vote for including the updated Dcraw in 4.1 after checking the new behavior ..
if (in addition to new models support) the new dcraw calculates a correct black level
for Panasonics it is a serious reason to update 

I am searching for a working dcraw(9.21).exe now ..

Reported by iliasgiarimis on 2014-05-20 08:38:49

Beep6581 commented 9 years ago
Just checked dcraw921.exe with Panasonic files. 

Indeed now dcraw finds the basic black level (usually at 128 but can vary a lot) then
adds the offset of +15 
The Black level of recent Panasonics can vary very much (up to 20 12bit levels) so
this is a significant update.

Reported by iliasgiarimis on 2014-05-20 09:24:53

Beep6581 commented 9 years ago
Thanks Ilias, I'm working at the patch.

Reported by heckflosse@i-weyrich.de on 2014-05-20 09:30:56

Beep6581 commented 9 years ago
After looking at the changes in new dcraw, I vote for tagging 4.1 now and include the
dcraw changes in 4.2

Why?

1.) There are a lot of changes for phase one (mainly integration from Anders' changes
to dcraw.cc now are in official dcraw.c, but Dave modified them a bit and stripped
all of Anders' comments). I would like to get a review of this changes from Anders.
2.) Before 1.) is done, I won't patch dcraw.cc, because it could be I have to to it
twice if I patch dcraw.cc now.
3.) When dcraw.cc is patched, we should test at least one or two weeks.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-05-20 11:10:12

Beep6581 commented 9 years ago
OK then, lets leave it for later .. it's better to have plenty of time available because
there are a lot of things to do in this .. :)

And it will be nice to find an effective way to communicate with David ..

Reported by iliasgiarimis on 2014-05-20 11:22:50

Beep6581 commented 9 years ago
Here's the dcraw 9.21 patch

Reported by heckflosse@i-weyrich.de on 2014-05-26 11:40:07


Beep6581 commented 9 years ago
I'll have to check, whether it applies to revision 320e548d67d7 :-(

Reported by heckflosse@i-weyrich.de on 2014-05-26 12:48:01

Beep6581 commented 9 years ago
oops I just made some Hasselblad fixes to current dcraw after I've got a bunch of Hasselblad
raw files sent to me (patch is to recognize models properly), feel free to hate me...
but anyway my patch is simple. Added 'model3' and a few if-statements in the "Hasselblad"
section in parse_tiff_ifd()

Reported by torger@ludd.ltu.se on 2014-05-26 13:19:39

Beep6581 commented 9 years ago
(I'll stay away now until dcraw9_21 is in there)

Reported by torger@ludd.ltu.se on 2014-05-26 13:20:12

Beep6581 commented 9 years ago
No problem. I'll make a new patch :-)

Reported by heckflosse@i-weyrich.de on 2014-05-26 13:29:27

Beep6581 commented 9 years ago
Patch from #109 applies fine to revision 320e548d67d7, so no necessity to make a new
one.

Reported by heckflosse@i-weyrich.de on 2014-05-26 13:49:04

Beep6581 commented 9 years ago
Thanks Ingo :)

The patch applied and run fine except that it does not add the +15 black offset with
Panasonic files ..

The new Dcraw reads the variable BlackLevelRed - green - blue from exif(see the attached
png). Here we are fine as RT reads and applies the per channel values 
Then an offset must be added. Dcraw9.21 adds a hardcoded offset of +15 which RT fails
to apply. It reports in console BL=127 and I checked with the unpatched build (where
the BL is hardcoded as 128+15 = 143) vs the patched build .. there is visual difference
in brightness and and color shift visible and measurable on the dark patch of the cc24.

http://216.18.212.226/PRODS/panasonic-lx7/FULLRES/LX7hVFATB.RW2
http://216.18.212.226/PRODS/panasonic-lx7/FULLRES/LX7hSLI03200NR1.RW2 (sample with
variable green-red-blue BLs which RT reads and applies fine)

Now .. this +15 offset comes from the sum of BlackLevel1-2-3 (tags decoded by Rawdigger/libraw
team) .. see the attached png ..
Looks like this is a constant for each model but it is not 15 for all models (GH4 has
12+4+1, others have 12+3+1) so it would be best to read and apply both the BlackLevelRed-green-blue
and the offset automatically or if this is difficult give the option to define it per
model in camconst.json. 
As it is now if I define "ranges": { "black": in camconst.json RT uses only the value
defined there i.e. the constant 143 and if I change it to 15 ( .. 16 .. 17) it will
use 15 as BL.
The BLs of Panasonics can vary much so the constant BlackLevel is not a good option
.. 

Reported by iliasgiarimis on 2014-05-26 20:51:13


Beep6581 commented 9 years ago
Though I'm really a fan of camcomst.json, I don't know much about the code behind it.
Anders, could you have a look at Ilias' suggestions?

Ingo

Reported by heckflosse@i-weyrich.de on 2014-05-26 21:07:28

Beep6581 commented 9 years ago
Sure, however I'm not sure if I follow what I should do.

Could you Ilias describe a bit for dummies exactly what changes/additions that are
required to camconst.json?

Reported by torger@ludd.ltu.se on 2014-05-27 08:07:21

Beep6581 commented 9 years ago
I think that we don't need any changes in camconst.json but in dcraw.cc so that RT behaves
on Panasonic files same as Dcraw.c which is
a) read the BlackLevelRed/green/blue tags' values (the patch works fine on this task)
b) add to the above values the black level (15) from Dcraw.cc (example for the LX-7
used in the samples)
7131     { "Panasonic DMC-LX7", 15, 0,
7132     { 10148,-3743,-991,-2837,11366,1659,-701,1893,4899 } }, ..

the patch fails on b) and should not ..

As camconst.json is in fact overwriting the above data, after this change in dcraw.cc
I will simply change the  "ranges { "black": 143 ... from Panasonic items in camconst.json
with "ranges { "black": 15 .. ( or 16 or 17 whichever is correct)

But it would be even better to expand Dcraw.cc to read and apply automatically the
BlackLevel1/2/3 upon BlackLevelRed/green/blue and not use any hardcoded values :) 

Reported by iliasgiarimis on 2014-05-27 09:28:36

Beep6581 commented 9 years ago
Ilias, I'll have a look whether I missed some thing in dcraw patch concerning b)

Ingo

Reported by heckflosse@i-weyrich.de on 2014-05-27 09:34:17

Beep6581 commented 9 years ago
I had a look. Missed nothing.

Reported by heckflosse@i-weyrich.de on 2014-05-27 10:12:26

Beep6581 commented 9 years ago
Screenshots
http://imgur.com/gXkb7qu
http://imgur.com/WeA3MJw

Dcraw.exe (v9.21) reports ..
Loading Panasonic DMC-LX7 image from LX7hVFATB.RW2 ...
Scaling with darkness 142, saturation 4095, and
multipliers 1.848485 1.000000 1.700758 1.000000
AHD interpolation...

While RT 4.1.8+dcraw921_00 reports .
RawTherapee, version 4.1.8
WARNING: closing this window will close RawTherapee!
..
Loading Panasonic DMC-LX7 image from C:\Users\Hlias\Desktop\RAW_TESTS-Kx-7D-ETC\GH2-E5-D3100\PANASONIC-LX7\LX7hVFATB.RW2...
no constants in camconst.json exists for "Panasonic DMC-LX7" (relying only on dcraw
defaults)
black levels: R:127 G1:127 B:127 G2:127 (provided by dcraw)
white levels: R:4095 G1:4095 B:4095 G2:4095 (provided by dcraw)
raw crop: 0 0 3664 2752 (provided by dcraw)
color matrix provided by dcraw
cam_mul:[488,000000 264,000000 449,000000 0,000000], AsShotNeutral:[0,540984 1,000000
0,587973 0,000000]
pre_mul:[1,000000 0,540984 0,920082 0,540984], scale_mul:[16,515877 8,934819 15,195961
8,934819], cblack:[127,000000 127,000000 127,000000 127,000000]
rgb_cam:[ [ 1,491964 -0,268919 0,030672], [-0,298067 1,633737 -0,577490], [-0,193898
-0,364818 1,546818] ]
cam_mul:[488,000000 264,000000 449,000000 0,000000], AsShotNeutral:[0,540984 1,000000
0,587973 0,000000]
pre_mul:[1,000000 0,562128 0,901539 0,564836], scale_mul:[16,515877 9,284041 14,889712
9,328763], cblack:[127,000000 127,000000 127,000000 127,000000]

Unpatced RT4.1.9 reports the harcoded BL 143 ..

Reported by iliasgiarimis on 2014-05-27 11:34:50

Beep6581 commented 9 years ago
I can't find that damned line of code in dcraw.c where black is added to cblack[..].

Reported by heckflosse@i-weyrich.de on 2014-05-27 14:11:11

Beep6581 commented 9 years ago
Ok, we have to make a change in rawimage.cc, because the addition of cblack[3] to black
is in main function of dcraw.c, which we don't have in dcraw.cc

It seems that in past either black was > 0 or cblack[] was > 0, but not both, so not
adding both had no negative impact.
If I'm right we have to add black to cblack[] (if black > 0). But this would also mean,
that we have to change the entries in dcraw_coeff_overrides(), at least the ones for
panasonic cams with black_level != -1. Same for panasonic camconst entries.

I can make a patch with the above mentioned change, but please tell me if I'm right,
first.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-05-27 15:30:34

Beep6581 commented 9 years ago
Patch also crashes with images from Sony ILCE 3000...

Reported by heckflosse@i-weyrich.de on 2014-05-27 18:06:02

Beep6581 commented 9 years ago
If we remove the "raw crop" line from ilce-3000 in camconst.json it does not crash.
In the present state (after Dcraw corrected the frame size) there no need to keep some
Sony models in camconst.json

Reported by iliasgiarimis on 2014-05-27 19:31:19

Beep6581 commented 9 years ago
This crash occurs when the raw crop in camconst.json is larger than what is defined
in dcraw.cc (lines 8400 + ) 

I had defined a 4 pixels larger width to counteract on the 4 pixels border that RT
puts on raw crop :)

It is not new .. now I remember it happened to me again some months ago when I wrote
the ILCE-6000 item in camconst.json. The raw size definitions already existed for 6048
pixel width sensors and it crashed on ilce-6000 while for ILCE-3000/5000 there was
no definitions so I was free to choose the desired raw crop without crashes :)

Reported by iliasgiarimis on 2014-05-27 20:09:04

Beep6581 commented 9 years ago
Thank Ilias, So this should be no problem. Now we have to decide, whether black level
in camconst.json should be absolute or relative (added to cblack values from exif).

Ingo

Reported by heckflosse@i-weyrich.de on 2014-05-27 20:46:25

Beep6581 commented 9 years ago
It has to be same as "black" at the current state of Dcraw .. i.e. Relative. 
It cannot be anything else because by "ranges": {"black": .. we ovewrite Dcraw's black
..
It should be so to gain the adaptation on the variability of Black Levels.

Even if it would be better to separate the two inserting a new term as "black offset
constant". But even without the new term I see no problem if the proper documentation
of the subject exists :)  

Ideally (and I think it is not a difficult task) there should be no need for BL constants
:) all Black Levels are defined by the manufacturers in exif (or maker) data or (for
Canons) can be calculated from the masked area. I wonder why David stoped midway for
Panasonic BLs !! ??

Same with white levels .. we can find the data in exifs and it looks to be pretty accurate
info .. even if it looks a bit over-conservative sometimes.

Hopefully Anders (or any other with time available) will find the time to rework BL/WL.

Additionally I wish the RT team could cooperate with David and the libraw/rawdigger
team, on raw decoding in general. 

Reported by iliasgiarimis on 2014-05-27 21:55:00

Beep6581 commented 9 years ago
Is it difficult to parse all the needed tags to have a complete correct BL for Panasonics
?? .. three tags are already used, three more are needed to be complete ..

Reported by iliasgiarimis on 2014-05-27 21:58:11

Beep6581 commented 9 years ago
Hi Ilias,

re #129: I totally agree about making 'black' relative. But we'll have to document
it well for the users, which made their own camcons changes for panasonic cams to avoid
confusion and spending hours to answer bug reports.

Concerning cooperation with David: That's my wish too!!!

re #130: I don't know anything about these 'three more' tags. Do you have some more
information?

I'll make a patch now for the 'relative' part, could you please make the changes to
camconst.json and dcraw_coeff_overrides afterwards? I'm not sure, which entries I'll
have to change.

Patch follows soon.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-05-27 22:15:54

Beep6581 commented 9 years ago
Hi Ingo,

Relative to the three more tags ..

I mean the BlackLevel1-2-3 you can see in the attachment in #116 (it's a screenshot
of exif data as shown by exiftool and exiftoolGUI as frondend). The black offset for
Panasonics is equal to the sum of these three tag values (12+2+1=15)

Reported by iliasgiarimis on 2014-05-27 22:30:12

Beep6581 commented 9 years ago
Ahh, ok, now I see. The green colored area in your attachment confused me. Here's the
patch which adds the 'relative' black levels (not the ones mentioned in #132, but the
ones hardcoded in dcraw (+15 for panasonic). As we have some time until 4.2 gets released,
I'm sure we'll find a good solution for #132 too :-)

In case someone wants to test this patch, please wait for Ilias' corrections to camconst.json
and dcraw_coeff_overrides.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-05-27 23:16:15


Beep6581 commented 9 years ago

Reported by heckflosse@i-weyrich.de on 2014-05-28 09:50:20

Beep6581 commented 9 years ago
I tested the patch .. worked fine in my limited tests after updating the camconst.json
with the new BL for Panasonics.

I updated camconst.json with all Panasonic models included in rawimage.cc so there
is no need for changes there at the moment.

Except from detailed BL/WL for the mentioned Panasonic models updated is the relative
documentation for the new behavior of BL.

Also a correction for the Samsung NX-mini "raw crop"

I could not build a patch for this because my setup is a mess now so I am uploading
the hole camconst.json

I will be away for the next week with limited access to the web ..

Reported by iliasgiarimis on 2014-06-03 01:26:52


Beep6581 commented 9 years ago
Ιf anyone wants to test the patch at #133 with Panasonic FX-150 raws please correct
in the camconst.json at #135 the typing error "DMC-FZ150" with "DMC-FX150" .. 

Reported by iliasgiarimis on 2014-06-05 11:09:28

Beep6581 commented 9 years ago
Ilias, the patch in combination with your camconst from #136 still crashes with some
sony files. Could you correct these camconst entries too or tell me what to do?

Ingo

Reported by heckflosse@i-weyrich.de on 2014-06-05 11:19:13

Beep6581 commented 9 years ago
Ingo, please define which files are crashing ..

OOHHH .. sorry ..

If it is for ILCE-3000/5000 it is because I forgot to either comment them out or change
"raw_crop" width to 5472 from the existing 5476

(dcraw.cc 8400+ 

 } else if (!strcmp(make,"Sony") && raw_width == 5504) {
    width -= height > 3664 ? 8 : 32;

see here 5504-32=5472

It would be better to find the reason it crashes and eliminate it .. we should be free
to choose the best frame .. think of a new model with different (longer) usable frame
which happens to have the same (5504 pixels long) full raw frame .. then it will crash
if we define in camconst.json the correct frame. 

But if it is difficult you can just
a) comment out these two "raw_crop" lines from camconst.json 
OR b) comment out from camconst.json all the problematic items since dcraw9.21 has
now correct matrix and frame defined for these two ..

Reported by iliasgiarimis on 2014-06-05 13:06:08

Beep6581 commented 9 years ago
Thanks Ilias, I know where it crashes and try to find a solution to avoid the crashes
now.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-06-05 15:34:16

Beep6581 commented 9 years ago
This patch is a combination of patch from #133, Ilias' camconst.json from #135 and a
fix for the crashes mentioned in #138. Sony ILCE-3000 works fine now without changes
to camconst.json.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-06-05 15:59:44

Beep6581 commented 9 years ago
I forgot to include #136. Here's the new patch.

Reported by heckflosse@i-weyrich.de on 2014-06-05 16:13:22


Beep6581 commented 9 years ago
hmm .. I will test it on the weekend (I hope so ..).

Just have to mention that I think it would be good before commiting (although not an
urgent need ..) to copy from the patched camconst.json the BL and WL changes for Panasonics
at rawimage.cc also .. i.e. change the BL from around 143 to 15 and the WL (use the
lowest per ISO value and convert to hex .. 

I can make the corrections in the next few hours if needed .. 

Reported by iliasgiarimis on 2014-06-05 16:50:14

Beep6581 commented 9 years ago
Ilias, no need to hurry. Take the time you need to make the necessary changes to BL
and WL in rawimage.cc

I'll wait with the commit, no problem.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-06-05 17:00:34

Beep6581 commented 9 years ago
Sorry for being so late :(

I messed with checking Panasonic black frames to decide which exactly black offset
to use but it is not clear yet ..

I think there is no reason to delay the commit of dcraw921.patch any more, we can update
camconst.json in the next days ..

For the moment the only changes needed for the compatibility with dcraw921 are

- revert back to FZ150 instead of the suggested FX150 in #136  ..  was I blind ??.

- update camconst-dcraw921.json with Nikon D610 data that have already been commited
in the current camconst.json

change the panasonic constants in rawimage.cc as follows .. Sorry that I cannot build
the patch myself :(

        { "Olympus XZ-1", -1, -1, /* RT - Colin Walker */
          { 8665,-2247,-762,-2424,10372,2382,-1011,2286,5189 } },

   /* since Dcraw_v9.21 Panasonic BlackLevel is read from exif (tags 0x001c BlackLevelRed,
0x001d BlackLevelGreen, 0x001e BlackLevelBlue 
      and we define here the needed offset of around 15. The total BL is BL + BLoffset
(cblack + black)  */

        { "Panasonic DMC-FZ150", 15, 0xfd2,  /* RT */
          { 10435,-3208,-72,-2293,10506,2067,-486,1725,4682 } },
        { "Panasonic DMC-G10", 15, 0xf50, /* RT - Colin Walker - variable WL 3920 -
4080 */
          { 8310,-1811,-960,-4941,12990,2151,-1378,2468,6860 } },
        { "Panasonic DMC-G1", 15, 0xf50,  /* RT - Colin Walker - variable WL 3920 -
4080 */
          { 7477,-1615,-651,-5016,12769,2506,-1380,2475,7240 } },
        { "Panasonic DMC-G2", 15, 0xf50,  /* RT - Colin Walker - variable WL 3920 -
4080  */
          { 8310,-1811,-960,-4941,12990,2151,-1378,2468,6860 } },
        { "Panasonic DMC-G3", 15, 0xfdc,  /* RT - Colin Walker - WL 4060 */
          { 6051,-1406,-671,-4015,11505,2868,-1654,2667,6219 } },
        { "Panasonic DMC-G5", 15, 0xfdc,   /* RT - WL 4060 */
          { 7122,-2092,-419,-4643,11769,3283,-1363,2413,5944 } },
        { "Panasonic DMC-GF1", 15, 0xf50, /* RT - Colin Walker - Variable WL 3920 -
4080 */
          { 7863,-2080,-668,-4623,12331,2578,-1020,2066,7266 } },
        { "Panasonic DMC-GF2", 15, 0xfd2, /* RT - Colin Walker - WL 4050 */
          { 7694,-1791,-745,-4917,12818,2332,-1221,2322,7197 } },
        { "Panasonic DMC-GF3", 15, 0xfd2, /* RT - Colin Walker - WL 4050 */
          { 8074,-1846,-861,-5026,12999,2239,-1320,2375,7422 } },
        { "Panasonic DMC-GH1", 15, 0xf5a,  /* RT - Colin Walker - variable WL 3930
- 4080 */
          { 6360,-1557,-375,-4201,11504,3086,-1378,2518,5843 } },
        { "Panasonic DMC-GH2", 15, 0xf5a,  /* RT - Colin Walker - variable WL 3930
- 4080 */
//        { 6855,-1765,-456,-4223,11600,2996,-1450,2602,5761 } }, disabled due to problems
with underwater WB
          { 7780,-2410,-806,-3913,11724,2484,-1018,2390,5298 } }, // dcraw original

        { "Pentax K200D", -1, -1,  /* RT */
          { 10962,-4428,-542,-5486,13023,2748,-569,842,8390 } },

Reported by iliasgiarimis on 2014-06-17 00:23:46

Beep6581 commented 9 years ago
Ilias, thank you! I'll commit the modified dcraw921.patch today.

Reported by heckflosse@i-weyrich.de on 2014-06-17 09:54:56

Beep6581 commented 9 years ago
Update to dcraw 9.21 commited to revision da558096e08a

Reported by heckflosse@i-weyrich.de on 2014-06-17 20:56:45

Beep6581 commented 9 years ago
Meanwhile David released Dcraw v9.21 r1.464 2014/6/13 .. :)
http://www.cybercom.net/~dcoffin/dcraw/dcraw.c

No idea about what chages vs v1.463 ..

Reported by iliasgiarimis on 2014-06-23 13:29:59

Beep6581 commented 9 years ago
Dcraw v9.21 r 1.465 2014/06/28 is out .. :) 

Reported by iliasgiarimis on 2014-06-29 19:59:52

Beep6581 commented 9 years ago
I had a look at the differences between 1.463 and 1.465:

1.) some changes in cubic_spline() (used in phase_one_correct())
2.) some changes in colorcheck() not used by RT IIRC
3.) a small change in tiff_get (don't understand this cryptic code)
4.) a small change in parse_makernote (could be related to 3.)
5.) a change for Fuji HS10

Ingo

Reported by heckflosse@i-weyrich.de on 2014-06-29 21:11:34

Beep6581 commented 9 years ago
9.22 1.466 is out.

Reported by entertheyoni on 2014-06-30 09:38:24

Beep6581 commented 9 years ago
I had a look. Additional to #149, there are this changes:

1.) Corrections for Nikon 1 V3, Nikon 1 J4 and Nikon 1 S2
2.) Panasonic DMC-GH4, Sony ILCA-77M2 and Sony ILCE-7S added

Ingo

Reported by heckflosse@i-weyrich.de on 2014-06-30 12:04:58

Beep6581 commented 9 years ago
I'll make the 9.22 1.467 patch at the weekend

Reported by heckflosse@i-weyrich.de on 2014-07-04 12:56:58