1. ocio-dev@lists.aswf.io
  2. Messages
   Date   
Review: API Change to ColorSpace.GPUAllocation

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

This updates the API for setting the ColorSpace GPU allocation hint.
(The allocation is used to determine how a particular lattice is best
sampled using a fixed domain lut on the GPU). Previously all
allocations were configured with 2 fixed arguments (min, max):

Prior:
Uniform: x mapped from [min, max] to [0,1]
Log2 log2(x) mapped from [min, max] to [0,1]

Now, allocations can be configured with a variable number of
arguments. (The definition of which is allocation specific). This is
useful as it allows for (optionally) more sophisticated allocations on
the GPU. The first allocation to take advantage of this is "LG2",
which adds a linear offset value as the 3rd variable:

Log2 log2(x+offset) mapped from [min, max] to [0,1]

Using a small, positive offset allows a log allocation to represent
the value 0.0 precisely, which is useful in some configurations.

This update also names GpuAllocation -> Allocation, as what it
represents is not GPU specific. (GPU is the use case, rather than the
description of what the data is).

Commits:
3f12fca8bf0aff910a4989b855d711ab609b61f9
e780e5d21333aca75de25403dd56852da17a7517
d17a6530982b86b5ef51601aae97a76e1c2da59d

-- Jeremy
OCIO 0.7.1 released

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

Version 0.7.1 (Nov 15 2010):
* Additional 3d lut formats: Truelight .cub + Iridas .cube
* FileTransform supports envvars and search paths
* Added Nuke plugins: LogConvert + FileTransform
* Improved OCIO profile formatting
* GCC visibility used (when available) to hide private symbols
* A few bug fixes


-- Jeremy
Re: Review: FileTransform will try all lut formats (if the initial read fails)

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

In the absence of any objections, I will check this into master.

-- Jeremy
Review: FileTransform will try all lut formats (if the initial read fails)

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

FileTransform will attempt to load the specified lut formats, if the
initial read fails. I also had to make some of the 3D lut loaders use
stricter parsing as they now get called (in error conditions) and
giving false positives. (I.e., an invalid .cube lut was errantly
being loaded by the .spi3d / .3dl loaders).

commits:
b6aef8748d6cd017e2dcf8e8d2baf8e9334eaa6e
91515b537187a8aa04cd389ea5036af65e0feb72
18bbb2734a67e442bb67b64f5f2b902ce578484b

-- Jeremy
Re: Review: Lut1D optimization

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

That's fair to add it as an option. I'll add it as an official TODO,
and will implement it once we have a few more examples of this type.

I'm not sure if this will be a transform specific option, or a
configuration level setting. (I could forsee if this comes up often
to have an optimization-level hint globally, which other blocks would
obey as well). I think if we're going to allow for customized
optimization levels, having one big switch that controlled all similar
settings would be much simpler to understand.

-- Jeremy

On Thu, Nov 11, 2010 at 3:25 PM, Rod Bogart <bog...@gmail.com> wrote:
I guess my preference would be to allow someone to ask for
optimization or not.  In the glorious future when OCIO is involved in
the creation of every pixel in every movie, the animated CTL example
may be rather common, and an option to avoid clever internal
heuristics might be a good debugging and processing tool.

RGB

On Wed, Nov 10, 2010 at 4:15 PM, Jeremy Selan <jeremy...@gmail.com> wrote:
Man, I can't get anything past you guys!   :)

The clamp was omitted on purpose, but I could probably be convinced
either way on this issue.*

If this code were inside a compiler, there would be no discussion -
clamp would be the only correct answer.

But ... in the context of writing a color processing library I would
propose that we have a bit of leeway here.  I tend to have a strong
distaste for proactively clamping outputs. Why destroy data if you
don't need to?  Who is to say that the extra data (previously
scheduled to be clamped) couldn't prove useful further down the
pipeline in certain circumstances?   Clamping is easy to add
downstream if/when needed.  I.e., if a subsequent image processing
operation depends on a clamped input in order to produce sensible
results, it can can always do it at that time.  And if it's not
needed, even better!

Our implementation of ASC CDL SOP already does this.

// out = clamp( (in * slope) + offset ) ^ power
// Note: the clamping portion of the CDL is only applied if
// a non-identity power is specified.

The same rationale applies.   The CDL transformation math strictly
requires a clamp between [0,1], making it useless on HDR data.
However, in practice many portions of the math (gain, sat) work great
on HDR data so it seems unnecessary to always clamp the output.  We
thus only clamp if the exponent potion is specified, which was the
intent as specified in the CDL docs).


The downside of this approach (in both the lut1d case, and the cdl
case) is that if someone ends up relying on either of these behaviors,
they are in a semi-precarious position.  I.e., their configuration is
sitting with magic numbers at certain values that happen to not
introduce this clamping, and this behavior will probably be
discontinuous (unintuitive?) over the parameter space.   Picture an
animating CDL exponent, where it goes from [0.95, 1.05].  For some
portion of the range there will be clamping, then no clamping, then
clamping again.

I agree this is not ideal, and that a sensible individual may prefer
the alternative, but this downside is a price I'm inclined to pay for
the benefit of preserving data in the commoner cases.

-- Jeremy


*(Technically the clamped range is not 0,1 but instead
[lut1d.from_min, lut1d.from_max] but this is inconsequential to the
overall point).


On Wed, Nov 10, 2010 at 3:14 PM, Rod Bogart <bog...@gmail.com> wrote:
Is this semantically correct if the presence of the 1D lut also
triggers clipping to 0-1?  Is there now a missing clamp?

RGB

On Wed, Nov 10, 2010 at 1:56 PM, Jeremy Selan <jeremy...@gmail.com> wrote:
On Lut loading, 1D luts are checked to determine if the underlying
transform represents 'identity'.  If so, they are skipped during op
generation.  This is particularly useful when working with 3D luts
that have shaper luts, which are often left as identity.

This check is added to the lut1d-finalize call, which takes a
'tolerance' as an arg as the underlying file quantization is file
dependent.

Commits:
177bf402f6ff95ba70224aa4466e7773c5a89f96

-- Jeremy
Review: .3dl loading updates

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

* Simplified the .3dl loading code
* Full support for .3dl shaper luts.

Commits:
76e11f2d894856142c6d96a6d0d317e911880993
f8fe2d5af218154189ab46830afc3d78e31c91ac
0176f8e96c24f06b0a80b8c78539087946e30af5

-- Jeremy
Review: Simplified .csp lut3d loading

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

Looking again at 3d lut index ordering, .csp files can be loaded
without any float reordering - none of the prior index mojo is
required. This simplifies the code quite a bit, results are identical
to prior implementation.

Commits:
eac3e125f910d7bc641d59c18c59ffd72ff985d4

-- Jeremy
Re: Review: Config.roles data storage is map

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

Yup, sending the map directly to the yaml stream works great.
Thanks.

-- Jeremy

On Thu, Nov 11, 2010 at 4:38 PM, Malcolm Humphreys
<malcolmh...@mac.com> wrote:
LGTM

I'm pretty sure yaml-cpp supports std::map serialization does the following
work?
sorry I can't test this myself.

--snip--
            // Roles
            if(m_impl->roles_.size() > 0)
            {
                out << YAML::Newline;
                out << YAML::Key << "roles" << YAML::Value;
                out << YAML::Value << m_impl->roles_; // std::map -> Map
            }
--snip--

.malcolm

On 10 Nov, 2010,at 11:22 AM, Jeremy Selan <jerem...@gmail.com> wrote:

The list of defined roles within a config previously maintained order.
Conceptually, this is actually a mapping type where order does not
matter (and the yaml serialization dropped ordering). This switches
the underlying storage to use a map instead.

Commits:
3942e6d8bd72b721b23d3e9991e434f2537a2759
Review: Added support for Iridas .cube luts (1d + 3d)

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

Added full read support for Iridas .cube luts. Verified results agree
with Nuke for all luts in FrameCycler distro.

commits:
1880192d63718dfba54d96d88ff396a0ac85fdee

-- Jeremy
Re: Review: Config.roles data storage is map

Malcolm Humphreys <malcolmh...@...>
 

LGTM

I'm pretty sure yaml-cpp supports std::map serialization does the following work?
sorry I can't test this myself.

--snip--
            // Roles
            if(m_impl->roles_.size() > 0)
            {
                out << YAML::Newline;
                out << YAML::Key << "roles" << YAML::Value;
                out << YAML::Value << m_impl->roles_; // std::map -> Map
            }
--snip--

.malcolm

On 10 Nov, 2010,at 11:22 AM, Jeremy Selan <jere...@...> wrote:

The list of defined roles within a config previously maintained order.
Conceptually, this is actually a mapping type where order does not
matter (and the yaml serialization dropped ordering). This switches
the underlying storage to use a map instead.

Commits:
3942e6d8bd72b721b23d3e9991e434f2537a2759
Re: Review: Switched auto_ptr -> ptr

Malcolm Humphreys <malcolmh...@...>
 

LGTM

.malcolm

On 06 Nov, 2010,at 11:24 AM, Jeremy Selan <jere...@...> wrote:

In the public API we were storing references to m_impl using autoptrs,
which really was unnecessary This only saved one line of code per
use, and made the memory model less obvious. This doesnt affect user
code at all, as it's impl related anyways.

Commits:
089aaa9297dfedffa59c2998bf4111cd65d53c5d

-- Jeremy
Re: Review: Inital pass at searchpath impl

Malcolm Humphreys <malcolmh...@...>
 

On 09 Nov, 2010,at 06:53 AM, Jeremy Selan <jere...@...> wrote:

* I like supporting only strings for now. In the future, I can also
see numbers being supported (such as allowing for CDL blocks that rely
on envvars), but this can be added as needed.
 
+1

* The FileTransform will probably have to get a bit smarter. Say
you're using a pershot lut, and for some shots you dont want any such
table. This could be solved by either not installing a lut in the
expected location, or by installing an identity lut. Which is
preferable? In the current implementation, if a lut is not found
this is always an error condition. Maybe we should introduce on the
FilmTransform a 'silent error mode', where this would just skip the
application of that specific LUT?
 
I was tempted to always have a lut that would match in the searchpath, say a global identity lut.
As this is possible to circumvent this way I would fix this later. The identity lut option should be
optimized out so it seems like a good option. But some explicit flag saying apply this lut when
the file is missing seems like a good way to try first.

* The "globals" pre-declaration could also define an optional default
(used if the envvar is not set). If the envvar is not set, and no
default is specified, any references to the global variable would
probably be an error.
 
+1

* I have some apprehensions about the config->getGlobal,
config->setGlobal implementation. My primary concern is that the
GlobalConfig() is const (by design), so clients couldnt actually call
setGlobals on the config in use. Instead, they would have to do
something like:

c = GetGlobalConfig().createEditableCopy()
c.setGlobals(...)
SetGlobalConfig( c )

This is a lot of copying, particularly as the globals often are tied
to the clip. If you're A/B-ing two clips in an image viewer,
conceptually there really is only one config, and two sets of globals.
Why force the client to copy and have duplicate configs?
 
What are your thoughts on having a config per clip and for each config to maintain each state.
I know this breaks the current concept of the config. Would be nice to re-confirm this is the way
we should go.

.malcolm
Re: Review: EnvExpand optimization

Malcolm Humphreys <malcolmh...@...>
 

LGTM

On 10 Nov, 2010,at 11:20 AM, Jeremy Selan <jere...@...> wrote:

Minor optimization: the potentially expensive EnvExpand call (which
expands envvars in file paths), does an early exit for paths which do
not contain special characters. ($ + %)

Commits:
38d53d46239ec3f7f090509f2273edbedd703d7e

-- Jeremy
Re: Review: CSP bugfix

Malcolm Humphreys <malcolmh...@...>
 

LGTM, sorry I did know about this I was in the middle of doing the spline op for the prelut section and waiting for a better name for GetAutodeskLut3DArrayOffset which
we were discussing in one of the threads about the houdini lut reader before fixing this.

I'll look into making the houdini reader match when I am back.

.malcolm


On 06 Nov, 2010,at 11:33 AM, Jeremy Selan <jere...@...> wrote:

Upon further testing, it appears the channel swizzling was backwards
during CSP lut importing.

I used Nuke as the baseline in terms of lut channel order, so it's
possible that I've actually broken things if Nuke's CSP Vector node is
broken. But if Nuke's works, OCIO's now agrees with the results.
Can anyone confirm Nuke's Vectorfield implementation of .csp files
works?

I am including the file I used for testing. This gains down the red
channel by 0.5 - green and blue are unchanged.

Commits:
d483d6a3660cec5cafa6868294f3cb62c05f4472

I've confirmed that our 3dl handing is correct.

-- Jeremy
Re: Review: Disable envvar unit tests

Malcolm Humphreys <malcolmh...@...>
 

hmm thats a bit weird the test should be creating all the files it is looking for.

--snip--
std::string lut2(two_dir+"somelut2.lut");
std::ofstream somelut2(lut2c_str());
somelut2.close();
--snip--

I'm guessing OCIO_TEST_AREA env isn't being set correctly, i'll check it out when I am back. But I'm guessing this might change with the work you are doing with the globals.

.malcolm


On 06 Nov, 2010,at 11:22 AM, Jeremy Selan <jere...@...> wrote:

Unit tests for envvars were failing, referencing files that weren't
checked into the source tree. Disabling these specific tests for now.

Commits:
0a052d9090c9d693f63cdb409260627406790d75

-- Jeremy
Review: Updated Nuke OCIOFileTransform

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

Added explicit control of transform direction + interpolation to the
Nuke OCIOFileTransform node.

Commits:
3143ccdb38fee9ec774a1be94150c8f04d9e70c5

-- Jeremy
Re: Review: Lut1D optimization

Rod Bogart <bog...@...>
 

I guess my preference would be to allow someone to ask for
optimization or not. In the glorious future when OCIO is involved in
the creation of every pixel in every movie, the animated CTL example
may be rather common, and an option to avoid clever internal
heuristics might be a good debugging and processing tool.

RGB

On Wed, Nov 10, 2010 at 4:15 PM, Jeremy Selan <jeremy...@gmail.com> wrote:
Man, I can't get anything past you guys!   :)

The clamp was omitted on purpose, but I could probably be convinced
either way on this issue.*

If this code were inside a compiler, there would be no discussion -
clamp would be the only correct answer.

But ... in the context of writing a color processing library I would
propose that we have a bit of leeway here.  I tend to have a strong
distaste for proactively clamping outputs. Why destroy data if you
don't need to?  Who is to say that the extra data (previously
scheduled to be clamped) couldn't prove useful further down the
pipeline in certain circumstances?   Clamping is easy to add
downstream if/when needed.  I.e., if a subsequent image processing
operation depends on a clamped input in order to produce sensible
results, it can can always do it at that time.  And if it's not
needed, even better!

Our implementation of ASC CDL SOP already does this.

// out = clamp( (in * slope) + offset ) ^ power
// Note: the clamping portion of the CDL is only applied if
// a non-identity power is specified.

The same rationale applies.   The CDL transformation math strictly
requires a clamp between [0,1], making it useless on HDR data.
However, in practice many portions of the math (gain, sat) work great
on HDR data so it seems unnecessary to always clamp the output.  We
thus only clamp if the exponent potion is specified, which was the
intent as specified in the CDL docs).


The downside of this approach (in both the lut1d case, and the cdl
case) is that if someone ends up relying on either of these behaviors,
they are in a semi-precarious position.  I.e., their configuration is
sitting with magic numbers at certain values that happen to not
introduce this clamping, and this behavior will probably be
discontinuous (unintuitive?) over the parameter space.   Picture an
animating CDL exponent, where it goes from [0.95, 1.05].  For some
portion of the range there will be clamping, then no clamping, then
clamping again.

I agree this is not ideal, and that a sensible individual may prefer
the alternative, but this downside is a price I'm inclined to pay for
the benefit of preserving data in the commoner cases.

-- Jeremy


*(Technically the clamped range is not 0,1 but instead
[lut1d.from_min, lut1d.from_max] but this is inconsequential to the
overall point).


On Wed, Nov 10, 2010 at 3:14 PM, Rod Bogart <bog...@gmail.com> wrote:
Is this semantically correct if the presence of the 1D lut also
triggers clipping to 0-1?  Is there now a missing clamp?

RGB

On Wed, Nov 10, 2010 at 1:56 PM, Jeremy Selan <jeremy...@gmail.com> wrote:
On Lut loading, 1D luts are checked to determine if the underlying
transform represents 'identity'.  If so, they are skipped during op
generation.  This is particularly useful when working with 3D luts
that have shaper luts, which are often left as identity.

This check is added to the lut1d-finalize call, which takes a
'tolerance' as an arg as the underlying file quantization is file
dependent.

Commits:
177bf402f6ff95ba70224aa4466e7773c5a89f96

-- Jeremy
Re: Review: Lut1D optimization

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

Man, I can't get anything past you guys! :)

The clamp was omitted on purpose, but I could probably be convinced
either way on this issue.*

If this code were inside a compiler, there would be no discussion -
clamp would be the only correct answer.

But ... in the context of writing a color processing library I would
propose that we have a bit of leeway here. I tend to have a strong
distaste for proactively clamping outputs. Why destroy data if you
don't need to? Who is to say that the extra data (previously
scheduled to be clamped) couldn't prove useful further down the
pipeline in certain circumstances? Clamping is easy to add
downstream if/when needed. I.e., if a subsequent image processing
operation depends on a clamped input in order to produce sensible
results, it can can always do it at that time. And if it's not
needed, even better!

Our implementation of ASC CDL SOP already does this.

// out = clamp( (in * slope) + offset ) ^ power
// Note: the clamping portion of the CDL is only applied if
// a non-identity power is specified.

The same rationale applies. The CDL transformation math strictly
requires a clamp between [0,1], making it useless on HDR data.
However, in practice many portions of the math (gain, sat) work great
on HDR data so it seems unnecessary to always clamp the output. We
thus only clamp if the exponent potion is specified, which was the
intent as specified in the CDL docs).


The downside of this approach (in both the lut1d case, and the cdl
case) is that if someone ends up relying on either of these behaviors,
they are in a semi-precarious position. I.e., their configuration is
sitting with magic numbers at certain values that happen to not
introduce this clamping, and this behavior will probably be
discontinuous (unintuitive?) over the parameter space. Picture an
animating CDL exponent, where it goes from [0.95, 1.05]. For some
portion of the range there will be clamping, then no clamping, then
clamping again.

I agree this is not ideal, and that a sensible individual may prefer
the alternative, but this downside is a price I'm inclined to pay for
the benefit of preserving data in the commoner cases.

-- Jeremy


*(Technically the clamped range is not 0,1 but instead
[lut1d.from_min, lut1d.from_max] but this is inconsequential to the
overall point).

On Wed, Nov 10, 2010 at 3:14 PM, Rod Bogart <bog...@gmail.com> wrote:
Is this semantically correct if the presence of the 1D lut also
triggers clipping to 0-1?  Is there now a missing clamp?

RGB

On Wed, Nov 10, 2010 at 1:56 PM, Jeremy Selan <jeremy...@gmail.com> wrote:
On Lut loading, 1D luts are checked to determine if the underlying
transform represents 'identity'.  If so, they are skipped during op
generation.  This is particularly useful when working with 3D luts
that have shaper luts, which are often left as identity.

This check is added to the lut1d-finalize call, which takes a
'tolerance' as an arg as the underlying file quantization is file
dependent.

Commits:
177bf402f6ff95ba70224aa4466e7773c5a89f96

-- Jeremy
Re: Review: Lut1D optimization

Rod Bogart <bog...@...>
 

Is this semantically correct if the presence of the 1D lut also
triggers clipping to 0-1? Is there now a missing clamp?

RGB

On Wed, Nov 10, 2010 at 1:56 PM, Jeremy Selan <jeremy...@gmail.com> wrote:
On Lut loading, 1D luts are checked to determine if the underlying
transform represents 'identity'.  If so, they are skipped during op
generation.  This is particularly useful when working with 3D luts
that have shaper luts, which are often left as identity.

This check is added to the lut1d-finalize call, which takes a
'tolerance' as an arg as the underlying file quantization is file
dependent.

Commits:
177bf402f6ff95ba70224aa4466e7773c5a89f96

-- Jeremy
Review: Lut1D optimization

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

On Lut loading, 1D luts are checked to determine if the underlying
transform represents 'identity'. If so, they are skipped during op
generation. This is particularly useful when working with 3D luts
that have shaper luts, which are often left as identity.

This check is added to the lut1d-finalize call, which takes a
'tolerance' as an arg as the underlying file quantization is file
dependent.

Commits:
177bf402f6ff95ba70224aa4466e7773c5a89f96

-- Jeremy
1921 - 1940 of 2191