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

Xtrans support #2398

Closed Beep6581 closed 8 years ago

Beep6581 commented 8 years ago

Originally reported on Google Code with ID 2415

Today I started work at Xtrans support. This first patch only eliminates an endless
loop when trying to open a Xtrans file and a crash when trying to fast demosaic. I
set demosaic to 'nodemosaic' for Xtrans files until I get the xtrans_interpolate from
dcraw working in RT.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-06-08 16:31:46


Beep6581 commented 8 years ago
Here's the first patch which includes xtrans_interpolate. I used the code from dcraw
and made an omp version. It needs about one second for a 1-pass xtrans_interpolate
of a 16 Mp file. This patch is far from being complete. A lot of problems have to be
solved. Actually the colors are a bit off, there is no thumbnail from raw, raw histogram
should also be wrong etc...
But at least we can demosaic these files now.

Any reviews are very welcome

Ingo

Reported by heckflosse@i-weyrich.de on 2014-06-10 13:39:14


Beep6581 commented 8 years ago
Almost same patch as last one, but I formatted the code of xtrans_interpolate and added
progress bar functionality. It's also a bit faster. Didn't solve any problem of patch
from #1.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-06-10 18:32:23


Beep6581 commented 8 years ago
I have a FujiFilm XT-1, I'll try and compile RT on linux and apply this patch. Thanks
a lot!

Reported by sguyader on 2014-06-13 14:26:54

Beep6581 commented 8 years ago
Any tests are welcome :-) Actually this is at a very early stage. Colors are still wrong
and a lot of other things need to be solved, so don't expect anything else than a hopefully
correct xtrans interpolation.
I'll make a new patch, when I solved the problem with the wrong colors.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-06-13 14:36:51

Beep6581 commented 8 years ago
Indeed the colors are wrong but it looks like the xtrans interpolation works as expected.

Reported by sguyader on 2014-06-13 18:02:42

Beep6581 commented 8 years ago
Just for information:

Here's a list of the todos based on my last patch:

Priority 1: Use data-array instead of image-array. I used image array in last patch
to simplify porting the xtrans_interpolate function. This is already done in my local
repository, which also includes a little speedup (3-pass xtrans-interpolate now takes
about 2 seconds at my system for a 16 Mp xtrans file)

Priority 2: correct the wrong colors caused by wrong black and white levels and multipliers.

Priority 3: use a simpler and faster demosaic method to get the thumbs from the raw
files (actually the embedded thumbs are used). There is a methof in dcraw, but I didn't
try it yet.

Priority 4: Spot-WB actually only works for Bayer raw files, have to make some changes
to get it working with xtrans files.

Priority 5: RawHistogramm actually only works for Bayer raw files, have to make some
changes to get it working with xtrans files.

Priority 6: Disable the pre demosaic functions, which are not compatible with x-trans
layout (that's easy)

Priority 7: Make xtrans versions for the pre demosaic functions, which are not compatible
with x-trans layout, if possible (could be possible for badpixels, flatfield and dark
frame).

Priority 8: Add a GUI-control for the number of xtrans-passes (last patch made only
one pass, next patch will make three, as in default of dcraw)

To be continued...

Ingo

Reported by heckflosse@i-weyrich.de on 2014-06-13 22:54:31

Beep6581 commented 8 years ago
Next one. Colors are much better now. I don't think that they are really correct, but
they match the thumb when neutral profile is applied and exposure is increased a bit
much better than before. For really good colors, Ilias should have a look :-)

I had to change the black levels for x-trans cams in dcraw.cc, because for an unknown
reason, I don't get the correct ones automatically, whereas dcraw.exe shows the correct
ones in verbose mode.

Actually I changed the entry for the X-Pro1 and X20 and generated an entry for X-T1.
The entry I generated for the X-T1 is a copy of the X-Pro1 entry with changed black
and white levels.

For other cams with X-Trans sensor this corrections have to be done too, but I've only
test images from the three cams mentioned above.

The other todos from #6 are still open.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-06-14 12:34:22


Beep6581 commented 8 years ago
Another todo:

Priority 4: Auto White Balance

Reported by heckflosse@i-weyrich.de on 2014-06-14 18:14:47

Beep6581 commented 8 years ago
Another todo:

Priority 3: Exposure Auto Levels

Reported by heckflosse@i-weyrich.de on 2014-06-14 18:44:02

Beep6581 commented 8 years ago
Ingo, I will be at home the next 2 days so I will be able to test :)

Reported by iliasgiarimis on 2014-06-15 08:33:45

Beep6581 commented 8 years ago
Ilias, great, thank you :-)

Reported by heckflosse@i-weyrich.de on 2014-06-15 09:58:10

Beep6581 commented 8 years ago
Next one. Almost same as last patch. Only changed black and white levels for X-E1, X-E2,
X-M1 and XQ1

Ingo

Reported by heckflosse@i-weyrich.de on 2014-06-15 10:37:12


Beep6581 commented 8 years ago
Ingo, I have some corrections for the dcraw921.patch regarding camconst.json and rawimage.cc
.. I'll upload them in a few hours .. then 

Isn't it better to commit the dcraw921.patch and build the X-trans patch upon it ??

I am thinking of the changes regarding BL calculation in dcraw921 .. :)

Reported by iliasgiarimis on 2014-06-15 13:32:48

Beep6581 commented 8 years ago
4 todos solved:

Border Interpolation
Spot White Balance
Auto White Balance
Exposure Auto Levels

Reported by heckflosse@i-weyrich.de on 2014-06-15 13:34:46


Beep6581 commented 8 years ago
Ilias, of course. As soon as I have your corrections I'll commit the dcraw921.patch
and make a new xtrans patch.

Reported by heckflosse@i-weyrich.de on 2014-06-15 13:40:03

Beep6581 commented 8 years ago
This patch adds a very simple fast xtrans interploation, which now is used at zoom levels
less than 100%. Needs about 100 ms at my system for a 16 MP xtrans file.

Reported by heckflosse@i-weyrich.de on 2014-06-15 20:07:30

Beep6581 commented 8 years ago
Just figured out that there is a better way to implement all this xtrans stuff. My actual
implementation uses 4*4*width*height bytes to store the rawData, only because the xtrans_interpolate
from dcraw uses channel 1 and 3 to store intermediate green limits for red and blue
pixels. I'll try to make an implementation, which uses only 3*4*width*height bytes
in xtrans_interpolate and only 4*width*height bytes elsewhere. This way the peak memory
consumption can be reduced, the code can be simplified and that also could lead to
a speedup.

Will post a new patch tomorrow.

Ingo

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

Beep6581 commented 8 years ago
Correction to last post: Memory consumption in xtrans_interpolate can at least be reduced
to (4+2)*width*height bytes = 6*width*height bytes (instead of 16*width*height bytes
it actually needs and 12*width*height bytes I mentioned in #17). Will have a look,
whether it can be further reduced by using tiled processing for this step....

Ingo

Reported by heckflosse@i-weyrich.de on 2014-06-16 00:29:19

Beep6581 commented 8 years ago
xtrans_06.patch had a bug (wrong placed bracket). Corrected with this one

Reported by heckflosse@i-weyrich.de on 2014-06-16 12:50:51


Beep6581 commented 8 years ago
Something is wrong in latest patch with XT-1. Other cams work fine. Have to figure that
out...

Reported by heckflosse@i-weyrich.de on 2014-06-16 15:37:55

Beep6581 commented 8 years ago
You were quicker than me. Here's a screenshot that shows the problem with the latest
patch. xtrans_05.patch is fine. Regarding the colors, it's a matter of profile I think,
if I point RT to use an XT-1 generic ICC profile extracted from Adobe Lightroom, the
colors are better. Maybe using the profile values from dcraw 9.21 rev 1.463 will do.

Reported by sguyader on 2014-06-16 16:37:45


Beep6581 commented 8 years ago
Ok, I just tried with the profile values from dcraw 9.21, and the colors are definitely
better. These values are the same for the X-E2. X-Pro1 has a different sensor (it was
first generation of x-trans) whereas X-E2 and X-T1 have and share the second generation
sensor. Please use the following values :
{ "Fujifilm X-T1", 1024, 15872,
    { 8458,-2451,-855,-4597,12447,2407,-1475,2482,6526 } },
{ "Fujifilm X-E2", 1024, 15872,
    { 8458,-2451,-855,-4597,12447,2407,-1475,2482,6526 } },

Reported by sguyader on 2014-06-16 16:58:23

Beep6581 commented 8 years ago
Ok, thanks a lot. I'll post a new corrected patch soon.

Reported by heckflosse@i-weyrich.de on 2014-06-16 17:01:05

Beep6581 commented 8 years ago
Ok, here's a new patch. X-T1 works again. The colors for X-T1 are much better now with
the values from #23 :-)
I also made a hack to allow you to compare 1-pass vs 3-pass vs fast xtrans_interpolate:

At zoom levels < 100 % fast is used

At zoom levels >= 100%

fast is used when fast is selected
3-pass is used then amaze is selected
1-pass is used when anything else is selected

Ingo

Reported by heckflosse@i-weyrich.de on 2014-06-16 17:50:50


Beep6581 commented 8 years ago
So far so good Ingo, the fast interpolation for <100% really speeds up the workflow.

Reported by sguyader on 2014-06-16 18:34:51

Beep6581 commented 8 years ago
Re #26: I'll try to speed up the xtrans_interpolations a bit with next patches. Bases
on my experiences with other speedups, I think, that factor 2 could be possible.

Re #27: There is no explicit median (which you can activate yourself) in RT, though
there are several parts in RT where a median is used, so it is no problem to make one
for xtrans too. Do you use 5 passes of median or 5 passes of x-trans interpolation
in dcraw?

Ingo

Reported by heckflosse@i-weyrich.de on 2014-06-16 18:57:54

Beep6581 commented 8 years ago
Just saw, that #27 is deleted...

Reported by heckflosse@i-weyrich.de on 2014-06-16 19:00:16

Beep6581 commented 8 years ago
Sorry, I deleted #27 because I found a reference stating that the 3false Color suppression
Steps" slider under the Demosaicing Method is a 5-pass median filter. However it doesn't
do anything yet for xtrans, apparently. If it is possible to make it work for xtrans,
it would be a good thing as it is more prone to color artifacts.

I use 3-pass xtrans interpolation, and in dcraw a 5-pass median filter.

Reported by sguyader on 2014-06-16 19:19:55

Beep6581 commented 8 years ago
I'll activate false color suppression for xtrans in next patch. To test before patch,
just change line 433 of rtengine/rawimagesource.cc to this ;-)

    if ((ri->isBayer() || ri->isXtrans()) && pp.skip==1)

Reported by heckflosse@i-weyrich.de on 2014-06-16 19:38:03

Beep6581 commented 8 years ago
Thanks a lot, it works now.

Reported by sguyader on 2014-06-16 19:56:30

Beep6581 commented 8 years ago
Great :-) There are a lot of open points with xtrans files in RT. It would be a great
help, if you could report all still present problems with xtrans files in RT in this
Issue. I know, there are a lot of, but don't think, that I know all; and 4 eyes see
more than two :-) At least, a not so bad first step is done now.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-06-16 20:29:41

Beep6581 commented 8 years ago
Made a better fast xtrans interpolate for zoom levels < 100%. Speed is the same as the
one from xtrans_08.patch. Will include it in next patch.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-06-16 23:39:06

Beep6581 commented 8 years ago
Re #33, I honestly think it is already more than usable. In terms of image quality,
I get almost the same output as I get from straight dcraw. I only had the initial impression
that the output was slightly sharper in RT (a good thing) but that it also emphasized
the color artifacts (a bad thing). In particular the use of default sharpening in RT
really brought up the color artifacts to an undesirable level. But with the false color
removal it should be all good. I'll check that again and let you know.

Sebastien

Reported by sguyader on 2014-06-17 11:38:49

Beep6581 commented 8 years ago
Sebastien, please use http://filebin.net/ to upload large files. Space on GC is limited.
Can you upload the reference X-T1 image please?

Ingo

Reported by heckflosse@i-weyrich.de on 2014-06-17 13:10:33

Beep6581 commented 8 years ago
Unfortunately 'Delete' doesn't remove the attachments, it only marks them as deleted
and they still need space...

Reported by heckflosse@i-weyrich.de on 2014-06-17 13:25:28

Beep6581 commented 8 years ago
Sorry, I uploaded the files on my Google drive (I hope it's fine):

The two image crops:
https://drive.google.com/file/d/0B_AvPFlUj8t5SFJQQ2FCZXJLUGc/edit?usp=sharing
https://drive.google.com/file/d/0B_AvPFlUj8t5eHU0NmNSVUpxSkk/edit?usp=sharing

The reference file (iso 200 raw, also downloadable from www.dpreview.com):
https://drive.google.com/file/d/0B_AvPFlUj8t5a1Y4N2g1RzNCWFk/edit?usp=sharing

Reported by sguyader on 2014-06-17 13:26:19

Beep6581 commented 8 years ago
Thanks a lot. Which command line options did you use for dcraw?

Reported by heckflosse@i-weyrich.de on 2014-06-17 13:32:32

Beep6581 commented 8 years ago
I can reproduce it here. I'll have a look.

Reported by heckflosse@i-weyrich.de on 2014-06-17 13:37:49

Beep6581 commented 8 years ago
My dcraw options: -v -w -W -k 1024 -S 15872 -q 3 -T
I just realized though that the levels in RT were not set to zero (there was some positive
exposure and contrast), so the artifacts are amplified. But they are still there.

Reported by sguyader on 2014-06-17 13:38:45

Beep6581 commented 8 years ago
One last crop, comparing RT after 5 passes of "false color suppression" to dcraw after
5 passes of median filtering:
https://drive.google.com/file/d/0B_AvPFlUj8t5eFJXRHFkSXdlckk/edit?usp=sharing

False color suppression doesn't help here on the MTF chart lines (although the cyan
cast gets mostly removed in the bottom right corner of the test chart, not shown here)...

I have to say that for normal images, everything looks fine, it's more when there's
a strong contrast along edges that these artifacts show up.

Reported by sguyader on 2014-06-17 13:56:51

Beep6581 commented 8 years ago
Sebastien, the cyan cast is, at least partly, caused by using the values from #23. If
you use the values from patch 03, the cyan cast is gone.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-06-17 18:44:31

Beep6581 commented 8 years ago
Here's an actual patch for latest RT revision (dcraw 9.21 committed). It's almost the
same as xtrans_08.patch but adds the change from #31.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-06-17 21:23:30


Beep6581 commented 8 years ago
Issue 1899 has been merged into this issue.

Reported by heckflosse@i-weyrich.de on 2014-06-17 23:48:41

Beep6581 commented 8 years ago
Seems to me that the dcraw values for X100S work better with XT-1 than the values for
X-E2

Ingo

Reported by heckflosse@i-weyrich.de on 2014-06-18 00:10:12

Beep6581 commented 8 years ago
I tried with these values:

    { "Fujifilm X-T1", 1024, 15872,
    { 10592,-4262,-1008,-3514,11355,2465,-870,2025,6386 } },

Still not optimal...

Reported by heckflosse@i-weyrich.de on 2014-06-18 00:11:51

Beep6581 commented 8 years ago
Don't know if it i suseful, but using Rawdigger I found that the maximum level in clipped
highlights is usually 16382-16383 for R and G channels, and 16315-16383 for B channel.
Maybe we could preserve some highlights by changing from 15872 to 16300?

Reported by sguyader on 2014-06-18 12:54:11

Beep6581 commented 8 years ago
Issue 2356 has been merged into this issue.

Reported by heckflosse@i-weyrich.de on 2014-06-18 15:44:26

Beep6581 commented 8 years ago
I don't know if this may help, but xtrans demosaicing works in a dev version of Darktable
from here: https://github.com/dtorop/darktable.
I can't post an image because file export makes a segfault...

Reported by sguyader on 2014-06-19 15:59:34

Beep6581 commented 8 years ago
Thanks a lot. I'll have a look. At least they use different values than dcraw for whitelevel.
See this link and search for 'x-t1'. https://github.com/dtorop/darktable/blob/xtrans2/src/external/rawspeed/data/cameras.xml

Almost the value you found out using Rawdigger :-)

Ingo

Reported by heckflosse@i-weyrich.de on 2014-06-19 17:34:42

Beep6581 commented 8 years ago
The enhanced comments in darktable xtrans code are really worthful :-)

Reported by heckflosse@i-weyrich.de on 2014-06-19 17:48:35