Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parameter Interpolation Type #116

Open
15 tasks
revisionfx opened this issue Apr 23, 2023 · 16 comments
Open
15 tasks

Parameter Interpolation Type #116

revisionfx opened this issue Apr 23, 2023 · 16 comments

Comments

@revisionfx
Copy link
Contributor

Open Effects Proposal for Standard Change

Please read the contribution guidelines first.

Standard Change Workflow

  • [ X] Create proposal as issue (you're doing this now!)
  • [X ] Tag this issue with standard change tag
  • Identify subcommittee: at least one plug-in vendor, and at least one host
  • Discuss the idea in this issue
  • Write new or updated code and doc
  • Publish updates as a pull request (ideally on a feature/PROPOSAL-NAME branch)
    • Make sure that PR references this issue number to keep them in sync
    • Discuss and review code in the PR
    • Meet all requirements below for accepting PR
  • When subcommittee signs off and other members don't have any further review comments,
    maintainer merges PR to master which closes PR and issue

Requirements for accepting a standard change:

  • Header files updated
  • Documentation updated
  • Release notes added
  • Compatibility review completed
  • Working code demonstrated with at least one host and one plugin
  • At least two members sign off
  • No further changes requested from membership

Summary

We discussed for a while supporting interpolation types for parameters.

Motivation

As we are discussing also effects interchange... a good thing would be to support key-framing representation

Problem

Except for boolean, and perhaps menu which we assumed is constant animation (specs should precise so one does not animate menus) - maybe string too?

We don't have a model for curve function curves. I put Smooth below, I presume we need two things: a form of cubic type (much like in GLTF) and would like a form of monotonous curve representation for time based parameters (e.g. in retiming you don't want the curve to make you go backwards which means going from A to B never goes outside the min and max...). From experience most application use a form of bezier for the type Smooth. Other types can be defined but not expected to be supported by app. It's been suggested that if an app does not support "smooth" interpolation that it can perhaps use an hybrid linear to position KF and a backing per frame of values...

Add kOfxParamPropInterpType

  • kOfxParamInterpTypeConstantStep
  • kOfxParamInterpTypeLinear
  • kOfxParamInterpTypeSmooth // whatever the type of curve the host defaults to

Impact

Likely suite would need to be V2?

Documentation Impact

What changes to the docs are needed for this change?

Stakeholders

Who will benefit from this proposed change? Plug-ins, hosts, or both? Specific types of hosts?

Discussion

@fxtech
Copy link
Contributor

fxtech commented Dec 5, 2023

Would this be requesting a default interpolation type for the parameter instead of just getting whatever the host decides? Or do you want to set specific key values and interpolation types as part of the default value?
Can you provide concrete examples of how this would be used?

@fxtech
Copy link
Contributor

fxtech commented Feb 10, 2024

This needs further discussion. I believe Pierre's example is he wants to do some analysis which then sets some keyframes, and instead of getting the default host linear or smooth interpolation he wants to indicate that the keys should be step-evaluated (ie. kOfxParamInterpTypeConstantStep or kOfxParamInterpTypeHold).

@fxtech
Copy link
Contributor

fxtech commented Feb 10, 2024

How about something like this?

enum OfxParamInterpType {
   kOfxParamInterpTypeConstantStep,   // hold until next key
   kOfxParamInterpTypeLinear,     // linear
   kOfxParamInterpTypeSmooth,  // some type of smooth curve
   kOfxParamInterpTypeCustom   // get-only, since this could be host-specific
};

typedef struct OfxParameterSuiteV2 {
   // ... same stuff as in V1
     OfxStatus (*paramGetInterpType)(OfxParamHandle  paramHandle,
			       unsigned int nthKey,
			       int *type);
     OfxStatus (*paramSetInterpType)(OfxParamHandle  paramHandle,
			       unsigned int nthKey,
			       int type);
};

@revisionfx
Copy link
Contributor Author

That would be fine with me.

Main application:

  1. Set Host default for when user adds a KF (often linear) -
  2. Set Interp Type when we write ourselves the KF.
    Two requests we have here as it's often Linear, is auto set to constantStep or to Curve... (many uses Bezier)
    constantStep helps clean UI even if we internally do equivalent sampling just the prevNearest KF as otherwise they see wrong values in the UI.

Additional notes: Phil was saying they dont have in their app ConstantStep.
We assume popup choice menu is always ConstantStep (we don't want to animate between choice 3 and 5), seen that issue before in more than one host.
Same for boolean checkbox. We don't want halfway to change from 0 to 1.
I am not clear if Strings are animatable :)

@revisionfx
Copy link
Contributor Author

OK in support of this FR, we rechecked all hosts.

RIght now if you have option choice menu with options A, B, C, D, E - kOfxParamTypeChoice
And you keyframe it
Say put a KF of value A at frame 1 and KF option E at frame 101

An example: Say you have an analysis process that detects 3:2 pulldown phases - you don't want marking first frame of segments to be producing linearly animated values. We use that here in mulitple places for example for plugins that request multiple frames from same clip to process a frame to allow user to mark scene cuts. In general as we get an instance change callback we could with such suite do it ourselves without host having to implement anything else.

I usually don't like to single out hosts but this seems a generalized issue... Some of the tests were done by other folks here so hope I am not lying :)

  1. Some hosts always return the first frame value (or I guess first key-frame) with paramGetValue - value A in the example, not the current time value
    This includes Silhouette and Resolve (who also does not display in KF curve editor such value)
    Easy enough fix if you know, always use for that parameter type paramGetValueAtTime

  2. About half the hosts linearly interpolates choiceParam, that is as you advance from first frame to frame 101 in this example, it flips to B, then to C, then to D, and finally to E. I have yet to imagine a possible use case where this would be wanted behavior.
    OK we can internally ignore that and always look if there are KF for previous or equal KF... but user stills see in UI B,C,D, when going to in--between frames... which is confusing for users
    This includes Nuke, Baselight, Scratch and Fusion (in case of Fusion it's not an app works like that as the native Fusion plugin behave as expected).

  3. For hosts that don't support choice menu animation we create a linked integer param, but that has same issue, it will be linearly animated by default.
    (2 of the 15 hosts we support reports option choice menu not supported, which is legit according to specs but take more UI space).
    e.g. Flame

I know a user can usually select all KF and change interpolation type themselves but that's annoying, not clear to user and I can't think of a use case where one would want linear interpolation of a choice param.

We haven't check all hosts for boolean param so it doen't have same issue. And have had no use yet for animating string param which likely only makes sense constantSteps (that param should also have not supported option like boolean and choice). We use that also on other param types such as in our color matching tool we ask user to set KF on reference frames (where the param is a frame number). In that case it's cosmetic as we always look for the frames with KF but user still sees wrong values in their UI.
Also does this affect kOfxParamTypeStrChoice ?

Finally, as this implies V2 suite, as these are global pointers, shouldn't we always put from hereon Version as first item of a suite so we don't have ourselves to do book-keeping of which suite version was loaded in a given host?

@garyo
Copy link
Contributor

garyo commented Mar 30, 2024

  • I would be surprised if any host tried to interpolate a StrChoiceParam, but it's not impossible since an order is defined.
  • String param interpolation requires a custom callback, which you could implement any way you like.
  • I agree that interpolating ChoiceParams is pretty odd, but I have seen it used in Nuke to switch among a set of inputs (where the choice param's options are which input to use). Perhaps the spec could add a property which plugins could use to say "don't interpolate this choice param" -- but we'd have to be very clear on what that means with keyframes.
  • Returning the wrong current value from paramGetValue() (i.e. different from paramGetValueAtTime(..., currentTime)) seems clearly like a bug. Should be reported to the hosts in question.
  • I think we have a long history of plugins tracking which suite versions are in use; I don't see any reason to change that now.

@revisionfx
Copy link
Contributor Author

revisionfx commented Mar 30, 2024

"but I have seen it used in Nuke to switch among a set of inputs (where the choice param's options are which input to use)"

But I assume they don't switch the input in-between... that could be another use case for displaying Clip menu in effects controls so at least additional inputs can be switched along timeline :)

" Perhaps the spec could add a property which plugins could use to say "don't interpolate this choice param" "

They should never :)

@garyo
Copy link
Contributor

garyo commented Mar 30, 2024

"but I have seen it used in Nuke to switch among a set of inputs (where the choice param's options are which input to use)"
But I assume they don't switch the input in-between...

Yes, that's exactly what happens. Set a kf value 0 at t0, and 10 at t10, it switches through all 10 inputs over the 10 sec. (But yes, not a fractional input of course, it uses int values.)

" Perhaps the spec could add a property which plugins could use to say "don't interpolate this choice param" "
They should never :)

I think it's a bit too late to specify that since as you say many hosts do it today.

@revisionfx
Copy link
Contributor Author

"but I have seen it used in Nuke to switch among a set of inputs (where the choice param's options are which input to use)"
But I assume they don't switch the input in-between...

Yes, that's exactly what happens. Set a kf value 0 at t0, and 10 at t10, it switches through all 10 inputs over the 10 sec. (But yes, not a fractional input of course, it uses int values.)

I am not aware of that - sounds like a conceptual bug, what we do for that here is to have user make a long clip abutting all as one big clip and use Frame Number to mark each clip first frame. OT but Foundry does have another UI issue if say you have 10 optional inputs, they only display the first 6 I think, so it you want to connect input 10, you have to connect 7,8,9 first so the connector shows up. Another host has a better UI in my opinion popping up a menu to select which optional input to add as input in UI of node.

" Perhaps the spec could add a property which plugins could use to say "don't interpolate this choice param" "
They should never :)

I think it's a bit too late to specify that since as you say many hosts do it today.

Yes, that's one thing this Proposal would address

@revisionfx
Copy link
Contributor Author

Adding: As per discussion in OpenFX meeting, there is no common curve editing method shared by all hosts. Two hosts responded they don't support bezier with handles in their KF editor.. so this will be limited to whatever the host sets for interpolation. For custom, one could make argument that one could pass back the custom int (is that a thing for first KF), also it could maybe adapt the same interpolation model as custom parameter. A situation that this does not handle directly is importing data directly in plug-in or using a custom modal dialog with key-frame handles, would the proper way to store these in some linked parameter?

@garyo
Copy link
Contributor

garyo commented Apr 5, 2024

A situation that this does not handle directly is importing data directly in plug-in or using a custom modal dialog with key-frame handles, would the proper way to store these in some linked parameter?

I don't understand this comment -- are you suggesting a change to this proposal, or is this something different?
Let's try to nail down exactly what this proposal looks like so we can get it implemented. Pierre, are you happy with @fxtech 's suggestion above? If not, what changes would you suggest?

Also Custom params, if animation is supported by the host, use the custom interpolation callback so the plugin is responsible for how it interpolates. It could, for instance, encode the desired interpolation type for each keyframe's data in the data itself.

@revisionfx
Copy link
Contributor Author

I guess that would be an application for kOfxParamPropDataPtr if I needed extra data for myself.

The only question is about custom option. If I want to check (get) the interpolation type and I get "custom", it seems I would not want to change interpolation type if I expect a curve of some sort, So it works for edit value. If I want to add a new KF if I select smooth curve I assume host will write whatever it would do if one manually added a KF there now. So not clear why we need custom type unless there was a host supplied custom type callback with its own enums... For intiial simplicity would rather not go there SO my suggestion then would be strike out custom option for now.

Only user - techsupport request right now is: be able to change choice option to constantStep if not the default, be able to change sliders to be constantStep or Curve if defaulted to linear.

@garyo
Copy link
Contributor

garyo commented Apr 5, 2024

my suggestion would be strike out custom option for now.

That works for me. @fxtech ?

@revisionfx
Copy link
Contributor Author

What is the conclusion for Interpolation type:

Aside adding an OfxParamPropInterp property which is clear,

Options seems to be:

  1. OfxParameterSuiteV2
  2. A side utility suite or a set of free floating Funcs that depends on SuiteV1
    e.g.: typedef OfxStatus (OfxParamInterpGetFuncV1)(OfxParamHandle paramHandle, OfxTime time, OfxParamPropInterp Interp):
    typedef OfxStatus (OfxParamInterpSetFuncV1)(OfxParamHandle paramHandle, OfxTime time, OfxParamPropInterp Interp):
    or via KeyIndex - note this at ParamDefine time is not creating KF, just setting param Inter time
    would need SetValue and SetValueAtTime

@fxtech
Copy link
Contributor

fxtech commented Oct 1, 2024

Proposed new API:

typedef enum OfxParamPropInterp
{
	kOfxParamPropInterpConstant,
	kOfxParamPropInterpLinear,
	kOfxParamPropInterpSmooth
} OfxParamPropInterp;

OfxStatus (*paramGetInterp)(OfxParamHandle paramHandle, OfxTime time, OfxParamPropInterp interp);
OfxStatus (*paramSetInterp)(OfxParamHandle paramHandle, OfxTime time, OfxParamPropInterp *interp);

@revisionfx
Copy link
Contributor Author

Looks good to me - I assume this would be added to ofxParam.h, not inside suite.
And also is there need for option to set OfxPropertySetHandle also so it can be set during param define?
as in:
PropHost->propSetInt(props, kOfxParamPropInterp, NULL, kOfxParamPropInterpSmooth);

Sorry I had to drive somewhere today and I got in cell phone vortex just when you started to talk about this.,,,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Upcoming Meeting Agenda Items
Development

No branches or pull requests

3 participants