Re: FileTransform interface changes


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


But considering that commercial apps are chomping at the bit to
release OCIO 1.0 binaries, and that we've promised an absolutely ready
to ship '1.0' just 2 weeks from now, I think it's too late in the game
to be considering non-trivial API changes. <tear/> Would you
concur? My gut tells me this is too much API design work to flesh out
in too short a time.
 
I leave it up to you to decide what makes it into 1.0. At the end of the day I would just be happy to somehow replace these:

FileTransform::getCCCId()
FileTransform::setCCCId(const char * id)

But I think for 2.0 a full blown solution would be a great addition.

Could we find a halfway point?
FileTransform::getOptionString()
FileTransform::setOptionString(const char * id)

This could take a 'token1=value1;token2=value2' string. This is a bit ugly but does allow for the interface to be not so format specific while also allowing to add other options if they are needed for fixing bugs on the path to 2.0.

eg.
FileTransform::setOptionString("CCCId=myfavshot;");

Of course I'm very open to considering such a change as a part of a
1.2 or 2.0 API. (As it wouldnt change the .ocio format, 1.2 is
possible). To do this issue 'right' we'd potentially want to give the
same treatment to all transform types, not just FileTransform. (As a
side effect all the static typed transforms would evaporate into a
single Transform with getType() !) See this #issue (from last week)
where we spec'd out some of the details.
https://github.com/imageworks/OpenColorIO/issues/155
 
 
I think that is a great idea and should be 2.0 as it's a big interface change. I like it even more after trying to wrap all the transforms types which is a hell of lot of boilerplate code.. (grumble)

So if you'd concur, let's go ahead and just add set/get
ShaperInterpolation in the meantime? What's the use case for
hasShaper? Is that for interface visibility?
 
yep just for the ui.

In terms of ICCTransform support, I have mixed feelings. First off, I
totally understand the use case, and I'm completely committed to
solving that workflow with OCIO's 'ecosystem'.
 
I would hope this would be just a straight FileTransform rather than a separate *Transform.

So do essentially would have to *require* core littlecms / icc support
to consider it. Are we willing to make that jump at this point? Is
OCIO's ICCTransfrom ready to ship? (and be locked down?).
https://github.com/imageworks/OpenColorIO/pull/87
 
 
I was more thinking of being able to add it in a 1.xx minor version rather than it being ready for 1.0.

I think my preference would be in the medium term, to add an OCIO API
where transforms such as ICC / Truelight / CTL could be built
externally - as needed - without introducing the build-time
dependency. Supporting apps wouldnt need to ship with these optional
plugins - the uses would compile them as required.
 
Well I'm all for that as it's originally what I suggested :) it would be good if this API also extended to adding additional file formats.
I would hope the long term goal would be to roll new formats into the core but this would allow adding/modifying existing support.

In the meantime, a compromise (suitable for 1.0.0) could be to add an
additional external app (or part of ocio2icc?) that handles the
functionality of ICCTransform, and bakes the result into a 3dlut
suitable for OCIO. That way folks who rely on ICC calibration would
have a command to generate the needed 3d display luts - it just would
happen as a preprocess, and not at runtime.
 
Sure, you kind can do that now as Ben has just posted so it's not really a big issue for 1.0 for me.

.malcolm

On Wed, Sep 14, 2011 at 7:00 AM, Malcolm Humphreys
<malcolmh...@...> wrote:
> Hi,
> As we are fast approaching 1.0 I know it's a bit late in the day for some
> API changes. But I noticed these while doing the JNI wrapping for the
> android port some things which would be really good to change before 1.0.
> Could we add the following to control the interpolation of the shaper lut
> Interpolation FileTransform::getShaperInterpolation() const;
> void FileTransform::setShaperInterpolation(Interpolation interp);
> bool FileTransform::hasShaper();
> Also these seem a bit smelly and very format specific in the current
> interface.
> FileTransform::getCCCId();
> FileTransform::setCCCId(const char * id);
> Could this be replaced with some kind of key, type, value interface.
> Maybe along the lines of (I would think the order of vars/parameters would
> be important for ui's which is why it's all index based):
> int FileTransform::getNumVars();
> VarType FileTransform::getVarType(int index);
> const char * FileTransform::getVarName(int index);
> int FileTransform::getVarIndex(const char * name);
> void FileTransform::setStringVar(int index, const char * value);
> const char * FileTransform::getStringVar(int index);
> void FileTransform::setFloatVar(int index, float value);
> float FileTransform::getFloatVar(int index);
> enum VarType
> {
>         VAR_TYPE_NONE = 0,
>         VAR_TYPE_STRING = 1,
>         VAR_TYPE_FLOAT = 2
> };
> eg.
> --snip--
> int index = ptr->getVarIndex("CCCId");
> if(ptr->getVarType(index) == VAR_TYPE_STRING)
> ptr->setStringVar(, "MYSHOTCCC");
> --snip--
> This would allow me to move the ICCTransfrom code into a FileTransform impl
> which would be nice.
> If your down with the idea I'm happy to whip something up when I have
> sometime after work.
> .malcolm
>

Join ocio-dev@lists.aswf.io to automatically receive all group messages.