Module API redesign #1852

vlazzarini · 2024-03-23T15:35:33Z

Started working on consolidating and rationalising the module API (aka public functions in CSOUND).
First step was to consolidate the real FFT interface.

…uld not call performKsmps in any circumstance

vlazzarini · 2024-03-25T17:40:48Z

Some more work on this:

Removed unused functions
Consolidated some
Moved all OPDS-related functions out of the structure as inline: the rationale is twofold, first these do not depend on CSOUND but strictly operate on OPDS, secondly, they are minimal. We can add these without affecting the CSOUND structure.

bramtayl · 2024-03-26T16:36:44Z

I like the renaming to make things easier to read/understand, maybe we should add deprecation messages though?

How much benefit is there from these public functions being part of the csound struct, instead of global functions? Global functions might be a bit easier to organize, and reduce the need for so much csound->. Is the idea that opcodes can swap out the functions for alternatives?

vlazzarini · 2024-03-26T18:33:06Z

Anything that depends on CSOUND needs to be inside CSOUND otherwise external opcodes will not be able to use them. This is because external opcodes and other modules are loaded into a running instance and use its functionality without having to be linked to libcsound. Internal opcodes can access CSOUND entirely and all the public libcsound functions, but external modules/plugins cannot. The module API is for the benefit of external opcodes.

All opcodes in ../Opcodes should be external and all opcodes in ../OOps should be internal. Not all external are built as plugins because of the constraints of platforms we support. Code in 6.x drifted to accessing certain internal parts of the library because of this, but I am working on fixing this. We don't open the CSOUND structure because access then cannot be controlled and we are open to vulnerabilities. This has to be tightly controlled to avoid externals bringing the system down.

No need for deprecation messages at this point because version 7.0 does not have API compatibility with 6.x.

… one outstanding case left

bramtayl · 2024-03-26T21:18:19Z

use its functionality without having to be linked to libcsound

Why are we avoiding linking plugins to libcsound though? Seems like it would be easier than having to carry around all these function definitions in the struct?

vlazzarini · 2024-03-26T21:29:10Z

We are not avoiding anything.
This is how the system is designed, plugins are dynamically loaded modules, Csound loads them and resolves all symbols from inside. It is how a plugin system should work.

…d as plugins

bramtayl · 2024-03-27T19:35:41Z

Hm, well, on my branch, I've started to move these out to global functions, and tests, including the plugin tests I've added, pass fine. bramtayl#29. I do link the plugins to libcsound though.

kunstmusik · 2024-03-27T20:38:14Z

Despite trying to explain that's not how plugin architectures are done multiple times the past months, it seems you're not interested in learning. Linking to libcsound will "work" for tests and running on your own system, it won't work for an ecosystem of hosts, csound libraries, and plugins (particularly sharing compiled plugins with other users). It's also particularly problematic for linking issues on Windows when distributing csound or csound-based apps and plugins.

…

On Wed, Mar 27, 2024 at 3:36 PM bramtayl ***@***.***> wrote: Hm, well, on my branch, I've started to move these out to global functions, and tests, including the plugin tests I've added, pass fine. bramtayl#29. I do link the plugins to libcsound though. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

bramtayl · 2024-03-27T20:46:30Z

sharing compiled plugins with other users

Hm, that doesn't seem like the best idea anyways?

problematic for linking issues on Windows

Can we add a test for that?

kunstmusik · 2024-03-27T21:12:45Z

We've already tested it through 20 years of this plugin system working and working with users and developers to solve these problems years ago. We won't be adding tests for this because it's not practical and would cost too much money to run. But if you want to set up an automated test system that installs different versions of Windows, with different user configurations, compiles on one system and loads csound-based plugins in various commercial DAWs on other systems, you or others are more than welcome to do it. As for sharing compiled plugins, it's not only a good idea, it's essential. It'd be foolish to create a system and ecosystem that only works for programmers/developers. Most of our users don't compile csound or plugins. And in general, most musicians do not compile their own applications or plugins.

…

On Wed, Mar 27, 2024 at 4:46 PM bramtayl ***@***.***> wrote: sharing compiled plugins with other users Hm, that doesn't seem like the best idea anyways? problematic for linking issues on Windows Can we add a test for that? — Reply to this email directly, view it on GitHub <#1852 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMMA62AN3RRBL5KXHB52ALY2MV3ZAVCNFSM6AAAAABFEXRR2CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRTHE2TKNRYGE> . You are receiving this because you commented.Message ID: ***@***.***>

bramtayl · 2024-03-27T21:36:34Z

I'm just looking for a way to trigger the issue, that's all. For example, github actions has several different OS versions to choose from: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories, we could just download the same plugin and try it on different versions.

Most of our users don't compile csound or plugins

Oh sure, of course users would want to download csound and the plugins all at once. I'm just skeptical of the wisdom of a user sharing a plugin with another user, especially because the two users could have totally different operating systems, c standard libraries, etc.

vlazzarini · 2024-03-27T21:43:25Z

@bramtayl why don't you do a fork of Csound using your own ideas? Provided you keep to the licensing, it's Free software and you can do whatever you want with it.

We're happy for you to pursue your own design ideas, but for Csound 7.0. we are keeping to the library and API design we have put together since 2003.

kunstmusik · 2024-03-27T22:29:50Z

As I said, you're welcome to implement something elsewhere, but we won't be integrating that and running it here due to costs. I've given you example scenarios where these kinds of problems manifest and they're real scenarios because we've worked through them before here. As for sharing plugins, of course you're not going to share Windows plugins with Mac users or even ones compiled on Ubuntu with a SuSE user. That's obvious. Compiling a plugin on Windows or Mac and distributing it to other users on those systems, however, is a use case that is common and one we support. "Of course users would want to download ... all at once" That's a very poor assumption and it would defeat one of the main reasons to have a plugin system, which is to allow users to extend their programs via 3rd parties. BTW: You should know people have tried linking plugins to libcsound before. You're not the first person to do this. It's always been a failure where it "works for me" for one person and stops working when people start distributing binaries. If that's the scope of what you want Csound to be, then you're welcome to go that path, though I wouldn't advise it.

…

On Wed, Mar 27, 2024 at 5:36 PM bramtayl ***@***.***> wrote: I'm just looking for a way to trigger the issue, that's all. For example, github actions has several different OS versions to choose from: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories, we could just download the same plugin and try it on different versions. Most of our users don't compile csound or plugins Oh sure, of course users would want to download csound and the plugins all at once. I'm just skeptical of the wisdom of a user sharing a plugin with another user, especially because the two users could have totally different operating systems, c standard libraries, etc. — Reply to this email directly, view it on GitHub <#1852 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMMA64SCSRIBXSNLMAT6SLY2M3XRAVCNFSM6AAAAABFEXRR2CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRUGAZTGNZXGM> . You are receiving this because you commented.Message ID: ***@***.***>

bramtayl · 2024-04-22T11:02:11Z

why don't you do a fork of Csound using your own ideas?

Ok, I'll work on my on fork for now. As of #29 I've moved all public member functions from CSound to global functions, which means for the most part I can just use a forward declaration for the struct. The only remaining need for a second definition of the struct is for the C++ wrapper in plugin.h, but I'm working on dropping that too.

Compiling a plugin on Windows or Mac and distributing it to other users on those systems

I still don't understand why linking libcsound in plugins would cause a problems in these use cases. Any breadcrumbs you have to specific previous issues would be helpful.

vlazzarini · 2024-04-22T11:32:44Z

Just to let you know that we will not be accepting any such changes in the Csound code structure, but of course you are welcome to fork and develop your own version.

kunstmusik · 2024-05-12T15:43:37Z

@vlazzarini Could you update this PR with the latest from develop? Will review once this is building through GH actions.

vlazzarini · 2024-05-12T16:06:34Z

will do

vlazzarini · 2024-05-12T18:38:51Z

It's all merged now.

include/csoundCore.h

Engine/fgens.c

kunstmusik · 2024-05-16T01:25:38Z

Top/csound.c

@@ -334,466 +303,476 @@ static inline const CS_TYPE *StringType(CSOUND *csound) {
 }

 static inline const CS_TYPE *AsigType(CSOUND *csound) {
-    return csound->asigType;
+  return csound->asigType;


Curious, why were these types added here as part of the API? Were these not accessible from csound_standard_types.h?

When I added the option to build opcodes as plugins, I got undefined symbols for these. The best solution I could find was to make them available through the Csound struct. This was originally done in another PR.

kunstmusik · 2024-05-16T01:28:48Z

H/prototyp.h

@@ -186,59 +186,7 @@ int csoundSetReleaseLength(void *p, int n);
 */
 MYFLT csoundSetReleaseLengthSeconds(void *p, MYFLT n);

-/**


These functions here look like they were previously a part of the API. Are there alternative ways now to for opcodes to access this information?

They don't seem to be used anywhere in the code and they are not public either. I don't know if we should add them to the module API. I tend to try and keep it to what is being used, at this stage I think we should know more or less what opcodes require. Happy to add it if you think it needs to be there.

kunstmusik · 2024-05-20T19:09:50Z

Engine/fgens.c

@@ -1769,7 +1744,7 @@ static int gen30(FGDATA *ff, FUNC *ftp)
      x[i] = FL(0.0);
    x[1] = x[l1];
    x[l1] = x[l1 + 1] = FL(0.0);
-    csound->InverseRealFFT(csound, x, l1);
+    csound->RealFFT(csound, csound->RealFFTSetup(csound,l1,FFT_FWD), x);


Checking here: the older code called csound->InverseRealFFT(), but this is now calling csound->RealFFT(). Is this correct?

yes, the interface is common to both directions, it gets set up differently. Just noted that it's FFT_INV, will fix.

vlazzarini added 5 commits March 23, 2024 15:29

consolidated FFT interface

79114d0

removed performKsmps from CSOUND as it is a vulnerabilty: opcodes sho…

95738da

…uld not call performKsmps in any circumstance

reorganised file IO functions

615f1ce

moved non-CSOUND dependent functions out and inlined them

e7961ad

fixing dssi

49d3ad1

vlazzarini added 3 commits March 25, 2024 18:04

some more consolidation and tidying up

957a82a

dssi again

7f437ec

removed some other functions, reorganised, started documenting

e098180

vlazzarini added 2 commits March 26, 2024 18:35

removing access to libcsound parts for external opcodes. NB: there is…

65381f8

… one outstanding case left

typo in the code fixed

23d282f

move complete: all opcodes in ./Opcodes can now potentially be offere…

904c839

…d as plugins

vlazzarini added 7 commits April 5, 2024 13:28

removed arrays dependency on aops

be83a17

merged from develop

1c2d033

moved function out of CSOUND

33605d5

Merge branch 'develop' into feature/moduleapi

383dcba

fixed dssi

0031e44

merged from develop

07954f1

fixed strtod etc

49438aa

vlazzarini added 4 commits April 14, 2024 20:37

forgot to set locale in CSOUND

854aa19

added check for locale validity

62bb96a

reverted to cs_strtod

265cbdd

completed the set of type functions

616cd08

merged develop

37e9b38

merged develop

a2862ac

vlazzarini requested a review from kunstmusik May 12, 2024 18:39

kunstmusik reviewed May 14, 2024

View reviewed changes

include/csoundCore.h Outdated Show resolved Hide resolved

kunstmusik reviewed May 14, 2024

View reviewed changes

include/csoundCore.h Show resolved Hide resolved

vlazzarini added 2 commits May 14, 2024 09:32

changes following review

10636fc

fixed comments

316d805

kunstmusik reviewed May 16, 2024

View reviewed changes

Engine/fgens.c Outdated Show resolved Hide resolved

kunstmusik reviewed May 16, 2024

View reviewed changes

changed to fft from module api

11e2ac5

kunstmusik reviewed May 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Module API redesign #1852

Module API redesign #1852

vlazzarini commented Mar 23, 2024

vlazzarini commented Mar 25, 2024

bramtayl commented Mar 26, 2024 •

edited

vlazzarini commented Mar 26, 2024 •

edited

bramtayl commented Mar 26, 2024

vlazzarini commented Mar 26, 2024

bramtayl commented Mar 27, 2024

kunstmusik commented Mar 27, 2024 via email

bramtayl commented Mar 27, 2024

kunstmusik commented Mar 27, 2024 via email

bramtayl commented Mar 27, 2024

vlazzarini commented Mar 27, 2024 •

edited

kunstmusik commented Mar 27, 2024 via email

bramtayl commented Apr 22, 2024 •

edited

vlazzarini commented Apr 22, 2024

kunstmusik commented May 12, 2024

vlazzarini commented May 12, 2024

vlazzarini commented May 12, 2024

kunstmusik May 16, 2024

vlazzarini May 16, 2024 •

edited

kunstmusik May 16, 2024

vlazzarini May 16, 2024

kunstmusik May 20, 2024

vlazzarini May 20, 2024 •

edited

Module API redesign #1852

Are you sure you want to change the base?

Module API redesign #1852

Conversation

vlazzarini commented Mar 23, 2024

vlazzarini commented Mar 25, 2024

bramtayl commented Mar 26, 2024 • edited

vlazzarini commented Mar 26, 2024 • edited

bramtayl commented Mar 26, 2024

vlazzarini commented Mar 26, 2024

bramtayl commented Mar 27, 2024

kunstmusik commented Mar 27, 2024 via email

bramtayl commented Mar 27, 2024

kunstmusik commented Mar 27, 2024 via email

bramtayl commented Mar 27, 2024

vlazzarini commented Mar 27, 2024 • edited

kunstmusik commented Mar 27, 2024 via email

bramtayl commented Apr 22, 2024 • edited

vlazzarini commented Apr 22, 2024

kunstmusik commented May 12, 2024

vlazzarini commented May 12, 2024

vlazzarini commented May 12, 2024

kunstmusik May 16, 2024

Choose a reason for hiding this comment

vlazzarini May 16, 2024 • edited

Choose a reason for hiding this comment

kunstmusik May 16, 2024

Choose a reason for hiding this comment

vlazzarini May 16, 2024

Choose a reason for hiding this comment

kunstmusik May 20, 2024

Choose a reason for hiding this comment

vlazzarini May 20, 2024 • edited

Choose a reason for hiding this comment

bramtayl commented Mar 26, 2024 •

edited

vlazzarini commented Mar 26, 2024 •

edited

vlazzarini commented Mar 27, 2024 •

edited

bramtayl commented Apr 22, 2024 •

edited

vlazzarini May 16, 2024 •

edited

vlazzarini May 20, 2024 •

edited