Review: First pass at TruelightTransform and TruelightOp


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

I think you can add additional pull requests if they are on uniquely named 'topic' branches.

How interesting! - a Truelight Transform...

Only quickly glancing at the code (always dangerous), does this add Truelight as an optional or required dependency?   I agree that this would be better as a plugin, but the runtime implications of plugins vs. optional compile-time functionality are essentially the same. And in this case, as we at SPI do not actively use Truelight, we'll probably want to address the compatibility implications in the short-term.

What's the advantage of using this plugin as opposed to - say - baking it into a baked 3d lut representation at ocio profile generation time?  Would you expect the end user (artist) to dynamically modify the input arguments?   If not, and the transforms inputs are essentially locked for each configuration, what would you think of adding this functionality not to the core library but instead to some of the helper apps?

You obviously implemented this with a workflow in mind, would you elaborate?

My largest compatibility concern is that in the near future, commercial apps will be shipping with native OCIO support. (Yay!)  But, this native support won't compile with Truelight functionality. So we'll essentially have a schism in support, where some people's internal pipelines support ocio+truelight, while the common commercial apps only support ocio-plain.

An OCIO transform plugin API would obviously help to alleviate this issue, but both implements open the pandora's box of profile compatibility issues.  Once plugins (or optional build dependencies) are in use .ocio profiles are no longer generically interchangeable between facilities. You would have to share the profiles, all plugins, potentially plugin source, etc...   And then one would have to consider the licensing aspect as well.

In the current system if you create a valid profile you know it really can be used in any app, on any OS, and easily shared (emailed!) to other vendors.

-- Jeremy

On Tue, Feb 22, 2011 at 8:14 PM, Malcolm Humphreys <malcolmh...@...> wrote:
Can't seem to send more than one pull request while one is in the queue.

https://github.com/malcolmhumphreys/OpenColorIO/commit/c1abbcb4061cd55461d90dfb29a41cbf0989fc20

I can send another pull request once the Baker stuff makes it in. This transform is one that I would have added as a plugin.

.malcolm



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

Hi,

I think you can add additional pull requests if they are on uniquely named 'topic' branches.

How interesting! - a Truelight Transform...

Only quickly glancing at the code (always dangerous), does this add Truelight as an optional or required dependency?  

Optional link dependancy, but all the profile serialisation will work the same all the time. You will only get an exception thrown when creating a processor with a truelight transform but have no support. If host apps are written correctly this shouldn't cause problems.

I agree that this would be better as a plugin, but the runtime implications of plugins vs. optional compile-time functionality are essentially the same. And in this case, as we at SPI do not actively use Truelight, we'll probably want to address the compatibility implications in the short-term.

What's the advantage of using this plugin as opposed to - say - baking it into a baked 3d lut representation at ocio profile generation time?  Would you expect the end user (artist) to dynamically modify the input arguments?   If not, and the transforms inputs are essentially locked for each configuration, what would you think of adding this functionality not to the core library but instead to some of the helper apps?

Depending on how many display devices are in play it's nice to have this dynamic. But most of the time the transform will be used in defining a colorspace to be used with ociobakelut into different lut formats.

You obviously implemented this with a workflow in mind, would you elaborate?

From time to time new display devices come into play and truelight can be used for device -> device mapping eg. a whole heap of diffrent monitors turnup halfway through a production, which need to look the same/similar to the current set.

I prefer that all the tools for modelling these transforms and baking luts are all in one place.

I really want a ocio profile to describe all the color decisions on a show + ways to regenerate luts formats even ones that the ocio profile uses. 

My largest compatibility concern is that in the near future, commercial apps will be shipping with native OCIO support. (Yay!)  But, this native support won't compile with Truelight functionality. So we'll essentially have a schism in support, where some people's internal pipelines support ocio+truelight, while the common commercial apps only support ocio-plain.

I have written it so that you can have profiles with the <!TruelightTransform> even if OCIO hasn't been built with it. In most host apps this wont be a problem as these wouldn't be used directly in production display lookups.

An OCIO transform plugin API would obviously help to alleviate this issue, but both implements open the pandora's box of profile compatibility issues.  Once plugins (or optional build dependencies) are in use .ocio profiles are no longer generically interchangeable between facilities. You would have to share the profiles, all plugins, potentially plugin source, etc...   And then one would have to consider the licensing aspect as well.

Agreed, I think this is a happy middle ground. Profiles are still interoperable while opening up the opportunity for 3rdParty OCIO enabled commercial apps to get truelight support if they wanted it.

As I'm guessing Ben has access to the cinespace sdk so it would also be possible to add a <!CineSpaceTransform> as well in the same spirit as this one.

...

I did say I wanted a plugin interface :)

.malcolm

In the current system if you create a valid profile you know it really can be used in any app, on any OS, and easily shared (emailed!) to other vendors.

-- Jeremy

On Tue, Feb 22, 2011 at 8:14 PM, Malcolm Humphreys <malcolmh...@...> wrote:
Can't seem to send more than one pull request while one is in the queue.

https://github.com/malcolmhumphreys/OpenColorIO/commit/c1abbcb4061cd55461d90dfb29a41cbf0989fc20

I can send another pull request once the Baker stuff makes it in. This transform is one that I would have added as a plugin.

.malcolm




"dbr/Ben" <b...@...>
 

When building from the current imageworks HEAD revision (ce75762bdfaec287db029986cdc17d6f136b7447), I get:

    src/core/TruelightOp.cpp: In member function ‘virtual OpenColorIO::v0::OpRcPtr OpenColorIO::v0::<unnamed>::TruelightOp::clone() const’:
    src/core/TruelightOp.cpp:191: error: cannot allocate an object of abstract type ‘OpenColorIO::v0::<unnamed>::TruelightOp’
    src/core/TruelightOp.cpp:49: note:   because the following virtual functions are pure within ‘OpenColorIO::v0::<unnamed>::TruelightOp’:
    src/core/Op.h:102: note: virtual void OpenColorIO::v0::Op::writeGpuShader(std::ostream&, const std::string&, const OpenColorIO::v0::GpuShaderDesc&) const
    src/core/TruelightOp.cpp: In function ‘void OpenColorIO::v0::CreateTruelightOps(OpenColorIO::v0::OpRcPtrVec&, const OpenColorIO::v0::TruelightTransform&, OpenColorIO::v0::TransformDirection)’:
    src/core/TruelightOp.cpp:385: error: cannot allocate an object of abstract type ‘OpenColorIO::v0::<unnamed>::TruelightOp’
    src/core/TruelightOp.cpp:49: note:   since type ‘OpenColorIO::v0::<unnamed>::TruelightOp’ has pure virtual functions
    make[2]: *** [src/core/CMakeFiles/OpenColorIO.dir/TruelightOp.cpp.o] Error 1
    make[1]: *** [src/core/CMakeFiles/OpenColorIO.dir/all] Error 2

Using OS X 10.6.6 with GCC 4.2.1, and running "cd build && cmake ../ && make" (Truelight isn't installed/enabled) - I'm quite possibly doing something stupid, but the error seemed bug'y

As I'm guessing Ben has access to the cinespace sdk so it would also be possible to add a <!CineSpaceTransform> as well in the same spirit as this one.

...hm, I do indeed. Might have a look at this when I have a bit of spare time - could be useful in conjuction with the Houdini LUT Baker! Should be straight-forward enough, although last time I done something with cineSpaceAPI, I lost several days to an incredibly obscure linker bug *mutters*


On 23/02/2011, at 6:03 PM, Malcolm Humphreys wrote:

Hi,

I think you can add additional pull requests if they are on uniquely named 'topic' branches.

How interesting! - a Truelight Transform...

Only quickly glancing at the code (always dangerous), does this add Truelight as an optional or required dependency?  

Optional link dependancy, but all the profile serialisation will work the same all the time. You will only get an exception thrown when creating a processor with a truelight transform but have no support. If host apps are written correctly this shouldn't cause problems.

I agree that this would be better as a plugin, but the runtime implications of plugins vs. optional compile-time functionality are essentially the same. And in this case, as we at SPI do not actively use Truelight, we'll probably want to address the compatibility implications in the short-term.

What's the advantage of using this plugin as opposed to - say - baking it into a baked 3d lut representation at ocio profile generation time?  Would you expect the end user (artist) to dynamically modify the input arguments?   If not, and the transforms inputs are essentially locked for each configuration, what would you think of adding this functionality not to the core library but instead to some of the helper apps?

Depending on how many display devices are in play it's nice to have this dynamic. But most of the time the transform will be used in defining a colorspace to be used with ociobakelut into different lut formats.

You obviously implemented this with a workflow in mind, would you elaborate?

From time to time new display devices come into play and truelight can be used for device -> device mapping eg. a whole heap of diffrent monitors turnup halfway through a production, which need to look the same/similar to the current set.

I prefer that all the tools for modelling these transforms and baking luts are all in one place.

I really want a ocio profile to describe all the color decisions on a show + ways to regenerate luts formats even ones that the ocio profile uses. 

My largest compatibility concern is that in the near future, commercial apps will be shipping with native OCIO support. (Yay!)  But, this native support won't compile with Truelight functionality. So we'll essentially have a schism in support, where some people's internal pipelines support ocio+truelight, while the common commercial apps only support ocio-plain.

I have written it so that you can have profiles with the <!TruelightTransform> even if OCIO hasn't been built with it. In most host apps this wont be a problem as these wouldn't be used directly in production display lookups.

An OCIO transform plugin API would obviously help to alleviate this issue, but both implements open the pandora's box of profile compatibility issues.  Once plugins (or optional build dependencies) are in use .ocio profiles are no longer generically interchangeable between facilities. You would have to share the profiles, all plugins, potentially plugin source, etc...   And then one would have to consider the licensing aspect as well.

Agreed, I think this is a happy middle ground. Profiles are still interoperable while opening up the opportunity for 3rdParty OCIO enabled commercial apps to get truelight support if they wanted it.

As I'm guessing Ben has access to the cinespace sdk so it would also be possible to add a <!CineSpaceTransform> as well in the same spirit as this one.

...

I did say I wanted a plugin interface :)

.malcolm

In the current system if you create a valid profile you know it really can be used in any app, on any OS, and easily shared (emailed!) to other vendors.

-- Jeremy

On Tue, Feb 22, 2011 at 8:14 PM, Malcolm Humphreys <malcolmh...@...> wrote:
Can't seem to send more than one pull request while one is in the queue.

https://github.com/malcolmhumphreys/OpenColorIO/commit/c1abbcb4061cd55461d90dfb29a41cbf0989fc20

I can send another pull request once the Baker stuff makes it in. This transform is one that I would have added as a plugin.

.malcolm





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

Thanks for the error text, that was definitely a bug. (Introduced by me).   This is now fixed in the latest master branch, and I've updated the v0.7.7 tag.

-- Jeremy

On Wed, Mar 2, 2011 at 6:42 AM, dbr/Ben <b...@...> wrote:

When building from the current imageworks HEAD revision (ce75762bdfaec287db029986cdc17d6f136b7447), I get:

    src/core/TruelightOp.cpp: In member function ‘virtual OpenColorIO::v0::OpRcPtr OpenColorIO::v0::<unnamed>::TruelightOp::clone() const’:
    src/core/TruelightOp.cpp:191: error: cannot allocate an object of abstract type ‘OpenColorIO::v0::<unnamed>::TruelightOp’
    src/core/TruelightOp.cpp:49: note:   because the following virtual functions are pure within ‘OpenColorIO::v0::<unnamed>::TruelightOp’:
    src/core/Op.h:102: note: virtual void OpenColorIO::v0::Op::writeGpuShader(std::ostream&, const std::string&, const OpenColorIO::v0::GpuShaderDesc&) const
    src/core/TruelightOp.cpp: In function ‘void OpenColorIO::v0::CreateTruelightOps(OpenColorIO::v0::OpRcPtrVec&, const OpenColorIO::v0::TruelightTransform&, OpenColorIO::v0::TransformDirection)’:
    src/core/TruelightOp.cpp:385: error: cannot allocate an object of abstract type ‘OpenColorIO::v0::<unnamed>::TruelightOp’
    src/core/TruelightOp.cpp:49: note:   since type ‘OpenColorIO::v0::<unnamed>::TruelightOp’ has pure virtual functions
    make[2]: *** [src/core/CMakeFiles/OpenColorIO.dir/TruelightOp.cpp.o] Error 1
    make[1]: *** [src/core/CMakeFiles/OpenColorIO.dir/all] Error 2

Using OS X 10.6.6 with GCC 4.2.1, and running "cd build && cmake ../ && make" (Truelight isn't installed/enabled) - I'm quite possibly doing something stupid, but the error seemed bug'y

As I'm guessing Ben has access to the cinespace sdk so it would also be possible to add a <!CineSpaceTransform> as well in the same spirit as this one.

...hm, I do indeed. Might have a look at this when I have a bit of spare time - could be useful in conjuction with the Houdini LUT Baker! Should be straight-forward enough, although last time I done something with cineSpaceAPI, I lost several days to an incredibly obscure linker bug *mutters*


On 23/02/2011, at 6:03 PM, Malcolm Humphreys wrote:

Hi,

I think you can add additional pull requests if they are on uniquely named 'topic' branches.

How interesting! - a Truelight Transform...

Only quickly glancing at the code (always dangerous), does this add Truelight as an optional or required dependency?  

Optional link dependancy, but all the profile serialisation will work the same all the time. You will only get an exception thrown when creating a processor with a truelight transform but have no support. If host apps are written correctly this shouldn't cause problems.

I agree that this would be better as a plugin, but the runtime implications of plugins vs. optional compile-time functionality are essentially the same. And in this case, as we at SPI do not actively use Truelight, we'll probably want to address the compatibility implications in the short-term.

What's the advantage of using this plugin as opposed to - say - baking it into a baked 3d lut representation at ocio profile generation time?  Would you expect the end user (artist) to dynamically modify the input arguments?   If not, and the transforms inputs are essentially locked for each configuration, what would you think of adding this functionality not to the core library but instead to some of the helper apps?

Depending on how many display devices are in play it's nice to have this dynamic. But most of the time the transform will be used in defining a colorspace to be used with ociobakelut into different lut formats.

You obviously implemented this with a workflow in mind, would you elaborate?

From time to time new display devices come into play and truelight can be used for device -> device mapping eg. a whole heap of diffrent monitors turnup halfway through a production, which need to look the same/similar to the current set.

I prefer that all the tools for modelling these transforms and baking luts are all in one place.

I really want a ocio profile to describe all the color decisions on a show + ways to regenerate luts formats even ones that the ocio profile uses. 

My largest compatibility concern is that in the near future, commercial apps will be shipping with native OCIO support. (Yay!)  But, this native support won't compile with Truelight functionality. So we'll essentially have a schism in support, where some people's internal pipelines support ocio+truelight, while the common commercial apps only support ocio-plain.

I have written it so that you can have profiles with the <!TruelightTransform> even if OCIO hasn't been built with it. In most host apps this wont be a problem as these wouldn't be used directly in production display lookups.

An OCIO transform plugin API would obviously help to alleviate this issue, but both implements open the pandora's box of profile compatibility issues.  Once plugins (or optional build dependencies) are in use .ocio profiles are no longer generically interchangeable between facilities. You would have to share the profiles, all plugins, potentially plugin source, etc...   And then one would have to consider the licensing aspect as well.

Agreed, I think this is a happy middle ground. Profiles are still interoperable while opening up the opportunity for 3rdParty OCIO enabled commercial apps to get truelight support if they wanted it.

As I'm guessing Ben has access to the cinespace sdk so it would also be possible to add a <!CineSpaceTransform> as well in the same spirit as this one.

...

I did say I wanted a plugin interface :)

.malcolm

In the current system if you create a valid profile you know it really can be used in any app, on any OS, and easily shared (emailed!) to other vendors.

-- Jeremy

On Tue, Feb 22, 2011 at 8:14 PM, Malcolm Humphreys <malcolmh...@...> wrote:
Can't seem to send more than one pull request while one is in the queue.

https://github.com/malcolmhumphreys/OpenColorIO/commit/c1abbcb4061cd55461d90dfb29a41cbf0989fc20

I can send another pull request once the Baker stuff makes it in. This transform is one that I would have added as a plugin.

.malcolm






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


As I'm guessing Ben has access to the cinespace sdk so it would also be possible to add a <!CineSpaceTransform> as well in the same spirit as this one.

...hm, I do indeed. Might have a look at this when I have a bit of spare time - could be useful in conjuction with the Houdini LUT Baker! Should be straight-forward enough, although last time I done something with cineSpaceAPI, I lost several days to an incredibly obscure linker bug *mutters*

If you can send me the header, I'm happy to flesh something out.

I didn't get around to it before I left DRD but I was hoping to use the lut baker to build a tool to take advantage of the lutio table which you might find interesting.


You could effectively have something like this.
.ocio "ocioToHoudini '%s' stdout.lut" "ocioFromHoudini stdin.lut '%s'"

If you did this you could support per shot dynamic grades, where you could feed houdini the ocio profile and it could spit out a lut baked for the current context. This would save rendering out graded plates or baking out luts for everyshot for lighting.

.malcolm

On 23/02/2011, at 6:03 PM, Malcolm Humphreys wrote:

Hi,

I think you can add additional pull requests if they are on uniquely named 'topic' branches.

How interesting! - a Truelight Transform...

Only quickly glancing at the code (always dangerous), does this add Truelight as an optional or required dependency?  

Optional link dependancy, but all the profile serialisation will work the same all the time. You will only get an exception thrown when creating a processor with a truelight transform but have no support. If host apps are written correctly this shouldn't cause problems.

I agree that this would be better as a plugin, but the runtime implications of plugins vs. optional compile-time functionality are essentially the same. And in this case, as we at SPI do not actively use Truelight, we'll probably want to address the compatibility implications in the short-term.

What's the advantage of using this plugin as opposed to - say - baking it into a baked 3d lut representation at ocio profile generation time?  Would you expect the end user (artist) to dynamically modify the input arguments?   If not, and the transforms inputs are essentially locked for each configuration, what would you think of adding this functionality not to the core library but instead to some of the helper apps?

Depending on how many display devices are in play it's nice to have this dynamic. But most of the time the transform will be used in defining a colorspace to be used with ociobakelut into different lut formats.

You obviously implemented this with a workflow in mind, would you elaborate?

From time to time new display devices come into play and truelight can be used for device -> device mapping eg. a whole heap of diffrent monitors turnup halfway through a production, which need to look the same/similar to the current set.

I prefer that all the tools for modelling these transforms and baking luts are all in one place.

I really want a ocio profile to describe all the color decisions on a show + ways to regenerate luts formats even ones that the ocio profile uses. 

My largest compatibility concern is that in the near future, commercial apps will be shipping with native OCIO support. (Yay!)  But, this native support won't compile with Truelight functionality. So we'll essentially have a schism in support, where some people's internal pipelines support ocio+truelight, while the common commercial apps only support ocio-plain.

I have written it so that you can have profiles with the <!TruelightTransform> even if OCIO hasn't been built with it. In most host apps this wont be a problem as these wouldn't be used directly in production display lookups.

An OCIO transform plugin API would obviously help to alleviate this issue, but both implements open the pandora's box of profile compatibility issues.  Once plugins (or optional build dependencies) are in use .ocio profiles are no longer generically interchangeable between facilities. You would have to share the profiles, all plugins, potentially plugin source, etc...   And then one would have to consider the licensing aspect as well.

Agreed, I think this is a happy middle ground. Profiles are still interoperable while opening up the opportunity for 3rdParty OCIO enabled commercial apps to get truelight support if they wanted it.

As I'm guessing Ben has access to the cinespace sdk so it would also be possible to add a <!CineSpaceTransform> as well in the same spirit as this one.

...

I did say I wanted a plugin interface :)

.malcolm

In the current system if you create a valid profile you know it really can be used in any app, on any OS, and easily shared (emailed!) to other vendors.

-- Jeremy

On Tue, Feb 22, 2011 at 8:14 PM, Malcolm Humphreys <malcolmh...@...> wrote:
Can't seem to send more than one pull request while one is in the queue.

https://github.com/malcolmhumphreys/OpenColorIO/commit/c1abbcb4061cd55461d90dfb29a41cbf0989fc20

I can send another pull request once the Baker stuff makes it in. This transform is one that I would have added as a plugin.

.malcolm