channel math and INK keyer

[nicholascarroll]

@olear suggested i propose this merge.

[devernay]

Thanks for contributing!

ChannelMath: does it have any advantage over SeExpr, GMICExpr and Shadertoy? Do we really need another expression plugin?

INK Keyer: unless it is a well-documented (in terms of algorithm and usage documentation) and solid keyer, I would not include one more keyer (we have ChromaKeyer, HSVTool and Difference already). I would prefer adding something that brings new functionality, such as a keyer that works like IBK on Nuke. (note: despill and key amount do not seem to work in INK).

Also note that for all contributions to the Natron code and plugins (openfx-misc, openfx-io, openfx-arena), we ask the author to sign a CLA, as mentioned in https://github.com/MrKepzie/Natron/blob/master/CONTRIBUTING.md - unfortunately I don't see the CLA itself anywhere, but we will fix that. Basically, we ask contributors to keep property on the code they wrote (so they can do whatever they want with it), but to grant us a perpetual unlimited license (this is called Copyright License by Project Harmony http://www.harmonyagreements.org/guide.html ).

[sozap]

ChannelMath :
It's really cool to add a simple expression tool, it looks familiar to me to Nuke expression node.
Shadertoy and SeExpr are wonderful tools but they need a bit of time to get into , especially if you're not used to do code. Sometime when doing compositing you just want to do quickly things like "(R+B)/2" , if you never get into shadertoy or SeExpr before you'll probably end up by doing it with nodes instead of using a simple tool like this one.

INK Keyer: Maybe it would be cool to test both color keyer with same input pictures to see if one work better than the other... as it's a key feature for VFX , it may be worth looking into it... I can look into some comparative test if you like ?

[nicholascarroll]

Hi, happy to read the CLI with a view to signing it. Algorithm and usage doc is published at http://casanico.com/ink-user-guide.html. The doc source file is in the Emacs org-mode and Latex syntax, in the INK source directory. INK's Not Keylight, meaning it is inspired by Keylight, but it is not endorsed by the owners of Keylight trademark and does not have any permission or association with the Foundry.

[nicholascarroll]

About Channelmath I will highlight that execution speed is important and the library it uses, by Arash Partow, is fast. Last year when I checked it was significantly faster than SeExpr. However perhaps Disney has made recent efforts to optimize execution speed in SeExpr?

[nicholascarroll]

@devernay please accept or decline this one.

Just to answer your other concern:

note: despill and key amount do not seem to work in INK

Key Amount does work, but Despill Balance and Matte Balance do not and I can't promise I'll ever get around to them.

[devernay]

ChannelMath

a nice contribution, but I still do not see the point in including it when we already have SeExpr.

Is there a need for a more simple SeExpr with separate expressions for r,g,b,a and predefined r g b a scalars instead of accessing elements of Cs[]? This is easy, we can do it instead and avoid having a very different expression node.

besides, it has the following issues:

licence

It has to use another expression library because the CPL used by exprtk is not GPL compatible https://en.wikipedia.org/wiki/Common_Public_License
It's a share, because exptk seems to be vey fast indeed.

see https://github.com/ArashPartow/math-parser-benchmark-project
we prefer to avoid GPL-licenced dependencies, so muparser seems to be an OK choice

performance

• prepare the expressions only once (eg in render() or setupAndProcess()) rather than once for every thread (eg in process())

As for the INK keyer, since it's not yet fully functional and has some remaining bugs, I propose we don't integrate it yet. It would be better to have it as a "mode" in the existing ChromaKeyer, because of the large parts of code in common.

[nicholascarroll]

About the math node, yes the need is as you say, one field per channel, because that is what makes it very easy to use. But above all, it's very important that it be FAST. This is the feedback I got from Hollywood pros using it in Resolve. Write to Arash Partow and see if he will help you resolve your license concern, because ExprTk is supposed to be a very liberal license.

About INK, yes why not simply add it as a mode in net.sf.openfx.KeyerPlugin (not ChromaKeyer) because it's an important type of colour difference keyer - 'Proportionate'.

I also mention again that the names I have provided for inputs Core and Garbage are more descriptive than InM and OutM.

In the screenie below you can see that the proportionate equation makes for a more balanced result than the 'Screen' option.

[nicholascarroll]

@olear has added these to the new Natron community repo which i think is a better option, so closing this.

[devernay]

wdas/SeExpr#59

[devernay]

I think it's better to use the same short names for garbage matte / outside mask and core matte / holdout matte / inside mask in all keyer plugins.
There is now the possibility to document the clips using setHint()

[ArashPartow]

the CPL used by exprtk is not GPL compatible

I'd be happy to provide a Boost or MIT license for ExprTk for this project.

[devernay]

Thanks @ArashPartow, and thanks for asking the SeExpr guys for build details wdas/SeExpr#59

Since we already use https://github.com/wdas/SeExpr for the same purpose, I'm waiting for a benchmark (probably yours https://github.com/ArashPartow/math-parser-benchmark-project ) to see if we integrate ExprTk or stay with SeExpr.

[ArashPartow]

Hopefully I'm not out of line, I discovered this thread from a mention that @devernay made on another issue. I thought perhaps I could provide some general feedback on the pull with regards to ExprTk

---

Clamp Issue

Specifically relating to this code:
https://github.com/nicholascarroll/openfx-misc/blob/b9acf0642498d456e53ea7a5f542bc5853c44376/ChannelMath/ChannelMath.cpp#L418

In ExprTk any or all reserved base functions can be disabled and user defined versions can be instituted. As is described here

As an example the normal version of sine is replaced with one that takes degrees instead of radians:
https://github.com/ArashPartow/exprtk/blob/master/readme.txt#L2318

For performance reasons, rather than use a compositor, my suggestion would be to implement a user defined clamp function and register it with the designated symbol_table like so:

template <typename T>
struct ofx_clamp : public exprtk::ifunction<T>
{
   ofx_clamp()
   : exprtk::ifunction<T>(3)
   {}

   inline T operator()(const T& v, const T& r0, const T& r1)
   {
      if (v < r0)
         return r0;
      else if (v > r1)
         return r1
      else
         return v;
   }
};

....

{
   ofx_clamp<T> clamp;

   symbol_table_t symbol_table;
   symbol_table.add_reserved_function("clamp",clamp);
   ....
}

Which is based on example 05

So I don't think there needs to be any modifications made to the library itself.

---

Initialisation Issue

I could be wrong about this, but a cursory review of the method process(const OfxRectI& procWindow), seems to suggest none of the initialisation stuff that has to do with ExprTk (symbol_table, and expression compilations) is actually related to the current call with the instance of procWindow. So perhaps all of that stuff can be done only once somewhere in a setup or an init phase of the class.

If the expectation is to run the process method multiple times in a loop say, then the incurred overhead of having to perform that same setup and later-on tear down of the various components per call will most likely be enormous.

There is a note here

---

Multiple Symbol Tables

This is more something to think about. Exprtk expressions can have multiple symbol tables associated with them. The use-case is that there may be functions and variables (eg: ofx_clamp, pi etc) that should be available to all expressions, where as there may be functions and variables that should only be associated with the specific expression. As an example you might have something like this:

const T ofx_magic_no = 42.0;

symbol_table_t global_symbol_table;
global_symbol_table.add_reserved_function("clamp",clamp);
global_symbol_table.add_constant("ofx_magic_no",ofx_magic_no);

....

{
   T exp0_x = ...;

   symbol_table_t symbol_table;
   symbol_table.add_variable("x",exp0_x);

   expression_t exp0;
   exp0.register_symbol_table(global_symbol_table);
   exp0.register_symbol_table(symbol_table);
}

....

{
   T exp1_x = ...;

   symbol_table_t symbol_table;
   symbol_table.add_variable("x",exp1_x);

   expression_t exp1;
   exp1.register_symbol_table(global_symbol_table);
   exp1.register_symbol_table(symbol_table);
}

Some information related to multiple symbol tables can be found here.

[ArashPartow]

@devernay Yes I too would very much like to see SeExpr in the benchmark - specially the LLVM backed version. Would have hoped that it would be a little more easier and clearer as to how to use the library correctly.

So for now I'm waiting on the SeExpr guys to get back to me.

[nicholascarroll]

Thanks @ArashPartow for the advice on the ChannelMath code its much appreciated.