Expand camconst.json to define raw image borders

[Beep6581]

Originally reported on Google Code with ID 2139

Defining raw picture borders are missing from camconst.json to be complete.

I can find at least one model for which the borders are improperly defined by Dcraw
(Sony NEX5N)

One of the great effects of using an external file for camera constants is that we
can have easy support for new models but many times we would need to define borders
also (Sony A7R is a recent example). 
Given that during last years Dcraw updates come in twice a year when Adobe's official
DNG converter updates come every 3 months (or less if we count RC1 releases) there
is a big gap between a model's launch and Dcraw support which we can easily cover by
using the ready DNG data in camconst.json .

Anders, sorry .. couldn't resist ..http://code.google.com/p/rawtherapee/issues/detail?id=2032#c71
:-) :-)

Reported by iliasgiarimis on 2013-12-18 10:51:40

[Beep6581]

I've not worked with raw border issues before so I'm a bit new to this. I'm assuming
what you're requesting parameters for the crop_masked_pixels() function, ie that the
raw image is larger than the usable area, and it should be possible to define how much
of the borders that should be removed?

Ie in dcraw it would be the "top_margin", "left_margin", plus image "height" and "width"
that needs adjustment.

Do you have a suggestion of how borders would be defined in the format?

Simply like this perhaps:

        "crop_mask": [ 20, 30, 4000, 3000 ]

meaning that 20 pixels is removed from the left side, 30 from the top and image width
height is 4000 x 3000.

I'll start implementing like this and let me know if it's ok.

Reported by torger@ludd.ltu.se on 2013-12-18 11:11:25

• Status changed: Accepted

[Beep6581]

I'm renaming crop_mask to raw_crop

Reported by torger@ludd.ltu.se on 2013-12-18 11:12:47

[Beep6581]

Yes, top, left, height, width is fine and consistent with DNG notation I think.

Ddraw uses various ways for example on Nikon cameras defines right border ..

Reported by iliasgiarimis on 2013-12-18 11:25:58

[Beep6581]

I've implemented that now in a patch, but dcraw seems to also have a more cryptic "mask[][]"
matrix which is used in addition to the margin and width height parameters.

Reported by torger@ludd.ltu.se on 2013-12-18 11:36:46

[Beep6581]

Here's the patch... suspect that we need to consider the mask[][] matrix too though.
I just don't know what it's supposed to contain.

Reported by torger@ludd.ltu.se on 2013-12-18 11:38:36

---

• _Attachment: raw_crop-patch.txt_

[Beep6581]

Mask seems to be the MaskedAreas parameter in DNG, used for measuring black level. Should
we add that too?

"masked_areas": [ 0, 0, 0, 0, 1,1,1,1, 2,2,2,2, ... ]

Reported by torger@ludd.ltu.se on 2013-12-18 11:44:32

[Beep6581]

fixed a bug, now it should work.

Is this enough to be able to add support for new cameras early?

Reported by torger@ludd.ltu.se on 2013-12-18 12:00:49

---

• _Attachment: raw_crop-patch-1.txt_

[Beep6581]

Reported by torger@ludd.ltu.se on 2013-12-18 12:01:02

• Status changed: PatchSubmitted

[Beep6581]

This gets complex .. we need an expert ..

As I understand it the masked areas are used only for Canon. And I recall when 1Dx
files first appeared Dcraw rendered a negative (with almost correct BL) but it was
the pixel's order different (or just the starting column shifted by 1 ?).

Defining the areas for measuring is different from defining where to crop. Usually
the border to crop is larger than the area to measure. It should be somehow automated
in Dcraw.c to use a crop of the masked pixels (leave some borders around).

Reported by iliasgiarimis on 2013-12-18 12:09:45

[Beep6581]

I'll add masked_areas, we'll see if it gets used or not.

I do it based on what stuff that gets used in the crop_masked_pixels() function, it
uses the raw crop and the masked areas. The camconst.json parameters is then fetched
just before calling the function.

Reported by torger@ludd.ltu.se on 2013-12-18 12:19:59

[Beep6581]

# 7, Sorry I cannot test non compiled features .. :-)

For all manufacturers (except for some Canon with first seen layout*) this should be
fine. 

* When Dcraw heuristics don't apply. 

Reported by iliasgiarimis on 2013-12-18 12:21:12

[Beep6581]

Now we have masked_areas too.

I guess the patch is "ready to commit" now...

Reported by torger@ludd.ltu.se on 2013-12-18 12:32:31

---

• _Attachment: raw_crop-patch-2.txt_

[Beep6581]

Reported by torger@ludd.ltu.se on 2013-12-18 12:33:39

• Blocking: #2016

[Beep6581]

Do you have a link to a raw file which has bad borders with current RawTherapee and
the proper values for them? Then I can make a quick test if this patch works, if it
works I'll commit.

Reported by torger@ludd.ltu.se on 2013-12-18 12:35:53

[Beep6581]

That's how RawSpeed organises it's external definition file

http://rawstudio.org/blog/?p=816

Anders, a bit off topic ..
Is it possible to change a bit the camconst.json format to make it more compact ?.
I mean we have to write many times the same WL while we could use 
ISO 100, 125, 200, 250, ... WL= XXXX

Reported by iliasgiarimis on 2013-12-18 12:37:50

[Beep6581]

We will with this support all rawspeed's features except CFA layout, but that I guess
is rarely wrong out of dcraw.

Compactness: you mean like this?

                { "iso": [ 100, 125, 200, 300 ], "levels": 15750 },

Reported by torger@ludd.ltu.se on 2013-12-18 12:43:54

[Beep6581]

Now I've added the possibility to group isos together like in #16

Reported by torger@ludd.ltu.se on 2013-12-18 12:51:36

---

• _Attachment: raw_crop-patch-3.txt_

[Beep6581]

# 14,
NEX-5N, Supported but the right 10 columns contain garbage. Dcraw does not even DNG's
borders.
http://www.imaging-resource.com/PRODS/NEX5N/NEX5NhSLI00100_NR_LOW.ARW.HTM

A7R, New Model, preliminary support by camconst.json http://filebin.net/8nmymge2a1/,
right 16 columns.
http://www.imaging-resource.com/camera-reviews/sony/a7r/AA7RhSLI00100NR0.ARW.HTM

Reported by iliasgiarimis on 2013-12-18 12:53:01

[Beep6581]

Anders, an off topic question (to not open an new issue).

I read in RT's log that all four BL are read. The question is "does RT use this info
in every instace it should ?. 
I ask because one year ago I had a file of Olympus EM-5 where the BLs (in exif maker
data) were something like 252, 263, 257, 260 and RT rendered a very problematic image
at the dark regions but after correcting with RT's BL sliders (base was 256 sliders
at -4, +7, +1, +4) it got much better.

If for investigating this, I put in camconst.json a model and it's black levels, which
levels will be used, these from camconst or those from file metadata ?.

Reported by iliasgiarimis on 2013-12-18 13:13:42

[Beep6581]

camconst.json should override black levels.

Reported by torger@ludd.ltu.se on 2013-12-18 13:23:56

[Beep6581]

this for a7r seems to work:

    "raw_crop": [ 0, 0, -16, 0 ]

committed to default.

Reported by torger@ludd.ltu.se on 2013-12-18 13:28:37

• Status changed: FixedPendingConfirmation

[Beep6581]

Many thanks.

Reported by iliasgiarimis on 2013-12-18 13:28:52

[Beep6581]

Andres, how do you make patches like these in less than 3 hours!:)

Reported by michaelezra000 on 2013-12-18 14:04:04

[Beep6581]

 In camconst.json   
               "+   // crop away masked sensor borders, 10 pixels left, 20 pixels top,
resulting image width/height 4000x3000
    66  +   // instead of width/height you can write a negative number which specifies
how much of right/bottom border
    67  +   // that should be removed
    68  +   "raw_crop": [ 10, 20, 4000, 3000 ],
    69  +   // same as MaskedAreas DNG tag, used for black level measuring here two areas
defined
    70  +   "masked_areas": [ 51, 2, 3804, 156, 51, 5794, 3804, 5792 ],"

it is a bit unclear the meaning and the order of "masked areas" numbers. It is different
than "raw crop" which is well defined.
After trial-error http://rawtherapee.com/forum/viewtopic.php?f=9&t=5222&p=35318#p35318
my understanding for the first 4 numbers is that 51 is the top border, 2 is the left
3804 the height and 156 the width of the first measured area.
Now the second set ... I don't understand... 

Reported by iliasgiarimis on 2013-12-23 06:24:08

[Beep6581]

it's the same as MaskedAreas DNG tag, which is documented in the DNG spec:

http://www.adobe.com/content/dam/Adobe/en/products/photoshop/pdfs/dng_spec_1.4.0.0.pdf

Quote:

"This tag contains a list of non-overlapping rectangle coordinates of fully masked
pixels, which can be optionally used by DNG readers to measure the black encoding level.

The order of each rectangle's coordinates is: top, left, bottom, right."

Ie you can have a list of areas/rectangles, the example shows two areas.

Reported by torger@ludd.ltu.se on 2013-12-23 19:08:56

[Beep6581]

So all the numbers are absolute coordinates not relative distances ?. I understand these
as distance of the sides of the quadrilateral measuring areas from the top and left
sides of the sensor then. But 51,5794,3804,5792 cannot be top, left, bottomn, right
..correct ?. 

Anyway, it would be good to be clearly stated in the text.

Reported by iliasgiarimis on 2013-12-23 21:07:26

[Beep6581]

It's the exact fromatting dcraw uses and it uses the same as DNG. So I just pass it
through, I know no more than you do.

Reported by torger@ludd.ltu.se on 2013-12-23 22:03:33

[Beep6581]

The format and the calculations on it are OK.

my point is that we should add in the explaining text a clear definition about what
exactly the values in "MaskedAreas" brackets mean.

" 70        "masked_areas": [ 51, 2, 3804, 156, 51, 5792, 3804, 5794 ],"  
(here I inverted the order of 5792 & 5794 because the left side cannot be farther from
sensor's left limit than the right side ..)

  71    +       // these are tetrads defining the rectangular areas where dcraw measures
BlackLevel. There can be more than one as in
  72    +       // the example above. In "maskedAreas" the values resemble the distance
of each MaskedArea's side from the sensor top or left side.
  73    +       // the first is the distance of the MaskedArea's top side from sensors
top
  74    +       // the second is the distance of MaskedArea's left side from sensor's
left
  75    +       // the third value is the distance of MaskedArea's bottom side from
sensor's top
  76    +       // the fourth value is the distance of maskedArea's right side from
sensor's left 

Merry Christmas :)   

Reported by iliasgiarimis on 2013-12-24 16:56:34

[Beep6581]

Thanks... merry christmas to you too! I'm totally stuffed with christmas food. I'll
add in the text after some rest, thanks.

Reported by torger@ludd.ltu.se on 2013-12-24 20:51:49

[Beep6581]

Can we close? It's blocking 4.1.

Reported by entertheyoni on 2013-12-30 10:40:58

[Beep6581]

I tried with new unsupported models before and after input data in camconst.json ..

Before any data in camconst.json RT cuts a 4 pixel border from the frame while Dcraw
keeps the hole frame.

Nikon Df ..http://www.imaging-resource.com/PRODS/nikon-df/DFhVFAI000100.NEF.HTM
The exif tag says frame dimensions are 4992 X 3292. Dcraw exports 4992 X 3292. In RT
they are 4984 X 3284.

Inspecting undemosaiced data (dcraw -D) it is obvious that we have to remove 2 columns
from left and 50 columns from the right side.

Converting to DNG we take a 4940 X 3292 file without side areas to remove. In DNG's
exif the "active area" tag is 0 0 3292 4940 ..

After inserting in camconst.json "raw_crop": [ 0, 0, 4940, 3292 ], the image dimensions
(for both NEF and DNG) in RT become 4932 X 3284 with no visual problems at the sides.
With "raw_crop": [ 0, 0, 4944, 3292 ] image is 4936 X 3284 and there are still no visual
problems at the sides for the NEF but RT crashes during opening the DNG.

With the alternative format for "raw crop" [ 0, 0, -46, -0 ] the result is again 4936
X 3284 in nef but 4896 X 3284 in DNG, 46 columns of useful area removed .. :(.

So it looks like if we want the full active area, this 4 pixel border should be removed
from RT .. 
Or camconst.json should be inactive for DNG files at least for the part of image area
- image borders .. then we could gain back the right and bottom borders ..  

"Full" camconst data for Nikon Df
    {
        "make_model": "Nikon Df",
        "dcraw_matrix": [ 8598,-2848,-857,-5618,13606,2195,-1002,1773,7137 ],
        "raw_crop": [ 0, 0, 4940, 3292 ],
        "ranges": { "black": 0, "white": 15480 }
    },

A more complicated situation with Canon Powershot S120 will follow in a few hours ..

Reported by iliasgiarimis on 2014-01-05 03:19:02

[Beep6581]

Hi Ilias, as you can compile now, you can test with no border by changing line 74 of
rawimagesource.cc from

,border(4)

to

,border(0)

Ingo

Reported by heckflosse@i-weyrich.de on 2014-01-05 21:06:48

[Beep6581]

Thanks Ingo, this will make the job (calculating measuring areas) easier if there is
not a side effect ..

Reported by iliasgiarimis on 2014-01-05 23:00:04

[Beep6581]

I tested with NEF files and fast- and amaze-demosaic and got no side effects, but that
doesn't mean that there are none...

Reported by heckflosse@i-weyrich.de on 2014-01-05 23:25:07

[Beep6581]

Most demosaicers cannot interpolate borders, as to calculate pixel at x,y information
from x+/-2 and y+/-2 (or more is needed). So what is done is that the borders are interpolated
with some simplistic demosaicer, usually bilinear filtering (I think that's the case
with RT too), and then the real demosaicer (amaze etc) is used for the rest. As the
border pixels will be of lower quality they are then cropped away.

The artifacts of bilinear demosaicing will only be evident if there's sharp detailed
subject there at the border though, and since there's often out of focus or fuzzy lens
there you will in practice quite seldom see artifacts I think. The safe way is to crop
it though, as it's done today.

Different raw converters crop away different amounts so one can see differences between
say RT and Lightroom what the output dimensions become. Doing like dcraw seems to do
(keep the whole frame) is rare though, I'd suspect that dcraw is quite alone doing
so.

The camconst.json constants should work pre-crop, which I think they do. It does make
it a bit more difficult to use though due to border cropping in program.

A check-box in the raw tab called "crop interpolated borders" enabled per default maybe
could be introduced so advanced users could disable it when adjusting border cropping
etc. What do you think?

I think that cropping should be enabled per default, ie we should not have border(0).

Reported by torger@ludd.ltu.se on 2014-01-06 15:28:37

[Beep6581]

I also think that we should stay at border(4). I only wanted to point Ilias to the relevant
line of code for his tests.

Reported by heckflosse@i-weyrich.de on 2014-01-06 15:44:46

[Beep6581]

I would not care for the 4 pixels border if it didn't skew the measures of "raw crop"
, "masked areas" etc. After all DNG uses larger cropped borders (6).
Now that Raw Digger is commercial the ability to do all the needed job for camconst.json
in RT is even more valuable. So we need either uncropped raw or clearly defined behavior.

BTW  I think "masked areas" is not the appropriate name for what we really define in
camconst. In masked areas of a sensor there are optically black areas where we can
measure the BlackLevel but also some areas where the pixel values are systematically
higher or lower than Black Level (calibration data ??). We only use the areas where
we can measure the Black Level so may be a "Black Level area" is more correct. This
will become clear after I will upload the S120 example ..

I had some (unexpected and pleasant) family meetings last days so I haven't completed
some work (yet) on this but I am in a state that I cannot define the precise method
to declare "masked areas". I think that even for "measuring area" RT crops after reading
camconst values. With Canon S120 the "measure area" looks to be columns 0-80 so (for
safety) I would crop 2-4 columns left and right i.e. 2-78 but experimenting with various
values I find that even 0-82 works OK !!. If anyone can find something related in the
code it would be useful to report (I am far from been able to read C code .. yet ..).

Anders's proposal for a "crop inerpolated borders" check box meets our needs. As it
will specially used only for raw inspection this could be in preferences ..
One more useful option for this kind of jobs (raw inspecting) is to make easier to
use the Demosaic method = none. As it is now in order to have undemosiced data, one
has to close RT and edit the pp3 file. Then RT keeps this setting (but this stops with
any pp3 preset selected afterwards..).

Reported by iliasgiarimis on 2014-01-06 16:50:04

[Beep6581]

maskedareas name is chosen because it's the same name and same formatting and meaning
as MaskedAreas in the DNG specification, and Dcraw code also used "mask", so while
black level areas may be a better name I prefer keeping MaskedAreas as DNG spec and
dcraw already chosen that name, ie it's established in raw converter code.

I'll look into the code now and see if there's some strangeness related to cropping.

Reported by torger@ludd.ltu.se on 2014-01-07 12:02:46

[Beep6581]

As far as I can see in the code, all camconst code is pre-crop, ie the border stuff
is done afterwards.

This is how it should be. However the problem is obviously if you use RawTherapee itself
for trial-and-error to find proper raw cropping you don't see the full frame and thus
it becomes difficult. Today it's probably better to use DCRAW.

Reported by torger@ludd.ltu.se on 2014-01-07 12:10:26

[Beep6581]

What's the status?

Reported by entertheyoni on 2014-02-02 15:54:26

[Beep6581]

A bit difficult to use RT to find the crop borders (better to use DCRAW), but once you
have them it can be put into camconst.json, so I think it's good as it is.

In the future we could probably put in some configure setting to not crop borders for
those that use RT to find the borders.

I think it could be set to "fixed" and any other stuff can be filed separately as improvements.

Reported by torger@ludd.ltu.se on 2014-02-03 09:51:49

[Beep6581]

Couldn't we add the hidden "none" demosaicing method to the visible combobox, and then
have RT not crop borders when it's selected?

Reported by entertheyoni on 2014-02-03 12:06:25

[Beep6581]

Yes I think that's a good idea, probably quite easy to do. Unfortunately I'm swamped
with work these days so I cannot contribute with code currently.

Reported by torger@ludd.ltu.se on 2014-02-03 12:25:07

[Beep6581]

I'll close this issue to free 4.1, and open a new one for comment #42/43.

Reported by entertheyoni on 2014-02-11 13:42:44

• Status changed: Fixed