Discussion:
[Oiio-dev] libRaw/dcraw 'auto_bright' changing raw image exposure
Haarm-Pieter Duiker
2016-09-10 14:45:59 UTC
Permalink
Hello,

I've been running into an issue with the OIIO libRaw support and wanted to
see if anyone else had run into the same problem and found a workaround.

The issue
Raw files read and developed by OIIO using libRaw are automatically
brightened.

The potential cause tl;dr
The RawInput::open method doesn't set the 'no_auto_bright' flag
https://github.com/OpenImageIO/oiio/blob/master/src/raw.imageio/rawinput.cpp#L130
like dcraw does when using either the '-4' flag or the '-W' flag.
https://github.com/LibRaw/LibRaw/blob/master/samples/dcraw_emu.cpp#L337
https://www.cybercom.net/~dcoffin/dcraw/dcraw.c - line 9963 or 9965

Context
If you use dcraw, or libRaw's dcraw_emu, to convert raw images to Tiff, and
you want to produce linear 16 bit data for your exposures, the '-4' flag is
often used. That is a shorthand for the flags '-6 -W -g 1 1'. '-6' set up
16 bit data processing. '-g 1 1' sets the gammas to 1.0, 1.0. '-W' turns
off dcraw's 'auto_bright' functionality. The last flag is especially
important if you want to merge exposures or otherwise treat your data as a
more exact measurement of the world.

Suggestion
Add the line:
m_processor.imgdata.params.no_auto_bright = 1;
around line 137 of rawinput.cpp
https://github.com/OpenImageIO/oiio/blob/master/src/raw.imageio/rawinput.cpp#L137

Open question
For general OIIO usage, is there a good reason to be able to turn
'auto_bright' on?

Will be testing that out this weekend but figured I'd see if there was an
alternate approach to solving this problem.

Thanks in advance for your help,
HP
Larry Gritz
2016-09-12 17:07:44 UTC
Permalink
There are certainly some things we want a reader to do automatically before the data is passed through OIIO to the calling app -- uncompress, de-bayerization, making equal resolution for all color channels, converting to some kind of RGB-based color space, converting to one of the standard (whole-byte) data types, etc.

But given those constraints, the general approach in OIIO is usually to get as close to raw data as possible. So if libraw is capable of giving us photometrically linear and relatively unprocessed data without losing precision, that's what we should be asking for and giving to the app. At least by default -- it's fine to have a "config" parameter that requests other functionality that libraw is capable of, such as the auto_bright or gamma correction. But I don't think that should be the default.

Is Mark or anybody else from DNeg reading this? Would that be ok for you if we changed the default, and you would use the configuration options if you had been intentionally using auto-bright all along? Or are we overlooking something?

It would probably be a good thing for somebody to read the libraw docs in detail, and make sure that we have ALL the defaults set sensibly, with 'config' overrides for any particularly interesting functionality that it can do upon reading.
Post by Haarm-Pieter Duiker
Hello,
I've been running into an issue with the OIIO libRaw support and wanted to see if anyone else had run into the same problem and found a workaround.
The issue
Raw files read and developed by OIIO using libRaw are automatically brightened.
The potential cause tl;dr
The RawInput::open method doesn't set the 'no_auto_bright' flag
https://github.com/OpenImageIO/oiio/blob/master/src/raw.imageio/rawinput.cpp#L130 <https://github.com/OpenImageIO/oiio/blob/master/src/raw.imageio/rawinput.cpp#L130>
like dcraw does when using either the '-4' flag or the '-W' flag.
https://github.com/LibRaw/LibRaw/blob/master/samples/dcraw_emu.cpp#L337 <https://github.com/LibRaw/LibRaw/blob/master/samples/dcraw_emu.cpp#L337>
https://www.cybercom.net/~dcoffin/dcraw/dcraw.c <https://www.cybercom.net/~dcoffin/dcraw/dcraw.c> - line 9963 or 9965
Context
If you use dcraw, or libRaw's dcraw_emu, to convert raw images to Tiff, and you want to produce linear 16 bit data for your exposures, the '-4' flag is often used. That is a shorthand for the flags '-6 -W -g 1 1'. '-6' set up 16 bit data processing. '-g 1 1' sets the gammas to 1.0, 1.0. '-W' turns off dcraw's 'auto_bright' functionality. The last flag is especially important if you want to merge exposures or otherwise treat your data as a more exact measurement of the world.
Suggestion
m_processor.imgdata.params.no_auto_bright = 1;
around line 137 of rawinput.cpp
https://github.com/OpenImageIO/oiio/blob/master/src/raw.imageio/rawinput.cpp#L137 <https://github.com/OpenImageIO/oiio/blob/master/src/raw.imageio/rawinput.cpp#L137>
Open question
For general OIIO usage, is there a good reason to be able to turn 'auto_bright' on?
Will be testing that out this weekend but figured I'd see if there was an alternate approach to solving this problem.
Thanks in advance for your help,
HP
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
--
Larry Gritz
***@larrygritz.com
Haarm-Pieter Duiker
2016-09-13 06:09:26 UTC
Permalink
After digging into a bit more, the dcraw flags that the current rawinput
libRaw calls correspond to are:

- -o <gamut>
- Choose the output colorspace
- -6
- Set up 16 bit processing
- -g 1 1
- Set the gamma curves to 1, 1

I'd suggest we add API calls corresponding to two more flags

- -W
- Turn _off_ auto exposure
- -w
- Use the camera white balance

The code to make those API calls is copied below. I've tested this on an
OSX build from the current repo head and found it to have the intended
effect.

Kevin, Alex, (whoever else is a dcraw guru): Any suggestions on other flags
to use. Those are the flags I've used.

It would be nice to be able to configure more of the raw processing options
before an image is opened.

Hope that helps,
HP

Additions to rawinput.cpp starting at line 137
// Turn off automatic exposure correction
m_processor.imgdata.params.no_auto_bright = 1;

// Turn on camera white balance
m_processor.imgdata.params.use_camera_wb = 1;
Post by Larry Gritz
There are certainly some things we want a reader to do automatically
before the data is passed through OIIO to the calling app -- uncompress,
de-bayerization, making equal resolution for all color channels, converting
to some kind of RGB-based color space, converting to one of the standard
(whole-byte) data types, etc.
But given those constraints, the general approach in OIIO is usually to
get as close to raw data as possible. So if libraw is capable of giving us
photometrically linear and relatively unprocessed data without losing
precision, that's what we should be asking for and giving to the app. At
least by default -- it's fine to have a "config" parameter that requests
other functionality that libraw is capable of, such as the auto_bright or
gamma correction. But I don't think that should be the default.
Is Mark or anybody else from DNeg reading this? Would that be ok for you
if we changed the default, and you would use the configuration options if
you had been intentionally using auto-bright all along? Or are we
overlooking something?
It would probably be a good thing for somebody to read the libraw docs in
detail, and make sure that we have ALL the defaults set sensibly, with
'config' overrides for any particularly interesting functionality that it
can do upon reading.
Hello,
I've been running into an issue with the OIIO libRaw support and wanted to
see if anyone else had run into the same problem and found a workaround.
The issue
Raw files read and developed by OIIO using libRaw are automatically brightened.
The potential cause tl;dr
The RawInput::open method doesn't set the 'no_auto_bright' flag
https://github.com/OpenImageIO/oiio/blob/master/
src/raw.imageio/rawinput.cpp#L130
like dcraw does when using either the '-4' flag or the '-W' flag.
https://github.com/LibRaw/LibRaw/blob/master/samples/dcraw_emu.cpp#L337
https://www.cybercom.net/~dcoffin/dcraw/dcraw.c - line 9963 or 9965
Context
If you use dcraw, or libRaw's dcraw_emu, to convert raw images to Tiff,
and you want to produce linear 16 bit data for your exposures, the '-4'
flag is often used. That is a shorthand for the flags '-6 -W -g 1 1'. '-6'
set up 16 bit data processing. '-g 1 1' sets the gammas to 1.0, 1.0. '-W'
turns off dcraw's 'auto_bright' functionality. The last flag is especially
important if you want to merge exposures or otherwise treat your data as a
more exact measurement of the world.
Suggestion
m_processor.imgdata.params.no_auto_bright = 1;
around line 137 of rawinput.cpp
https://github.com/OpenImageIO/oiio/blob/master/
src/raw.imageio/rawinput.cpp#L137
Open question
For general OIIO usage, is there a good reason to be able to turn 'auto_bright' on?
Will be testing that out this weekend but figured I'd see if there was an
alternate approach to solving this problem.
Thanks in advance for your help,
HP
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
--
Larry Gritz
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
Kevin Wheatley
2016-09-13 07:36:39 UTC
Permalink
So here are the kinds of things we do...

// Setup as much as possible to mean linear 16 bit
RawProcessor.imgdata.params.output_bps = 16;
RawProcessor.imgdata.params.gamm[0] = 1;
RawProcessor.imgdata.params.gamm[1] = 1;
RawProcessor.imgdata.params.no_auto_bright = 1;
RawProcessor.imgdata.params.adjust_maximum_thr = 0.0;

// Some parameters effect open() and unpack()
RawProcessor.imgdata.params.use_camera_matrix = <variable>
RawProcessor.imgdata.params.use_camera_wb = <variable>

and some more parts... but those are the important colour ones, other
than intercepting the raw colour space buffer and calculating the
matrix multiplication ourselves in float to avoid clipping due to
white balance/etc.

Kevin
Haarm-Pieter Duiker
2016-09-14 04:41:18 UTC
Permalink
It looks like that 'adjust_maximum_thr' should be set to 0.0 generally. The
comment from the libRaw changelog is as follows:
"
* dcraw_emu's -c parameter now wants numeric (float) argument.
This value
is assigned to params.adjust_maximum_thr.
Use -c 0.0 for dcraw compatibility.

"
It's odd that they overrode the -c command line option. With dcraw, that
forces it to write data to stdout. dcraw_emu's -c option has completely
different behavior.

Regardless, setting that struct value to 0.0 seems like it should be done
as a matter of course.

HP
Post by Kevin Wheatley
So here are the kinds of things we do...
// Setup as much as possible to mean linear 16 bit
RawProcessor.imgdata.params.output_bps = 16;
RawProcessor.imgdata.params.gamm[0] = 1;
RawProcessor.imgdata.params.gamm[1] = 1;
RawProcessor.imgdata.params.no_auto_bright = 1;
RawProcessor.imgdata.params.adjust_maximum_thr = 0.0;
// Some parameters effect open() and unpack()
RawProcessor.imgdata.params.use_camera_matrix = <variable>
RawProcessor.imgdata.params.use_camera_wb = <variable>
and some more parts... but those are the important colour ones, other
than intercepting the raw colour space buffer and calculating the
matrix multiplication ourselves in float to avoid clipping due to
white balance/etc.
Kevin
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
Larry Gritz
2016-09-15 03:20:00 UTC
Permalink
How does this look? https://github.com/OpenImageIO/oiio/pull/1490 <https://github.com/OpenImageIO/oiio/pull/1490>
"
* dcraw_emu's -c parameter now wants numeric (float) argument. This value
is assigned to params.adjust_maximum_thr.
Use -c 0.0 for dcraw compatibility.
"
It's odd that they overrode the -c command line option. With dcraw, that forces it to write data to stdout. dcraw_emu's -c option has completely different behavior.
Regardless, setting that struct value to 0.0 seems like it should be done as a matter of course.
HP
So here are the kinds of things we do...
// Setup as much as possible to mean linear 16 bit
RawProcessor.imgdata.params.output_bps = 16;
RawProcessor.imgdata.params.gamm[0] = 1;
RawProcessor.imgdata.params.gamm[1] = 1;
RawProcessor.imgdata.params.no_auto_bright = 1;
RawProcessor.imgdata.params.adjust_maximum_thr = 0.0;
// Some parameters effect open() and unpack()
RawProcessor.imgdata.params.use_camera_matrix = <variable>
RawProcessor.imgdata.params.use_camera_wb = <variable>
and some more parts... but those are the important colour ones, other
than intercepting the raw colour space buffer and calculating the
matrix multiplication ourselves in float to avoid clipping due to
white balance/etc.
Kevin
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org>
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
--
Larry Gritz
***@larrygritz.com
Larry Gritz
2016-09-19 21:23:18 UTC
Permalink
HP, does this look right to you?
Post by Larry Gritz
How does this look? https://github.com/OpenImageIO/oiio/pull/1490 <https://github.com/OpenImageIO/oiio/pull/1490>
"
* dcraw_emu's -c parameter now wants numeric (float) argument. This value
is assigned to params.adjust_maximum_thr.
Use -c 0.0 for dcraw compatibility.
"
It's odd that they overrode the -c command line option. With dcraw, that forces it to write data to stdout. dcraw_emu's -c option has completely different behavior.
Regardless, setting that struct value to 0.0 seems like it should be done as a matter of course.
HP
So here are the kinds of things we do...
// Setup as much as possible to mean linear 16 bit
RawProcessor.imgdata.params.output_bps = 16;
RawProcessor.imgdata.params.gamm[0] = 1;
RawProcessor.imgdata.params.gamm[1] = 1;
RawProcessor.imgdata.params.no_auto_bright = 1;
RawProcessor.imgdata.params.adjust_maximum_thr = 0.0;
// Some parameters effect open() and unpack()
RawProcessor.imgdata.params.use_camera_matrix = <variable>
RawProcessor.imgdata.params.use_camera_wb = <variable>
and some more parts... but those are the important colour ones, other
than intercepting the raw colour space buffer and calculating the
matrix multiplication ourselves in float to avoid clipping due to
white balance/etc.
Kevin
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org>
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
--
Larry Gritz
--
Larry Gritz
***@larrygritz.com
Haarm-Pieter Duiker
2016-09-20 06:57:50 UTC
Permalink
The pull request is behaving as expected for the
attributes "raw:auto_bright", "raw:use_camera_wb",
"raw:adjust_maximum_thr", "raw:use_camera_matrix"
and "raw:ColorSpace".

"raw:adjust_maximum_thr" and "raw:use_camera_matrix" don't do much in my
tests though.

The 'buf.reset' call from the Python path you outlined gives an error,
copied below. The Python path from
https://github.com/hpd/general/blob/master/hdr/python/mkhdr.py#L104
works though.

HP
Post by Larry Gritz
Post by Kevin Wheatley
buf.reset (inputPath, config=spec)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
Boost.Python.ArgumentError: Python argument types in
ImageBuf.reset(ImageBuf, str)
did not match C++ signature:
reset(OpenImageIO::v1_7::ImageBuf {lvalue},
OpenImageIO::v1_7::ImageSpec)
reset(OpenImageIO::v1_7::ImageBuf {lvalue},
std::__1::basic_string<char, std::__1::char_traits<char>,
std::__1::allocator<char> >, int, int, OpenImageIO::v1_7::ImageSpec)
reset(OpenImageIO::v1_7::ImageBuf {lvalue},
std::__1::basic_string<char, std::__1::char_traits<char>,
std::__1::allocator<char> >, int, int)
reset(OpenImageIO::v1_7::ImageBuf {lvalue},
std::__1::basic_string<char, std::__1::char_traits<char>,
std::__1::allocator<char> >)
Post by Larry Gritz
HP, does this look right to you?
How does this look? https://github.com/OpenImageIO/oiio/pull/1490
It looks like that 'adjust_maximum_thr' should be set to 0.0 generally.
"
* dcraw_emu's -c parameter now wants numeric (float) argument. This value
is assigned to params.adjust_maximum_thr.
Use -c 0.0 for dcraw compatibility.
"
It's odd that they overrode the -c command line option. With dcraw, that
forces it to write data to stdout. dcraw_emu's -c option has completely
different behavior.
Regardless, setting that struct value to 0.0 seems like it should be done
as a matter of course.
HP
On Tue, Sep 13, 2016 at 12:36 AM, Kevin Wheatley <
Post by Kevin Wheatley
So here are the kinds of things we do...
// Setup as much as possible to mean linear 16 bit
RawProcessor.imgdata.params.output_bps = 16;
RawProcessor.imgdata.params.gamm[0] = 1;
RawProcessor.imgdata.params.gamm[1] = 1;
RawProcessor.imgdata.params.no_auto_bright = 1;
RawProcessor.imgdata.params.adjust_maximum_thr = 0.0;
// Some parameters effect open() and unpack()
RawProcessor.imgdata.params.use_camera_matrix = <variable>
RawProcessor.imgdata.params.use_camera_wb = <variable>
and some more parts... but those are the important colour ones, other
than intercepting the raw colour space buffer and calculating the
matrix multiplication ourselves in float to avoid clipping due to
white balance/etc.
Kevin
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
--
Larry Gritz
--
Larry Gritz
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
Larry Gritz
2016-09-20 18:37:00 UTC
Permalink
My bad, I think the reset line,

buf.reset ("myrawfile.cr2", config=cfg)

will work if instead you write:

buf.reset ("myrawfile.cr2", 0, 0, cfg)

I think I neglected to add the right instructions to let it understand the default arguments and parameter names. I'll submit a separate patch for that.

I'm not sure I understand your comment about maximum_thr and camera_matrix -- do you mean "they seem to work, but my example doesn't use them so I haven't tested it thoroughly", or do you mean "my example uses them, but I see no behavior change when I adjust them, so I suspect they are not working properly?"
The pull request is behaving as expected for the attributes "raw:auto_bright", "raw:use_camera_wb", "raw:adjust_maximum_thr", "raw:use_camera_matrix" and "raw:ColorSpace".
"raw:adjust_maximum_thr" and "raw:use_camera_matrix" don't do much in my tests though.
The 'buf.reset' call from the Python path you outlined gives an error, copied below. The Python path from
https://github.com/hpd/general/blob/master/hdr/python/mkhdr.py#L104 <https://github.com/hpd/general/blob/master/hdr/python/mkhdr.py#L104>
works though.
HP
Post by Larry Gritz
buf.reset (inputPath, config=spec)
File "<stdin>", line 1, in <module>
Boost.Python.ArgumentError: Python argument types in
ImageBuf.reset(ImageBuf, str)
reset(OpenImageIO::v1_7::ImageBuf {lvalue}, OpenImageIO::v1_7::ImageSpec)
reset(OpenImageIO::v1_7::ImageBuf {lvalue}, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, int, int, OpenImageIO::v1_7::ImageSpec)
reset(OpenImageIO::v1_7::ImageBuf {lvalue}, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, int, int)
reset(OpenImageIO::v1_7::ImageBuf {lvalue}, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >)
HP, does this look right to you?
Post by Larry Gritz
How does this look? https://github.com/OpenImageIO/oiio/pull/1490 <https://github.com/OpenImageIO/oiio/pull/1490>
"
* dcraw_emu's -c parameter now wants numeric (float) argument. This value
is assigned to params.adjust_maximum_thr.
Use -c 0.0 for dcraw compatibility.
"
It's odd that they overrode the -c command line option. With dcraw, that forces it to write data to stdout. dcraw_emu's -c option has completely different behavior.
Regardless, setting that struct value to 0.0 seems like it should be done as a matter of course.
HP
So here are the kinds of things we do...
// Setup as much as possible to mean linear 16 bit
RawProcessor.imgdata.params.output_bps = 16;
RawProcessor.imgdata.params.ga <http://rawprocessor.imgdata.params.ga/>mm[0] = 1;
RawProcessor.imgdata.params.ga <http://rawprocessor.imgdata.params.ga/>mm[1] = 1;
RawProcessor.imgdata.params.no <http://rawprocessor.imgdata.params.no/>_auto_bright = 1;
RawProcessor.imgdata.params.ad <http://rawprocessor.imgdata.params.ad/>just_maximum_thr = 0.0;
// Some parameters effect open() and unpack()
RawProcessor.imgdata.params.us <http://rawprocessor.imgdata.params.us/>e_camera_matrix = <variable>
RawProcessor.imgdata.params.us <http://rawprocessor.imgdata.params.us/>e_camera_wb = <variable>
and some more parts... but those are the important colour ones, other
than intercepting the raw colour space buffer and calculating the
matrix multiplication ourselves in float to avoid clipping due to
white balance/etc.
Kevin
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org>
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org>
--
Larry Gritz
--
Larry Gritz
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org>
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
--
Larry Gritz
***@larrygritz.com
Haarm-Pieter Duiker
2016-09-20 20:12:06 UTC
Permalink
"they seem to work, but my example doesn't use them so I haven't tested it
thoroughly"

HP
Post by Larry Gritz
My bad, I think the reset line,
buf.reset ("myrawfile.cr2", config=cfg)
buf.reset ("myrawfile.cr2", 0, 0, cfg)
I think I neglected to add the right instructions to let it understand the
default arguments and parameter names. I'll submit a separate patch for
that.
I'm not sure I understand your comment about maximum_thr and camera_matrix
-- do you mean "they seem to work, but my example doesn't use them so I
haven't tested it thoroughly", or do you mean "my example uses them, but I
see no behavior change when I adjust them, so I suspect they are not
working properly?"
The pull request is behaving as expected for the
adjust_maximum_thr", "raw:use_camera_matrix" and "raw:ColorSpace".
"raw:adjust_maximum_thr" and "raw:use_camera_matrix" don't do much in my tests though.
The 'buf.reset' call from the Python path you outlined gives an error,
copied below. The Python path from
https://github.com/hpd/general/blob/master/hdr/python/mkhdr.py#L104
works though.
HP
Post by Larry Gritz
Post by Kevin Wheatley
buf.reset (inputPath, config=spec)
File "<stdin>", line 1, in <module>
Boost.Python.ArgumentError: Python argument types in
ImageBuf.reset(ImageBuf, str)
reset(OpenImageIO::v1_7::ImageBuf {lvalue},
OpenImageIO::v1_7::ImageSpec)
reset(OpenImageIO::v1_7::ImageBuf {lvalue},
std::__1::basic_string<char, std::__1::char_traits<char>,
std::__1::allocator<char> >, int, int, OpenImageIO::v1_7::ImageSpec)
reset(OpenImageIO::v1_7::ImageBuf {lvalue},
std::__1::basic_string<char, std::__1::char_traits<char>,
std::__1::allocator<char> >, int, int)
reset(OpenImageIO::v1_7::ImageBuf {lvalue},
std::__1::basic_string<char, std::__1::char_traits<char>,
std::__1::allocator<char> >)
Post by Larry Gritz
HP, does this look right to you?
How does this look? https://github.com/OpenImageIO/oiio/pull/1490
It looks like that 'adjust_maximum_thr' should be set to 0.0 generally.
"
* dcraw_emu's -c parameter now wants numeric (float) argument. This value
is assigned to params.adjust_maximum_thr.
Use -c 0.0 for dcraw compatibility.
"
It's odd that they overrode the -c command line option. With dcraw, that
forces it to write data to stdout. dcraw_emu's -c option has completely
different behavior.
Regardless, setting that struct value to 0.0 seems like it should be done
as a matter of course.
HP
On Tue, Sep 13, 2016 at 12:36 AM, Kevin Wheatley <
Post by Kevin Wheatley
So here are the kinds of things we do...
// Setup as much as possible to mean linear 16 bit
RawProcessor.imgdata.params.output_bps = 16;
RawProcessor.imgdata.params.ga
<http://rawprocessor.imgdata.params.ga/>mm[0] = 1;
RawProcessor.imgdata.params.ga
<http://rawprocessor.imgdata.params.ga/>mm[1] = 1;
RawProcessor.imgdata.params.no
<http://rawprocessor.imgdata.params.no/>_auto_bright = 1;
RawProcessor.imgdata.params.ad
<http://rawprocessor.imgdata.params.ad/>just_maximum_thr = 0.0;
// Some parameters effect open() and unpack()
RawProcessor.imgdata.params.us
<http://rawprocessor.imgdata.params.us/>e_camera_matrix = <variable>
RawProcessor.imgdata.params.us
<http://rawprocessor.imgdata.params.us/>e_camera_wb = <variable>
and some more parts... but those are the important colour ones, other
than intercepting the raw colour space buffer and calculating the
matrix multiplication ourselves in float to avoid clipping due to
white balance/etc.
Kevin
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
--
Larry Gritz
--
Larry Gritz
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
--
Larry Gritz
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
Larry Gritz
2016-09-20 21:39:08 UTC
Permalink
In that case -- and considering that the code is plainly setting the parameter passed to libraw and I don't know what else I would do -- I'm going to consider that a "LGTM" and merge the fix.
Post by Larry Gritz
"they seem to work, but my example doesn't use them so I haven't tested it thoroughly"
HP
My bad, I think the reset line,
buf.reset ("myrawfile.cr2", config=cfg)
buf.reset ("myrawfile.cr2", 0, 0, cfg)
I think I neglected to add the right instructions to let it understand the default arguments and parameter names. I'll submit a separate patch for that.
I'm not sure I understand your comment about maximum_thr and camera_matrix -- do you mean "they seem to work, but my example doesn't use them so I haven't tested it thoroughly", or do you mean "my example uses them, but I see no behavior change when I adjust them, so I suspect they are not working properly?"
The pull request is behaving as expected for the attributes "raw:auto_bright", "raw:use_camera_wb", "raw:adjust_maximum_thr", "raw:use_camera_matrix" and "raw:ColorSpace".
"raw:adjust_maximum_thr" and "raw:use_camera_matrix" don't do much in my tests though.
The 'buf.reset' call from the Python path you outlined gives an error, copied below. The Python path from
https://github.com/hpd/general/blob/master/hdr/python/mkhdr.py#L104 <https://github.com/hpd/general/blob/master/hdr/python/mkhdr.py#L104>
works though.
HP
Post by Larry Gritz
buf.reset (inputPath, config=spec)
File "<stdin>", line 1, in <module>
Boost.Python.ArgumentError: Python argument types in
ImageBuf.reset(ImageBuf, str)
reset(OpenImageIO::v1_7::ImageBuf {lvalue}, OpenImageIO::v1_7::ImageSpec)
reset(OpenImageIO::v1_7::ImageBuf {lvalue}, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, int, int, OpenImageIO::v1_7::ImageSpec)
reset(OpenImageIO::v1_7::ImageBuf {lvalue}, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, int, int)
reset(OpenImageIO::v1_7::ImageBuf {lvalue}, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >)
HP, does this look right to you?
Post by Larry Gritz
How does this look? https://github.com/OpenImageIO/oiio/pull/1490 <https://github.com/OpenImageIO/oiio/pull/1490>
"
* dcraw_emu's -c parameter now wants numeric (float) argument. This value
is assigned to params.adjust_maximum_thr.
Use -c 0.0 for dcraw compatibility.
"
It's odd that they overrode the -c command line option. With dcraw, that forces it to write data to stdout. dcraw_emu's -c option has completely different behavior.
Regardless, setting that struct value to 0.0 seems like it should be done as a matter of course.
HP
So here are the kinds of things we do...
// Setup as much as possible to mean linear 16 bit
RawProcessor.imgdata.params.output_bps = 16;
RawProcessor.imgdata.params.ga <http://rawprocessor.imgdata.params.ga/>mm[0] = 1;
RawProcessor.imgdata.params.ga <http://rawprocessor.imgdata.params.ga/>mm[1] = 1;
RawProcessor.imgdata.params.no <http://rawprocessor.imgdata.params.no/>_auto_bright = 1;
RawProcessor.imgdata.params.ad <http://rawprocessor.imgdata.params.ad/>just_maximum_thr = 0.0;
// Some parameters effect open() and unpack()
RawProcessor.imgdata.params.us <http://rawprocessor.imgdata.params.us/>e_camera_matrix = <variable>
RawProcessor.imgdata.params.us <http://rawprocessor.imgdata.params.us/>e_camera_wb = <variable>
and some more parts... but those are the important colour ones, other
than intercepting the raw colour space buffer and calculating the
matrix multiplication ourselves in float to avoid clipping due to
white balance/etc.
Kevin
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org>
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org>
--
Larry Gritz
--
Larry Gritz
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org>
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org>
--
Larry Gritz
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org>
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
--
Larry Gritz
***@larrygritz.com
Larry Gritz
2016-09-20 22:37:38 UTC
Permalink
I think this will fix the buf.reset issue: https://github.com/OpenImageIO/oiio/pull/1492 <https://github.com/OpenImageIO/oiio/pull/1492>
Post by Larry Gritz
My bad, I think the reset line,
buf.reset ("myrawfile.cr2", config=cfg)
buf.reset ("myrawfile.cr2", 0, 0, cfg)
I think I neglected to add the right instructions to let it understand the default arguments and parameter names. I'll submit a separate patch for that.
I'm not sure I understand your comment about maximum_thr and camera_matrix -- do you mean "they seem to work, but my example doesn't use them so I haven't tested it thoroughly", or do you mean "my example uses them, but I see no behavior change when I adjust them, so I suspect they are not working properly?"
The pull request is behaving as expected for the attributes "raw:auto_bright", "raw:use_camera_wb", "raw:adjust_maximum_thr", "raw:use_camera_matrix" and "raw:ColorSpace".
"raw:adjust_maximum_thr" and "raw:use_camera_matrix" don't do much in my tests though.
The 'buf.reset' call from the Python path you outlined gives an error, copied below. The Python path from
https://github.com/hpd/general/blob/master/hdr/python/mkhdr.py#L104 <https://github.com/hpd/general/blob/master/hdr/python/mkhdr.py#L104>
works though.
HP
Post by Larry Gritz
buf.reset (inputPath, config=spec)
File "<stdin>", line 1, in <module>
Boost.Python.ArgumentError: Python argument types in
ImageBuf.reset(ImageBuf, str)
reset(OpenImageIO::v1_7::ImageBuf {lvalue}, OpenImageIO::v1_7::ImageSpec)
reset(OpenImageIO::v1_7::ImageBuf {lvalue}, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, int, int, OpenImageIO::v1_7::ImageSpec)
reset(OpenImageIO::v1_7::ImageBuf {lvalue}, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, int, int)
reset(OpenImageIO::v1_7::ImageBuf {lvalue}, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >)
HP, does this look right to you?
Post by Larry Gritz
How does this look? https://github.com/OpenImageIO/oiio/pull/1490 <https://github.com/OpenImageIO/oiio/pull/1490>
"
* dcraw_emu's -c parameter now wants numeric (float) argument. This value
is assigned to params.adjust_maximum_thr.
Use -c 0.0 for dcraw compatibility.
"
It's odd that they overrode the -c command line option. With dcraw, that forces it to write data to stdout. dcraw_emu's -c option has completely different behavior.
Regardless, setting that struct value to 0.0 seems like it should be done as a matter of course.
HP
So here are the kinds of things we do...
// Setup as much as possible to mean linear 16 bit
RawProcessor.imgdata.params.output_bps = 16;
RawProcessor.imgdata.params.ga <http://rawprocessor.imgdata.params.ga/>mm[0] = 1;
RawProcessor.imgdata.params.ga <http://rawprocessor.imgdata.params.ga/>mm[1] = 1;
RawProcessor.imgdata.params.no <http://rawprocessor.imgdata.params.no/>_auto_bright = 1;
RawProcessor.imgdata.params.ad <http://rawprocessor.imgdata.params.ad/>just_maximum_thr = 0.0;
// Some parameters effect open() and unpack()
RawProcessor.imgdata.params.us <http://rawprocessor.imgdata.params.us/>e_camera_matrix = <variable>
RawProcessor.imgdata.params.us <http://rawprocessor.imgdata.params.us/>e_camera_wb = <variable>
and some more parts... but those are the important colour ones, other
than intercepting the raw colour space buffer and calculating the
matrix multiplication ourselves in float to avoid clipping due to
white balance/etc.
Kevin
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org>
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org>
--
Larry Gritz
--
Larry Gritz
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org>
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
--
Larry Gritz
_______________________________________________
Oiio-dev mailing list
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
--
Larry Gritz
***@larrygritz.com

Loading...