free-audio / clap

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

1.1.7 #270

Closed abique closed 1 year ago

abique commented 1 year ago

One open question was if preset-discovery.h should be in ext/ or not.

robbert-vdh commented 1 year ago

One open question was if preset-discovery.h should be in ext/ or not.

It's not a plugin extension, but maybe there's another good way to organize this and other factories like the invalidation factory, converters, and whatever else is needed in the future? With the current setup a single subdirectory for all factories would suffice. Then you could even have a draft directory in there to separate draft factories from non-draft factories.

baconpaul commented 1 year ago

One open question was if preset-discovery.h should be in ext/ or not.

It's not a plugin extension, but maybe there's another good way to organize this and other factories like the invalidation factory, converters, and whatever else is needed in the future? With the current setup a single subdirectory for all factories would suffice. Then you could even have a draft directory in there to separate draft factories from non-draft factories.

right so i like the idea of directories for things. ext/ for extensions factory/ for factories. And since this is a factory perhaps it belongs there?

I think just from an "i approach the software anew" perspective it is too prominent in the top directory side by side with clap.h.

otherwise looks correct to me!

Trinitou commented 1 year ago

One open question was if preset-discovery.h should be in ext/ or not.

It's not a plugin extension, but maybe there's another good way to organize this and other factories like the invalidation factory, converters, and whatever else is needed in the future? With the current setup a single subdirectory for all factories would suffice. Then you could even have a draft directory in there to separate draft factories from non-draft factories.

right so i like the idea of directories for things. ext/ for extensions factory/ for factories. And since this is a factory perhaps it belongs there?

Maybe like this?

abique commented 1 year ago

maybe we need to make those plural?

I think we can change that as we have clap.h which covers us.

robbert-vdh commented 1 year ago

I'd stick with singular. Using singular for namespaces and directories seems fairly common in most languages, with C# being the only language that comes to mind where the plural form is preferred. But ultimately it's just an aesthetic choice, so it doesn't really matter that much.

robbert-vdh commented 1 year ago

This comment probably also needs to be updated: https://github.com/free-audio/clap/blob/18c4c37f195705e4246d0bf2128abce87e6099a3/include/clap/entry.h#L56

A nice thing about moving the plugin factory to a factory directory is that it's puts more emphasis on the fact that entry.get_factory() can be used for more than just the plugin factory. Since some people were unconditionally returning their plugin factory from that function.

baconpaul commented 1 year ago

I think that is smart but moving the base factory mag be a 1.2 thing perhaps? Also agree with Robbert that singular is fine and often used

abique commented 1 year ago

I think that is smart but moving the base factory mag be a 1.2 thing perhaps? Also agree with Robbert that singular is fine and often used

I think it is fine, it doesn't break the ABI, and I suspect that all of the projects simply includes clap.h.

baconpaul commented 1 year ago

I think that is smart but moving the base factory mag be a 1.2 thing perhaps? Also agree with Robbert that singular is fine and often used

I think it is fine, it doesn't break the ABI, and I suspect that all of the projects simply includes clap.h.

Cool! I agree that as long as we mark the renames in the changelog should be fine

abique commented 1 year ago

I'm happy with clap_preset_discovery_factory_t as a name.

abique commented 1 year ago

There are multiple inconsistencies in the naming of the draft identifiers over the project, but we should not fix them now because it'd create a break-point.

I don't think it is a very important thing, and at the end of the day if the extension becomes stable then the draft disappears.

abique commented 1 year ago

I wondering if with preset discovery, the flags should be set for the preset "as well or instead" of the location?

It could be that within the same folder some preset can be used and some others can't because they rely upon license requirement for some modules etc... So it definitely seems that this is needed for the preset as well.

Trinitou commented 1 year ago

Maybe the factories concept could be documented in the readme as well. Following aspects might be interesting for developpers looking into CLAP for the first time:

I'm not 100% sure about the exact formulations.

Also it could be good to not make it a new "Factories" section in the Readme.md but integrate the points about factories into the "Extensions" section

abique commented 1 year ago

We could bring back some info about unknown/unavailable timestamps using the new CLAP_TIMESTAMP_UNKNOWN constant

Thanks! :)