Proposal for ext-state-context

[defiantnerd]

Sometimes it is desirable to add additional information about a plugin state depending on context. The extension is designed to be a full replacement of the ext-state extensions of CLAP.

[CLAassistant]

All committers have signed the CLA.

[robbert-vdh]

Is there a way to get this functionality without deprecating the entire state extension? Or is the idea that plugins and hosts keep using state, and only the plugins that need this more granular save and restore functionality use this new extension?

[defiantnerd]

A host should always prefer state2 over state if the plugin provides it. The plugin should also accept state, but the user may loose some functionality/comfort.

[robbert-vdh]

Since the majority of plugins may not use this functionality (or it would have come up earlier), an alternative way to do this would be to have a separate extension that sets the context for subsequent state saves and loads. This has the downside of turning this into a state machine (i.e. you need to keep track of the last context), but the benefit is that you don't need to throw out the entire state extension just to add a second flag parameter.

[tim-janik]

Since the majority of plugins may not use this functionality (or it would have come up earlier), an alternative way to do this would be to have a separate extension that sets the context for subsequent state saves and loads. This has the downside of turning this into a state machine (i.e. you need to keep track of the last context), but the benefit is that you don't need to throw out the entire state extension just to add a second flag parameter.

Is that really a downside? If most plugins don't care, an optional extension makes more sense than demanding that future plugins implement two duplicate interfaces. Here is a simple draft to make the comparison more concrete:

static CLAP_CONSTEXPR const char CLAP_EXT_STATE_CONTEXT[] = "clap.state-context";

enum  clap_plugin_state_context_type {
      CLAP_STATE_CONTEXT_PROJECT = 1,
      CLAP_STATE_CONTEXT_PRESET = 2,
      CLAP_STATE_CONTEXT_CLONE = 3
}; // TODO: copy context use case descriptions from PR#118

typedef struct clap_plugin_state_context {
  // Hosts that use the set_context() function should *always* call it directly before
  // ->save() or load(). Plugins that implement the set_context() function should
  // keep the last assigned context around, regardless of the frequency of invocations.

  // Assign the context for subsequent calls to ->save() or load() of the
  // clap_plugin_state extension.
  void (*set_context)(const clap_plugin_t *plugin, uint32_t context);
} clap_plugin_state_context_t;

I think that'd be a lot simpler than the proposed PR, fewer plugins have to bother implementing this and just need to keep an additional uint32_t around.

[defiantnerd]

I've updated the PR according to the discussion. The advantage of the smooth implementation outweighs the disadvantage of having an additional uint32_t.

[abique]

Hello,

I like that this extension does not deprecate the state extension. Yet I'm not a big fan of the disconnection between the context and the state.

What about this:

• we introduce clap_plugin_contextual_state, which is presented as a more advanced version of the state but we do not deprecate state
• the hosts needs the same amount of check with both approaches:

  
  // Stateless solution
  if (canUseContextualState)
  contextualState->save(plugin, context, out);
  else if (canUseState)
  state->save(plugin, out);

// Statefull solution if (canUseState) { if (canUseContextualState) contextualState->set_context(plugin, context); state->save(plugin, out); }

- the glue layer can do some magic so the plugin state is delegating the work to contextual state for example

Also I don't have a problem with introducing this extension this early.

So I personally would prefer:
```C
struct clap_plugin_contextual_state {
   bool (*save)(const clap_plugin_t *plugin, uint32_t context, const clap_ostream_t *stream);
   bool (*load)(const clap_plugin_t *plugin, uint32_t context, const clap_istream_t *stream);
};

[robbert-vdh]

Isn't that exactly the same as the original state2 posted by @defiantnerd? That ends up making the state extension obsolete and requires hosts to implement both, which I think would be a very bad idea. The stateful solution requires less code duplication on both the plugin's and the host's side, and it doesn't make the state extension obsolete. Most of the plugins likely won't need this context information, so keeping the state extension as is and having this be an optional extension to enhance the state extension rather than replace it seems like a better idea in my eyes.