free-audio / clap

Audio Plugin API
https://cleveraudio.org/
MIT License
1.77k stars 100 forks source link

Track info #197

Closed abique closed 1 year ago

abique commented 1 year ago
Approach struct properties (enum) properties (char*)
easy to use yes needs specific code no, we can have helpers
can have optional attributes no, but empty strings, CLAP_INVALID_ID do the job yes yes
extensible requires private extension and changed() method yes yes
extensible by third parties requires private extension and changed() method no yes
notes We may think of a generic host properties extensions instead
robbert-vdh commented 1 year ago

Below is my suggestion for reworking this extension as I described earlier.

The benefits here is that the API is much simpler than a full dictionary style interface, while still allowing future CLAP versions to add new properties without requiring a new extension. The host also does not need to provide all track information like it would need to in the current draft for this extension.

If hosts want to provide their own third party track information, then they can simply define their own extension in a similar style. Using an enum here makes the implementation and usage simpler than having to compare strings everywhere. Another benefit of this approach is that it makes it very clear to plugins whether this third party track information is being provided or not. Since this API is only a single line get function, I think trying to deduplicate this interface by factoring it out into a generic dictionary style API only makes things more complicated. And with the API as suggested in this PR it's also possible for plugins to modify the track information, which is probably not intended.

track-info.h ```c #pragma once #include "../../plugin.h" #include "../../color.h" #include "../../string-sizes.h" static CLAP_CONSTEXPR const char CLAP_EXT_TRACK_INFO[] = "clap.track-info.draft/2"; // These are the various information types available through this extension. // Each property needs to be queried with the correct `CLAP_TRACK_INFO_TYPE_*` // value or the host will return an error. Not all hosts support all properties. enum { // TODO: What is this? // Uses `CLAP_TRACK_INFO_TYPE_INT64`. CLAP_TRACK_INFO_PROP_ID, // TODO: Is this a linear index? // // Uses `CLAP_TRACK_INFO_TYPE_INT64`. CLAP_TRACK_INFO_PROP_INDEX, // The track's name, either explicitly set by the user or automatically // generated. // // Uses `CLAP_TRACK_INFO_TYPE_STRING`. CLAP_TRACK_INFO_PROP_NAME, // A path in the format `Group 1/Nested Group 2/Track Name` describing the // track's name and parent groups in the project's hierarchy. If this is a // top level track, then the value should be the same as `CLAP_TRACK_INFO_PROP_NAME`. // // Uses `CLAP_TRACK_INFO_TYPE_STRING`. CLAP_TRACK_INFO_PROP_PATH, // TODO: Is this the track's final output port? Input port? // // Uses `CLAP_TRACK_INFO_TYPE_STRING`. CLAP_TRACK_INFO_PROP_AUDIO_PORT_TYPE, // TODO: Same // // Uses `CLAP_TRACK_INFO_TYPE_INT64`. CLAP_TRACK_INFO_PROP_AUDIO_CHANNEL_COUNT, // The track's associated color, if it has one. // // Uses `CLAP_TRACK_INFO_TYPE_CLAP_COLOR`. CLAP_TRACK_INFO_PROP_COLOR, // Whether this track is an aux, send, or return track. // // Uses `CLAP_TRACK_INFO_TYPE_BOOL`. CLAP_TRACK_INFO_PROP_IS_RETURN_TRACK, }; typedef uint32_t clap_track_info_prop; // These are the value types for a track information property. Every property // should be queried with the correct value type as documented alongside the // property or the host will return an error. enum { // `data` should be a `char*`, and `size` should be the size of the string // buffer. The string may be truncated if the buffer is not large enough. See // the `string-sizes.h` header for the recommended buffer sizes. CLAP_TRACK_INFO_TYPE_STRING, // `data` should be a `int64_t*`, and `size` should be `sizeof(int64_t)`. CLAP_TRACK_INFO_TYPE_INT64, // `data` should be a `bool*`, and `size` should be `sizeof(bool)`. CLAP_TRACK_INFO_TYPE_BOOL, // `data` should be a `clap_color_t*`, and `size` should be `sizeof(clap_color_t)`. CLAP_TRACK_INFO_TYPE_COLOR, }; typedef uint32_t clap_track_info_type; #ifdef __cplusplus extern "C" { #endif typedef struct clap_plugin_track_info { // Informs the plugin that the given property changed. // If prop_id is null, then all properties are invalidated. // [main-thread] void(CLAP_ABI *changed)(const clap_plugin_t *plugin, clap_track_info_prop *prop_id); } clap_plugin_track_info_t; typedef struct clap_host_track_info { // Get a property's value, if the host supports the property. See the // property enum for a list of possible properties and their associated // types. // // The return value is either a positive value indicating the number of bytes // written to `data`. A negative return value of -1 indicates that the host // does not support this attribute, while a value of -2 indicates that either // the type does not match the property, or that the size parameter does not // match the type's fixed size. // [main-thread] int64_t(CLAP_ABI *get)(const clap_host_t *host, clap_track_info_type type, clap_track_info_prop property, void *data, uint64_t size); } clap_host_track_info_t; #ifdef __cplusplus } #endif ```
abique commented 1 year ago

After sometime thinking about I see two options:

@robbert-vdh I don't see the benefit of having a property style approach but exclusive to the track-info. It is likely that the properties may be needed for other things later and having a single implementation for properties makes everything uniform (you learn once the interface to query properties, and you implement only once the properties interface. By the way a dictionnary can have read-only properties and reject the set operation on some keys.

abique commented 1 year ago

I think the property style approach has a weakness:

abique commented 1 year ago

Something that could be in line with the original approach and what @robbert-vdh proposed:

// This is the initial track info
struct track_info0 {
   // attrs
};

// Additionnal info came in a later version of clap
struct track_info1 {
   // attrs
};

// Additionnal info came in a later version of clap again
struct track_info2 {
  // attrs
};

track_info0 info0;
track_info1 info1;
track_info2 info2;
host_track_info->get(host, 0, &info0);
host_track_info->get(host, 1, &info1);
host_track_info->get(host, 2, &info2);

This avoid doing many low level conversion to int, double, ...

robbert-vdh commented 1 year ago

I think the property style approach has a weakness:

* it is complicate and not in line with the "simplicity" we're used to

In my opinion a generic key-value store complicates things even further. The main problems are that you now a) don't know which keys exist, b) third party hosts can and will define their own keys, something CLAP normally would use third party extensions for. This would be the same as the IInfoListener in VST3 and having used that it complicates the interface a lot. An enum with the possible info types simplifies the interface for both the plugin and the host.

* it opens many questions, what if a int64 is queried with a pointer to 4 bytes, do you implicitly convert to int32 and let the query succeed?

* passing `void*` and `size_t` will lead to type error and other problems

The types and sizes are properly documented. That's the reason why I proposed a clap_track_info_type argument in the first place. That argument adds another layer of verification. If you pass a wrong size and/or the wrong type argument for the property, the host returns an error code. That is in fact exactly the same as the generic dictionary interface from this PR, but with the types and properties determined by enums instead of strings to make it simpler to use and to encourage creating another extension if someone needs more context data. Just like we encourage creating additional factory types when a plugin may need to expose more static data during plugin scanning.