JPverb for SuperNova fix
In my ongoing quest to find native SuperCollider equivalents to various sound processing that people are used to from everyday music listening, I found JPverb to be just what the doctor ordered. Lush reverb, complex and algorithmic, somewhat Alesis-ish but still unique. Unfortunately, supernova didn't recognize the compiled plugin library file .so.
I figured out that Faust-Generated UGens like JPverb use the string Faust for their symbols, and, without really taking the time to understand the details, I figured out you can just replace all instances of the String Faust with the plugin name and SuperNova will work just fine (along with a server flag):
# Replace symbol names
sed -i 's/Faust/PluginName/g' PluginName.cpp
# Add flag
echo "
+#ifdef SUPERNOVA
+extern "C" FAUST_EXPORT int server_type(void) { return sc_server_supernova; }
+#else
+extern "C" FAUST_EXPORT int server_type(void) { return sc_server_scsynth; }
+#endif
" >> PluginName.cpp
To modify any Faust-generated supercollider ugen to work with Supernova.
I took the liberty of applying this to JPverb manually. If there are no complaints, I would take this upstream to the Faust .CPP template for supercollider.
looking great, though I strongly object against manual editing of faust-generated cpp files. Please talk to the FAUST guys at https://github.com/grame-cncm/faust (please mention this thread (and possibly me @lfsaw in the issue you raise there).
Thanks!
Thanks for the feedback! OK I'll take it upstream.
Curious to learn about the outcome of this, I just submitted the freshly compiled HOA UGens and can't wait to learn if they work for supernova.
@florian-grond Great to know about your HOA port!
I'm guessing they won't, without using the patched Faust template I intend to create.
Fantastic @carlocapocasa, the patched Faust template will be much appreciated.
Hi @carlocapocasa I was curious if you had integrated the supernova patch to Faust already?
OK I narrowed down the fix with the latest version of the cpp file. Two things are required to make it work in SuperNova: (1) The SC_FAUST_PREFIX must be PluginName (in my case JPverbRaw) and I needed to manually add
#ifdef SUPERNOVA
extern "C" FAUST_EXPORT int server_type(void) { return sc_server_supernova; }
#else
extern "C" FAUST_EXPORT int server_type(void) { return sc_server_scsynth; }
#endif
Couldn't figure out what to change in Faust yet to make that work.
OK I asked the Faust people to merge my pull request https://github.com/grame-cncm/faust/pull/69 to include the supernova API version function.
I am less sure about what to do about SC_FAUST_PREFIX- they are using it for their faust2sc script, but it prefixes every possible plugin name with "Faust", so it is more an accident that the plugin loads with scsynth- supernova is correct to assume the plugin is called FaustJPverb.
I am wondering if the simplest solution would not be to make an exception with manual patching in this case- the faust people specifically request one do so when compiling without the faust2sc script.
@florian-grond This is not all that trivial- for a quick fix to immediately compile your plugin, refer to the minimal manual patching above.
Will check when back, thanks!
On Jul 18, 2017 8:28 PM, "Carlo Capocasa" [email protected] wrote:
@florian-grond https://github.com/florian-grond This is not all that trivial- for a quick fix to immediately compile your plugin, refer to the minimal manual patching above.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/supercollider/sc3-plugins/pull/149#issuecomment-316237110, or mute the thread https://github.com/notifications/unsubscribe-auth/ABVJxmWsIcjQGCejw1t0jHgkKqQ8p7jxks5sPU2vgaJpZM4N9gQg .
Great news! Faust merged the patch.
As soon as we get the plugin to accept SC_FAUST_PREFIX="" from CMakeList.txt we may start compiling Faust plugins with SuperNova support with the latest master-dev Faust.
great ! a good step forward
Okay I've got it licked- The makefile was not setting the faust prefix for targets JPverb_supernova and Greyhole_supernova- just JPverb and Greyhole. Therefore, the Faust prefix was not disabled and it could not be loaded.
As a side effect, NDEBUG was not set for JPverb_supernova and Greyhole_superova either, which is why we were seeing some Faust debug output- this was not what the faust template intended and this pull request fixes this issue as well.
I modified the pull request to purely affect the CMakeLIsts.txt.
I wonder why travis fails- builds fine for me.
Edit: fix needed to be conditional on SUPERNOVA flag. @scztt, thanks a lot for setting CI up.
What's the state of this? @carlocapocasa , sorry to follow up so late in the process, is this still a non-stale PR?
Yeah, this will merge and is necessary to build JPverb working on supernova as well as get rid of some error messages that look like Faust-plugins are noisy, but it's really the build system's fault.
ok, I'll have a look at it ASAP.
No worries! Thanks for looking into it!
Do I see this right and I'd need to re-run faust1 (in its latest versions) to generate c++ code in order for your changes to take effect?
I believe Carlos changes to make Faust fit for supernova did, in fact, add some lines to the C++ code, I plan to test it soon on the SC-HOA plugins.
www.grond.at
On Tue, Nov 7, 2017 at 2:15 PM, LFSaw [email protected] wrote:
Do I see this right and I'd need to re-run faust to generate c++ code in order for the changes to take effect?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/supercollider/sc3-plugins/pull/149#issuecomment-342590688, or mute the thread https://github.com/notifications/unsubscribe-auth/ABVJxitE6jtkYgbCS68a3ayeB-9c0aqiks5s0KxMgaJpZM4N9gQg .
I believe Carlos changes to make Faust fit for supernova did, in fact, add some lines to the C++ code, I plan to test it soon on the SC-HOA plugins.
I guess, you mean
did, in fact, add some lines to the C++ code generator
?
Yes, that is correct- a recompile is required.
However, in order to avoid possible regressions, I would suggest to make an exception and manually edit the cpp file instead, unless there is something compelling Faust has improved in the mean time.
Adding this to the bottom:
#ifdef SUPERNOVA extern "C" FAUST_EXPORT int server_type(void) { return sc_server_supernova; } #else extern "C" FAUST_EXPORT int server_type(void) { return sc_server_scsynth; } #endif
If you prefer to recompile, I would suggest to check out the latest stable Faust release branch and cherry pick my commit b4e4dab0 before building it. Otherwise, you would have to build from Faust master-dev. The Faust build process itself is quite painless in any case.
curious: why do you think it is not a good idea to use the latest stable that includes your commit?
On 08. Nov 2017, at 18:26, Carlo Capocasa [email protected] wrote:
Yes, that is correct- a recompile is required.
However, I would suggest to make an exception and manually edit the cpp file and add
#ifdef SUPERNOVA extern "C" FAUST_EXPORT int server_type(void) { return sc_server_supernova; } #else extern "C" FAUST_EXPORT int server_type(void) { return sc_server_scsynth; } #endif
in order to avoid possible regressions, unless there is something compelling Faust has improved since the present compilation.
If you prefer to recompile, I would suggest to check out the latest stable Faust release branch and cherry pick my commit b4e4dab0 before building it. Otherwise, you would have to build from Faust master-dev. The Faust build process itself is quite painless in any case.
— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub, or mute the thread.
Pure conservatism- we know that the present code worked for lots of people, but merely think that the new code will work for lots of people.
In my estimation, a benefit to end users, such as improved performance, would justify the change, while a benefit to us developers, such as the satisfying application of principle, would not.
my idea behind this is:
I'd like to provide material that is easily (and documented'ly (?)) to be repeated by anyone else but me, in case there are things coming up that e.g. make a change in the faust-code necessary (this is what triggered my last update). That's my reason why I don't want to mess with generated code...
In case you'd like to fumble around with it, I'm happy to get an update pf this PR that consists of
- updated FAUST file (if needed)
- updated (generated) c++ file
- updated faust_src/README.md
I'd find it important that both c++ sources (for Greyhole, JPVerb) are built with the same faust version.
If you don't, I'll try to find time to do this (hopefully soon) myself.
On 08. Nov 2017, at 19:49, Carlo Capocasa [email protected] wrote:
Pure conservatism- we know that the present code worked for lots of people, but merely think that the new code will work for lots of people.
In my estimation, a benefit to end users, such as improved performance, would justify the change, while a benefit to us developers, such as the satisfying application of principle, would not.
— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub, or mute the thread.
I was kinda sure that the README contains the git-sha of the faust version used but this is apparently not the case. For the future, I think this is a good idea, though.
On 08. Nov 2017, at 20:31, Till Bovermann [email protected] wrote:
my idea behind this is:
I'd like to provide material that is easily (and documented'ly (?)) to be repeated by anyone else but me, in case there are things coming up that e.g. make a change in the faust-code necessary (this is what triggered my last update). That's my reason why I don't want to mess with generated code...
In case you'd like to fumble around with it, I'm happy to get an update pf this PR that consists of
- updated FAUST file (if needed)
- updated (generated) c++ file
- updated faust_src/README.md
I'd find it important that both c++ sources (for
Greyhole,JPVerb) are built with the same faust version. If you don't, I'll try to find time to do this (hopefully soon) myself.On 08. Nov 2017, at 19:49, Carlo Capocasa [email protected] wrote:
Pure conservatism- we know that the present code worked for lots of people, but merely think that the new code will work for lots of people.
In my estimation, a benefit to end users, such as improved performance, would justify the change, while a benefit to us developers, such as the satisfying application of principle, would not.
— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub, or mute the thread.
@carlocapocasa, I updated the .cpp and header files in a separate PR, integrating your changes #172 . Please have a look and test on SuperNova.
Thanks, great stuff!
It will take a little while until I'll be able to context-switch my brain back to SC dev to test it properly, but I'll get to it as soon as I can.
others are welcome to test, too :)
Fantastic Till, will test soon!
On Thursday, November 16, 2017, LFSaw [email protected] wrote:
others are welcome to test, too :)
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/supercollider/sc3-plugins/pull/149#issuecomment-344905043, or mute the thread https://github.com/notifications/unsubscribe-auth/ABVJxj9cbLcpE31kgb2fT2sD6TrMEITcks5s3CZ8gaJpZM4N9gQg .
--
www.grond.at