Review: Refactored baker code to allow for more advanced lut output


Jeremy Selan <jeremy...@...>
 

https://github.com/imageworks/OpenColorIO/pull/116

This addresses oliver's bug where an HDR lin -> Dreamcolor ociobakelut
command was clipping the highlights.
After this fix, you can get a proper output .csp both with and without
the --shaper specified. (if the shaper is not
specified, it will fall back on the allocation for the input space,
and for oliver's linear example (lg2, -7.5, 4) will match
the 'log' shaper space almost exactly.

There is one known regression in this code - I have temporarily
disabled the .hdl lut baking as I have not ported that code.
But I wanted to get this checkin in before then, as it's such a large
change (in terms of number of lines of code).

Other changes:
* Added .3dl baking
* Added Processor.hasChannelCrosstalk()
* Exposed both FormatName + FormatExtension
* FileFormats can register multiple names. (ex: lustre 3dl / flame 3dl)
* Baker defaults are format specific
* ociobakeluts has cleaned up syntax, fewer args needed in most cases

The bakelut logic has been moved from the Baker.cpp, into each output format.
This can eventually be re-refactored, but in the meantime until we have more
examples I think we can get higher quality (format specific) color output
by doing the logic in each outputter.

Remaining tasks:
* update houdini lut baker to work again. (currently disabled)
* let the .csp output dynamically pick between a 1d and 3d output (now
that hasChannelCrosstalk is exposed, this will be trivial)

-- Jeremy


Oliver Farkas <oliver...@...>
 

Hey Jeremy, sorry about the late reply, I seem to have looked over this email.

This is really helpful, thanks for this.

I found a crash bug in the OCIO code. Ocioconvert crashes when you feed in EXR's that have render errors in them, basically pixel values that are  way off. After loading the file with OIIO calls, these values come out as NaNs and cause the processor->apply code to segfault and crash.

It happens in Lut1Dop.cpp on line 272.

I could work around this by checking for the buffer values in this function, and replacing them if they're NaN, but it's obviously not the best place to do this sort of check. I have had not much time to dig deeper but I thought it's gonna help you to fix it.

Thanks again for your fix, 
Oliver

On Sat, May 14, 2011 at 4:18 AM, Jeremy Selan <jeremy...@...> wrote:
https://github.com/imageworks/OpenColorIO/pull/116

This addresses oliver's bug where an HDR lin -> Dreamcolor ociobakelut
command was clipping the highlights.
After this fix, you can get a proper output .csp both with and without
the --shaper specified.  (if the shaper is not
specified, it will fall back on the allocation for the input space,
and for oliver's linear example (lg2, -7.5, 4) will match
the 'log' shaper space almost exactly.

There is one known regression in this code - I have temporarily
disabled the .hdl lut baking as I have not ported that code.
But I wanted to get this checkin in before then, as it's such a large
change (in terms of number of lines of code).

Other changes:
* Added .3dl baking
* Added Processor.hasChannelCrosstalk()
* Exposed both FormatName + FormatExtension
* FileFormats can register multiple names. (ex: lustre 3dl / flame 3dl)
* Baker defaults are format specific
* ociobakeluts has cleaned up syntax, fewer args needed in most cases

The bakelut logic has been moved from the Baker.cpp, into each output format.
This can eventually be re-refactored, but in the meantime until we have more
examples I think we can get higher quality (format specific) color output
by doing the logic in each outputter.

Remaining tasks:
* update houdini lut baker to work again. (currently disabled)
* let the .csp output dynamically pick between a 1d and 3d output (now
that hasChannelCrosstalk is exposed, this will be trivial)

-- Jeremy


Jeremy Selan <jeremy...@...>
 

Thanks for the report. I'll try to get this fixed asap. What platform
are you on?

On Thu, May 19, 2011 at 5:12 PM, Oliver Farkas <oliver...@gmail.com> wrote:
Hey Jeremy, sorry about the late reply, I seem to have looked over this
email.
This is really helpful, thanks for this.
I found a crash bug in the OCIO code. Ocioconvert crashes when you feed in
EXR's that have render errors in them, basically pixel values that are  way
off. After loading the file with OIIO calls, these values come out as NaNs
and cause the processor->apply code to segfault and crash.
It happens in Lut1Dop.cpp on line 272.
I could work around this by checking for the buffer values in this function,
and replacing them if they're NaN, but it's obviously not the best place to
do this sort of check. I have had not much time to dig deeper but I thought
it's gonna help you to fix it.
Thanks again for your fix,
Oliver
On Sat, May 14, 2011 at 4:18 AM, Jeremy Selan <jeremy...@gmail.com>
wrote:

https://github.com/imageworks/OpenColorIO/pull/116

This addresses oliver's bug where an HDR lin -> Dreamcolor ociobakelut
command was clipping the highlights.
After this fix, you can get a proper output .csp both with and without
the --shaper specified.  (if the shaper is not
specified, it will fall back on the allocation for the input space,
and for oliver's linear example (lg2, -7.5, 4) will match
the 'log' shaper space almost exactly.

There is one known regression in this code - I have temporarily
disabled the .hdl lut baking as I have not ported that code.
But I wanted to get this checkin in before then, as it's such a large
change (in terms of number of lines of code).

Other changes:
* Added .3dl baking
* Added Processor.hasChannelCrosstalk()
* Exposed both FormatName + FormatExtension
* FileFormats can register multiple names. (ex: lustre 3dl / flame 3dl)
* Baker defaults are format specific
* ociobakeluts has cleaned up syntax, fewer args needed in most cases

The bakelut logic has been moved from the Baker.cpp, into each output
format.
This can eventually be re-refactored, but in the meantime until we have
more
examples I think we can get higher quality (format specific) color output
by doing the logic in each outputter.

Remaining tasks:
* update houdini lut baker to work again. (currently disabled)
* let the .csp output dynamically pick between a 1d and 3d output (now
that hasChannelCrosstalk is exposed, this will be trivial)

-- Jeremy


Jeremy Selan <jeremy...@...>
 

I've been able to reproduce this now - thanks.

-- Jeremy

On Thu, May 19, 2011 at 5:36 PM, Jeremy Selan <jeremy...@gmail.com> wrote:
Thanks for the report. I'll try to get this fixed asap. What platform
are you on?

On Thu, May 19, 2011 at 5:12 PM, Oliver Farkas <oliver...@gmail.com> wrote:
Hey Jeremy, sorry about the late reply, I seem to have looked over this
email.
This is really helpful, thanks for this.
I found a crash bug in the OCIO code. Ocioconvert crashes when you feed in
EXR's that have render errors in them, basically pixel values that are  way
off. After loading the file with OIIO calls, these values come out as NaNs
and cause the processor->apply code to segfault and crash.
It happens in Lut1Dop.cpp on line 272.
I could work around this by checking for the buffer values in this function,
and replacing them if they're NaN, but it's obviously not the best place to
do this sort of check. I have had not much time to dig deeper but I thought
it's gonna help you to fix it.
Thanks again for your fix,
Oliver
On Sat, May 14, 2011 at 4:18 AM, Jeremy Selan <jeremy...@gmail.com>
wrote:

https://github.com/imageworks/OpenColorIO/pull/116

This addresses oliver's bug where an HDR lin -> Dreamcolor ociobakelut
command was clipping the highlights.
After this fix, you can get a proper output .csp both with and without
the --shaper specified.  (if the shaper is not
specified, it will fall back on the allocation for the input space,
and for oliver's linear example (lg2, -7.5, 4) will match
the 'log' shaper space almost exactly.

There is one known regression in this code - I have temporarily
disabled the .hdl lut baking as I have not ported that code.
But I wanted to get this checkin in before then, as it's such a large
change (in terms of number of lines of code).

Other changes:
* Added .3dl baking
* Added Processor.hasChannelCrosstalk()
* Exposed both FormatName + FormatExtension
* FileFormats can register multiple names. (ex: lustre 3dl / flame 3dl)
* Baker defaults are format specific
* ociobakeluts has cleaned up syntax, fewer args needed in most cases

The bakelut logic has been moved from the Baker.cpp, into each output
format.
This can eventually be re-refactored, but in the meantime until we have
more
examples I think we can get higher quality (format specific) color output
by doing the logic in each outputter.

Remaining tasks:
* update houdini lut baker to work again. (currently disabled)
* let the .csp output dynamically pick between a 1d and 3d output (now
that hasChannelCrosstalk is exposed, this will be trivial)

-- Jeremy


Jeremy Selan <jeremy...@...>
 

In case the nan/inf bug is holding you up - I've fixed the crash and
pushed it to a local branch:
https://github.com/jeremyselan/OpenColorIO/tree/naninf

I'm holding off though in pushing this to the main trunk pending a bit
more work.

I'm in the process of adding explicit unit tests to all image
processing operators, to guarantee that we have uniform nan/inf
handling through the entire library. I hope to get it all wrapped up
next week. (FYI - my intent is to preserve nan/inf value to the
greatest extend possible). I.e., if you run a 'nan' though a 1d lut,
it preserves the nan value. If you run a nan though a 3d lut (in any
input channel), the output will be nan in all 3 output color channels.
I believe this best preserves image integrity and obeys the design
principle of 'least surprise'.

-- Jeremy


Oliver Farkas <oliver...@...>
 

Ok, thanks a lot for the fix Jeremy. 

Oliver

On Wed, May 25, 2011 at 3:08 AM, Jeremy Selan <jeremy...@...> wrote:
In case the nan/inf bug is holding you up - I've fixed the crash and
pushed it to a local branch:
https://github.com/jeremyselan/OpenColorIO/tree/naninf

I'm holding off though in pushing this to the main trunk pending a bit
more work.

I'm in the process of adding explicit unit tests to all image
processing operators, to guarantee that we have uniform nan/inf
handling through the entire library. I hope to get it all wrapped up
next week.  (FYI - my intent is to preserve nan/inf value to the
greatest extend possible).  I.e., if you run a 'nan' though a 1d lut,
it preserves the nan value.  If you run a nan though a 3d lut (in any
input channel), the output will be nan in all 3 output color channels.
 I believe this best preserves image integrity and obeys the design
principle of 'least surprise'.

-- Jeremy