Bug 25368 - Apply colour matrices in the pixbuf loader.
Summary: Apply colour matrices in the pixbuf loader.
Status: RESOLVED MOVED
Alias: None
Product: libopenraw
Classification: Unclassified
Component: General (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Hubert Figuiere
QA Contact:
URL: https://bugzilla.gnome.org/show_bug.c...
Whiteboard:
Keywords:
Depends on: 87996
Blocks:
  Show dependency treegraph
 
Reported: 2009-11-30 13:45 UTC by Priit Laes (irc: plaes)
Modified: 2018-08-20 21:24 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
ufraw-vs-eog.png (692.34 KB, image/png)
2009-11-30 13:45 UTC, Priit Laes (irc: plaes)
Details

Description Priit Laes (irc: plaes) 2009-11-30 13:45:19 UTC
Created attachment 31609 [details]
ufraw-vs-eog.png

When viewing RAW CR2 (Canon) files with eog, the colours are wrong.

Eog guys say it has something todo with libopenraw's gdkpixbuf handling.

Sample image: http://plaes.org/files/2009-Q4/IMG_8535.CR2

And I have attached a comparison as an attachment.
Comment 1 Hubert Figuiere 2009-12-18 20:46:34 UTC
This feature is experimental, completely.
Comment 2 Priit Laes (irc: plaes) 2009-12-21 02:14:37 UTC
(In reply to comment #1)
> This feature is experimental, completely.


Experimental or not, but it is a regression as it used to work a while ago.
Comment 3 Hubert Figuiere 2009-12-21 08:16:06 UTC
how can it regress since the code hasn't changed ?
Comment 4 Priit Laes (irc: plaes) 2009-12-21 09:01:45 UTC
(In reply to comment #3)
> how can it regress since the code hasn't changed ?
> 

Hmm.. nice.
The issue might be in Eog then.. (although they sent me here..)

Anyway, I'll do some bisecting...
Comment 5 Priit Laes (irc: plaes) 2009-12-21 09:28:37 UTC
Now I have actually no idea which component is faulty.

The libopenraw backend **used** to work because I fixed this big memleak a while ago... but now when testing with various different eog versions I still see the same too-green picture :S
Comment 6 Hubert Figuiere 2010-03-05 22:12:41 UTC
Are you sure it was using libopenraw via gdkpixbuf for the rendering?
Comment 7 Gary Ching-Pang Lin 2011-06-15 00:16:43 UTC
I tried pixbufload.c in demo/, and the output image is green. However, the thumbnail generated by gdk.c is normal.
Comment 8 Hubert Figuiere 2011-06-15 00:31:26 UTC
(In reply to comment #7)
> I tried pixbufload.c in demo/, and the output image is green. However, the
> thumbnail generated by gdk.c is normal.

That's because thumbnails are extracted from the file.
Comment 9 Gary Ching-Pang Lin 2011-06-15 00:44:48 UTC
(In reply to comment #8)
> (In reply to comment #7)
> > I tried pixbufload.c in demo/, and the output image is green. However, the
> > thumbnail generated by gdk.c is normal.
> 
> That's because thumbnails are extracted from the file.

Is the greenish image produced by pixbufload.c expected?

Actually, I bumped into the similar issue with nautilus. I updated libopenraw from 0.0.8 to git master to fix nautilus crash when it was loading ORF files. The update stopped the crash, but the thumbnails in nautilus is green. I wonder whether the wrong color issue related to libopenraw or not.
Comment 10 Hubert Figuiere 2011-06-15 00:52:01 UTC
Yes it is expected. The white balance coefficient are not applied to the RAW decoding.

This is on the TODO list.
Comment 11 Gary Ching-Pang Lin 2011-06-15 00:58:01 UTC
Ok, Thanks! Then I'll wait :-)
Comment 12 Mark Hahnenberg 2015-01-02 18:00:08 UTC
I'm running into this issue too, and it seems like this bug has been open for a while, so I thought I'd take a stab at it. In particular, I can view my images using the gdk pixbuf loader, but they appear as grayscale with random green and magenta streaks scattered around the image.

I googled around and found a couple bugs (including this one) related to applying white balance and colour matrices after demosaicing. I've looked around in the libopenraw code and found the TODO in rawdata.cpp for applying colour matrices after demosaicing. I've tried modifying the code in a couple of ways for my particular camera model (Canon 600D) in hopes of being able to generalize any solution I found, but I can't seem to get it to work. 

First I tried using the colour matrix returned from getColourMatrix1. This didn't return anything (i.e. matrixSize was 0). It's not exactly clear why there are two sets of colour matrix functions, and I don't see any calls to either setColourMatrix* or getColourMatrix* anywhere in the code.

Then I tried hardcoding the matrix constants from the BuiltinColourMatrix for my particular camera model and scaling them by 1/10000 as a comment suggested, but this didn't produce an improved output.

Then I tried hardcoding the colour matrix for my camer model I found here: http://www.dxomark.com/Cameras/Canon/EOS-600D---Measurements. This also didn't work.

Then I tried just applying the white balance coefficients for my camera model also found at dxomark.com. This also didn't work.

I also tried looking at the dcraw source to see how colour matrices are applied there, but that code isn't very pleasant to read, so I gave up on that.

I'm using the pixbufload demo to test the various changes I've made.

I've also read through both http://www.odelama.com/photo/Developing-a-RAW-Photo-by-hand and http://www.odelama.com/photo/Developing-a-RAW-Photo-by-hand/Developing-a-RAW-Photo-by-hand_Part-2/ to try to help my understanding of what I needed to add/change. I learned a lot from those two links, but I'm still no closer to getting this working :-)

I'd be interested in fixing this, but it seems like I'm kind of stuck. Any help getting unstuck would be appreciated :-)
Comment 13 Hubert Figuiere 2015-01-02 20:33:45 UTC
(In reply to Mark Hahnenberg from comment #12)
> I'm running into this issue too, and it seems like this bug has been open
> for a while, so I thought I'd take a stab at it. In particular, I can view
> my images using the gdk pixbuf loader, but they appear as grayscale with
> random green and magenta streaks scattered around the image.
> 
> I googled around and found a couple bugs (including this one) related to
> applying white balance and colour matrices after demosaicing. I've looked
> around in the libopenraw code and found the TODO in rawdata.cpp for applying
> colour matrices after demosaicing. I've tried modifying the code in a couple
> of ways for my particular camera model (Canon 600D) in hopes of being able
> to generalize any solution I found, but I can't seem to get it to work. 
> 
> First I tried using the colour matrix returned from getColourMatrix1. This
> didn't return anything (i.e. matrixSize was 0). It's not exactly clear why
> there are two sets of colour matrix functions, and I don't see any calls to
> either setColourMatrix* or getColourMatrix* anywhere in the code.

I'll assume that you have libopenraw from trunk.

First you should check that the color matrix is properly set for your camera. There is "ordiag" in the tools subdir that is built when building libopenraw, that will display info. Here it works on the sample 600D file I do have.

As for dcraw, what you are looking for is probably cam_xyz_coeff()
Comment 14 Hubert Figuiere 2015-01-02 20:34:09 UTC
by "trunk" above I meant git "master".
Comment 15 Mark Hahnenberg 2015-01-03 14:11:48 UTC
> I'll assume that you have libopenraw from trunk.

Yep!
 
> First you should check that the color matrix is properly set for your
> camera. There is "ordiag" in the tools subdir that is built when building
> libopenraw, that will display info. Here it works on the sample 600D file I
> do have.

ordiag reports that it found one color matrix for the file I've been testing on. Looking at the code, it seems like the RawFile correctly detects and fills in the BuiltinColourMatrix for my camera model, but I guess that info doesn't make it into the RawData's colourMatrix. I added some code to fill in RawData's colourMatrix inside Cr2File::_getRawData, so now RawData::getRenderedImage has a properly filled colourMatrix to work with.
 
> As for dcraw, what you are looking for is probably cam_xyz_coeff()

Okay, I'll give that a look.
Comment 16 Hubert Figuiere 2015-01-03 15:50:02 UTC
(In reply to Mark Hahnenberg from comment #15)

> ordiag reports that it found one color matrix for the file I've been testing
> on. Looking at the code, it seems like the RawFile correctly detects and
> fills in the BuiltinColourMatrix for my camera model, but I guess that info
> doesn't make it into the RawData's colourMatrix. I added some code to fill
> in RawData's colourMatrix inside Cr2File::_getRawData, so now
> RawData::getRenderedImage has a properly filled colourMatrix to work with.


Oh. Definitely a bug. I never fill the colourMatrix in the raw data. My bad.

Filed bug 87996
Comment 17 Mark Hahnenberg 2015-01-03 17:17:20 UTC
> > As for dcraw, what you are looking for is probably cam_xyz_coeff()
> 
> Okay, I'll give that a look.

I looked at the dcraw source for cam_xyz_coeff, and as far as I can tell it looks like a function that converts the colour matrix for the XYZ colour space to the colour matrix for the RGB colour space. So I tried moving it over into rawdata.cpp along with its dependencies. I used it to convert the RawData's colourMatrix in RawData::getRenderedImage, and then I applied the resulting rgb_cam matrix to the BitmapData's dst buffer. Unfortunately this didn't work.

Maybe my next step should be to just step through dcraw in a debugger to see exactly how it uses the rgb_cam matrix since it seems like the naive matrix multiplication of the bitmap data didn't quite work.
Comment 18 Mark Hahnenberg 2015-01-05 17:37:07 UTC
Okay, after some more investigation, I think I found another easy-to-fix issue in bimedian_demosaic. At the end of each iteration, the red, green, and blue values are divided by 16.0 prior to being stored into the dst_buf. I'm not 100% sure why, but I think it's in order to scale the 16-bit values (which are currently doubles) into the 8-bit range for the uint8_t dst array.

The max value in the src array is the max value that the camera sensors could record. Then we need to scale it down into 8 bits for the dst. I think that scaling constant needs to be dependent on the model of camera and how many bits it records internally. For example, my camera has a 14-bit ADC, so the max value is 2^14 rather than 2^16. Thus, dividing by 16.0 wasn't enough to always get every RGB value into the 2^8 range, so some of my pictures came out with saturated RGB channels which caused weird artifacts on the image.

So as a fix for this issue I've been scaling by 1.0/64.0 instead of 1.0/16.0. But there are other camera models with, e.g., 12 bits, and I'm sure there are/will be others with different numbers of bits as well.
Comment 19 Mark Hahnenberg 2015-01-05 17:46:54 UTC
It also might make sense to do the down-scaling to 8-bit after the demosaic instead of combining them into the same step. That would allow us to gamma correction, color matrix correction, etc. on 16-bit values before converting to 8-bit if necessary.
Comment 20 GitLab Migration User 2018-08-20 21:24:24 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/libopenraw/libopenraw/issues/3.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.