added HOAUgens #146

florian-grond · 2017-06-02T19:09:11Z

Hello, I have added the HOAUgens to the SC3plugins. The Plugins include Ambisonics encoding, transforming (rotation, beamforming, mirroring) and decoding up to the 5th order. Any feedback very welcome, Florian

nhthn

whoa, thanks florian! this is awesome!

one problem: i'm pretty sure we also need the FAUST source files to comply with GPL.

florian-grond · 2017-06-02T19:25:13Z

The FAUST code has been implemented by Pierre Lecomte and is here on GitHub:
https://github.com/sekisushai/ambitools

nhthn · 2017-06-02T19:31:53Z

could you add a README to HOAUgens that links to the repo? after that we're set

florian-grond · 2017-06-02T19:40:16Z

done!

nhthn

thanks!

bagong

This has not been added to the build, it seems? ;)

I checked on VS2015, unfortunately the old

error C2375: 'api_version': redefinition; different linkage

is back. Florian, looks like you didn't use the latest Faust compiler? Check my comment in #143 and the ensuing discussion. Likely once you've regenerated the sources with a recent Faust version and added you Plugins in source/CMakeLists.txt everything is file. Thanks!

also added them to the CMakeLists.txt where I also added: set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fbracket-depth=512") since the Faust autogenerated code creates deep bracket nestings for higher orders

florian-grond · 2017-06-05T15:31:44Z

Hi bagong, I recompiled some of the HOAUgens with the most recent FAUST version, also added HOAUGens to CMakeLists.txt where I also added:
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fbracket-depth=512")
as some of the autogenerated code for higher orders generates deep nesting.
Do I need to make a new PR?

telephon

Thank you for these UGens, they will be a game changer for many of us. And everyone will be buying lots of speakers!

I suppose this is generated SuperCollider class code, but still I have some suggestions of simplifications. I think they will make the code easier to debug, too, and arguably easier to use.

Isn't it easier to pass an array around?

telephon · 2017-06-05T15:36:33Z

source/HOAUGens/sc/Classes/HOAAzimuthRotator2.sc

@@ -1,4 +1,4 @@
-HOARotatorAz2 : MultiOutUGen
+FaustHOAAzimuthRotator2 : MultiOutUGen
 {
  *ar { | in1, in2, in3, in4, in5, in6, in7, in8, in9, azimuth(0.0) |


@reading this, I was wondering if it wouldn't be better to just pass an array? Like

*ar { | inputs, azimuth(0.0) | if(inputs.size < 9) { Error("not enough inputs").throw }; ^this.multiNewList('audio', inputs ++ azimuth) }

And if you define numInputChannels in each class (see comment further below), you can make this more general:

if(inputs.size < this.numInputChannels) { Error("not enough inputs").throw };

telephon · 2017-06-05T15:43:22Z

source/HOAUGens/sc/Classes/HOAAzimuthRotator1.sc

@@ -25,6 +25,6 @@ HOARotatorAz1 : MultiOutUGen
      ^this.initOutputs(4, rate)
  }

-  name { ^"HOARotatorAz1" }
+  name { ^"FaustHOAAzimuthRotator1" }


this is redundant, it's defined in UGen as:

name { ^this.class.name.asString; }

telephon · 2017-06-05T15:43:53Z

source/HOAUGens/sc/Classes/HOAAzimuthRotator3.sc

@@ -1,4 +1,4 @@
-HOARotatorAz3 : MultiOutUGen
+FaustHOAAzimuthRotator3 : MultiOutUGen
 {
  *ar { | in1, in2, in3, in4, in5, in6, in7, in8, in9, in10, in11, in12, in13, in14, in15, in16, azimuth(0.0) |


… same here and all of them, why not array?.

telephon · 2017-06-05T15:46:41Z

source/HOAUGens/sc/Classes/HOAEncEigenMike1.sc

+      ^this.multiNew('control', in1, in2, in3, in4, in5, in6, in7, in8, in9, in10, in11, in12, in13, in14, in15, in16, in17, in18, in19, in20, in21, in22, in23, in24, in25, in26, in27, in28, in29, in30, in31, in32, gain)
+  } 
+
+  checkInputs {


You could make all of these UGens inherit from one AbstractFaustHOAUGen.
for each UGen, you can make a method like this:

*numInputChannels { ^32 }

and then the code below would be:

this.class.numAudioInputs.do({|i|

once and for all of them.

telephon · 2017-06-05T15:47:30Z

source/HOAUGens/sc/Classes/HOARotator1.sc

+      ^this.multiNew('control', in1, in2, in3, in4, pitch, roll, yaw)
+  } 
+
+  checkInputs {


see comment above

telephon · 2017-06-05T15:47:43Z

source/HOAUGens/sc/Classes/HOA_Azimuth_Rotator1.sc

+
+  checkInputs {
+    if (rate == 'audio', {
+      4.do({|i|


… and so on …

Thanks for all these useful comments!
The C++ and the .sc files are all autogenerated from a HOA FAUST implementation made by Pierre Lecomte:
https://github.com/sekisushai/ambitools
so I plan not to change much on that level. But, once the plugins work, I'll publish an HOA Quark that wraps all this in PseudoUgens, with documentation and tutorials:
https://github.com/florian-grond/SC-HOA
so that we don't need to deal with this low level:
http://musinf.univ-st-etienne.fr/lac2017/pdfs/13_C_D_141345N.pdf

Let me know if this answers most of your concerns.

and to answer more specifically the array question, this will be dealt with in the PseudoUGens, i.e. the b-format will be an array.

bagong · 2017-06-05T16:07:13Z

@florian-grond , I'll wait checking the Windows side until you've decided the rest is final, okay? Give me an okay once you've considered other reviews. @LFSaw , maybe you want to have a look, this is Faust-land ;)

nhthn · 2017-06-05T16:33:54Z

let's not spend too much time perfecting the design here. sc3-plugins is a junkyard and it's okay for stuff to be unstable and incomplete

telephon · 2017-06-05T17:27:53Z

@snappizz well if Florian likes, he can of course do it. I've deliberately only commented. No duty!

telephon · 2017-06-05T17:31:21Z

… junkyard is not a very precise description. It reminds me a little bit of the broken window theory …

miguel-negrao · 2017-06-05T17:41:26Z

sc3-plugins is a junkyard

I was surprised by this also... I wasn't aware that sc3-plugins was that unmaintained.

florian-grond · 2017-06-05T18:14:43Z

Well, my plan was not to throw junk into the yard ;-) It is just that Pierre's FAUST lib is very useful, it is 1) structured the way it is. Pierre Lecomte who implemented it did not have SC in mind when he built it, but 2) FAUST generated code just looks that way. Also, I think the SC class files of Faust generated code should not be modified, I believe LFSaw, was of that opinion too at some point.
I think that might be a good moment to discuss how to integrate FAUST generated code in the plugins, as there might be more of this comming in the future.

telephon · 2017-06-05T18:29:45Z

@florian-grond ah good! Would you like to make a separate issue out of this, where we discuss this?

nhthn · 2017-06-05T18:34:43Z

sorry for going off topic here, but let's be honest with ourselves -- most stuff hasn't been touched in years, and there's no shortage of basic quality issues like uninitialized Ctor samples, unprotected calls to RTAlloc, and failure to interpolate control-rate arguments :/

i've personally hit my share of sc3-plugins landmines and just haven't had the motivation to fix them myself. i mostly use core these days.

LFSaw · 2017-06-05T18:54:04Z

Hey there! First of all, this is awesome :)

some things:

IMHO, the faust sources for the UGens should be included in the repo, ideally in a submodule
@telephon, I think your suggestions are valid. However, those are FAUST-generated classes. Within the development of the JPverb UGens, we realised that it is a good idea to leave those alone and build wrapper classes around the "raw" UGens instead. This is what @florian-grond did with the Wrapper UGens in https://github.com/florian-grond/SC-HOA. Actually, I do not see a reason why those should be separated from the sc-plugins UGens.
@florian-grond For including the HOA Ugens into sc-plugins, I'd strongly recommend looking at, adapting and extending the workflow we established for JPverb and Greyhall. You can find a detailed description on this in https://github.com/supercollider/sc3-plugins/blob/master/source/DEINDUGens/faust_src/README.md

florian-grond · 2017-06-05T19:27:55Z

@LFSaw good idea to include the sources. In the HOA case they are generated from a master .dsp script dynamically so there is not one .dsp file per cpp source. I believe a link to Pierre Lecomte's repository should be enough with a clear description of how to use his script.
This is why your template for JPVerb etc only applies in part.

@telephon @LFSaw splitting the plugins from a Quark and additional resources is very much inspired by the ATK, the Quark will also have extensive Tutorial and Guides etc. I actually did not document the UGens as they are very many and autogenerated.

LFSaw · 2017-06-05T19:44:12Z

@snappizz , if this is your feeling, then, well... that's your feeling about it.
Personally, I think we need a plugins-system that integrates into SC as does the Quarks system. So that people can provide their ideas of language extensions the way they think they want, possibly inviting others to review them, and, on the other hand, giving people the freedom to in(/ex)clude portions where they see them fit. So actually, we'd need integration of plugins into the quarks system :)

bagong · 2017-06-05T19:46:49Z

@LFSaw , once @brianlheim 's path-handling reform is brought to the planned point that plugin binaries can be found anywhere where class-files can be found, that should be quite easy.

florian-grond · 2017-06-05T19:50:37Z

I think bagong is still waiting for telephon to approve or disapprove

telephon · 2017-06-05T19:54:27Z

@bagong are you waiting for me?

bagong · 2017-06-05T19:57:25Z

@florian-grond , I think @telephon is among the "liberals" here, as am I ;) So I take your comment as the okay I was waiting for. I am just checking if it builds on Windows, as we have a cross-platform commitment, no intention to act as gatekeeper, just using these fabulous buttons from Github ;)

Thinking a bit into the future: the option to have plugin binaries in class-folders is in reach now. So is the option to use travis and apveyor as build-engines for binaries that could be downloaded into Quarks. SC3 plugins could then be reduced to a binaries repository where we make sure that things build and are cross-platform. And maybe a few tests ;)

LFSaw · 2017-06-05T20:21:49Z

I guess what I want to say is, please make sure that someone else (but you) is able to rebuild the plugin c++ code from the FAUST sources. It would be a shame if all the work would go to pieces because no-one remembers how to do this in 5 years time. Also, IMHO it would be good to explicitly include a link to the wrappers, if you don't include them here. IMHO, the raw sc classes are on the verge of being unusable without the (excellent) wrappers, so they should be mentioned, e.g. in the README file. cheers Till

…

On 05. Jun 2017, at 21:27, fgrond ***@***.***> wrote: @LFSaw good idea to include the sources. In the HOA case they are generated from a master .dsp script dynamically so there is not one .dsp file per cpp source. I believe a link to Pierre Lecomte's repository should be enough with a clear description of how to use his script. This is why your template for JPVerb etc only applies in part. @telephon @LFSaw splitting the plugins from a Quark and additional resources is very much inspired by the ATK, the Quark will also have extensive Tutorial and Guides etc. I actually did not document the UGens as they are very many and autogenerated. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

bagong · 2017-06-05T20:29:30Z

@florian-grond , the good news, it builds now with VS2015. But I get duplicate classes errors? Weird that they only show on Windows:

ERROR: duplicate Class found: 'FaustHOAAzimuthRotator1' 
C:\Users\Rainer\AppData\Local\SuperCollider\Extensions\SC3plugins\HOAUGens\Classes\HOAAzimuthRotator1.sc
C:\Users\Rainer\AppData\Local\SuperCollider\Extensions\SC3plugins\HOAUGens\Classes\HOA_Azimuth_Rotator1.sc

ERROR: duplicate Class found: 'FaustHOAAzimuthRotator2' 
C:\Users\Rainer\AppData\Local\SuperCollider\Extensions\SC3plugins\HOAUGens\Classes\HOAAzimuthRotator2.sc
C:\Users\Rainer\AppData\Local\SuperCollider\Extensions\SC3plugins\HOAUGens\Classes\HOA_Azimuth_Rotator2.sc

ERROR: duplicate Class found: 'FaustHOAAzimuthRotator3' 
C:\Users\Rainer\AppData\Local\SuperCollider\Extensions\SC3plugins\HOAUGens\Classes\HOAAzimuthRotator3.sc
C:\Users\Rainer\AppData\Local\SuperCollider\Extensions\SC3plugins\HOAUGens\Classes\HOA_Azimuth_Rotator3.sc

ERROR: duplicate Class found: 'FaustHOAAzimuthRotator4' 
C:\Users\Rainer\AppData\Local\SuperCollider\Extensions\SC3plugins\HOAUGens\Classes\HOAAzimuthRotator4.sc
C:\Users\Rainer\AppData\Local\SuperCollider\Extensions\SC3plugins\HOAUGens\Classes\HOA_Azimuth_Rotator4.sc

ERROR: duplicate Class found: 'FaustHOAAzimuthRotator5' 
C:\Users\Rainer\AppData\Local\SuperCollider\Extensions\SC3plugins\HOAUGens\Classes\HOAAzimuthRotator5.sc
C:\Users\Rainer\AppData\Local\SuperCollider\Extensions\SC3plugins\HOAUGens\Classes\HOA_Azimuth_Rotator5.sc
````

djiamnot · 2018-06-12T17:47:45Z

Thanks @patrickdupuis for clarification. I was, in fact, using a "contaminated" copy of the sc3-plugins.

* add basic README * small fix

there is a compile flag that defaults to OFF

patrickdupuis · 2018-06-18T19:43:18Z

Whoa! I didn't see that coming 😄

telephon · 2018-06-18T19:49:33Z

was it ok? I am sorry, if that was too quick. I had assumed all was actually done.

florian-grond · 2018-06-18T19:50:57Z

Excellent! very happy, Thanks Patrick for finding a way to make it compatible! Florian www.grond.at

…

On Mon, Jun 18, 2018 at 9:43 PM, Patrick Dupuis ***@***.***> wrote: Whoa! I didn't see that coming 😄 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#146 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABVJxtWzwpSjW-tl-aWLwwEJ8_A2tx_Lks5t-ALYgaJpZM4Nuqz7> .

patrickdupuis · 2018-06-18T19:53:20Z

@telephon yes, it is fine. I had just added a README to the PR before you merged.

LFSaw · 2018-06-18T19:55:30Z

so, default flag for this is "Off"?

telephon · 2018-06-18T19:56:09Z

Because the temperature is rising, I was worried that it would get stale before we could eat it ...

telephon · 2018-06-18T19:56:34Z

so, default flag for this is "Off"?

as it seems, yes. Should it perhaps be ON?

LFSaw · 2018-06-18T19:57:13Z

no, leave it Off please. For now at least (since it does not compile on all platforms)

telephon · 2018-06-18T20:01:53Z

yes, that was my thinking.

florian-grond · 2018-06-18T20:05:39Z

I just made some suggestions offlist how we might be able to partially switch it on, let's keep it off for now www.grond.at

…

On Mon, Jun 18, 2018 at 10:01 PM, Julian Rohrhuber ***@***.*** > wrote: yes, that was my thinking. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#146 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABVJxh8ouKJwzVqteGxtylGZfNPM61kqks5t-AczgaJpZM4Nuqz7> .

telephon · 2018-06-18T20:08:36Z

actually, trying to switch it on in my usual build folder seems not to work. Maybe I'm doing something wrong.

cmake -DSC_PATH=/Volumes/xyz/supercollider -HOA_UGENS=ON ..

and then

cmake -L

shows me

HOA_UGENS:BOOL=OFF

patrickdupuis · 2018-06-18T20:11:42Z

try with a second build folder called build2 and see if you get the same result.

telephon · 2018-06-18T20:16:54Z

still the same

patrickdupuis · 2018-06-18T20:19:54Z

Oh! it's -DHOA_UGENS (you forgot the D)

mossheim · 2018-06-18T20:35:19Z

The HOAUGens are unable to build using MSVC due a limitation in the depth of bracket nesting that is permitted by this compiler.

Almost all the plugins do build with MSVC, it's only three that have a problem. Is there any plan to provide a convenient way to skip the bad ones (HOADecBinaural3/4/5)? Such as just excluding those plugins when MSVC is detected? How do you expect anyone to provide a testable build otherwise? Same with RPi, sounds above like HOADecBinaural5 doesn't build there and the rest could be used.

patrickdupuis · 2018-06-18T20:38:15Z

Yes - this is what @florian-grond is thinking about doing as a next step. For now, though, this is the way it had to be in order to move this PR forward. Maybe we should open an issue?

mossheim · 2018-06-18T21:50:55Z

For now, though, this is the way it had to be in order to move this PR forward.

??? Is there a link to a discussion? There is no consensus in the discussion above.

jamshark70 · 2018-06-18T23:30:49Z

At least the PR appears to be incomplete. AFAICS the readme does not explain the new cmake option nor that HOAUGens in their current state should not be expected to build in Visual C or perhaps on RPi as well.

Without this documentation as a bare minimum (or full compatibility), merging was premature.

patrickdupuis · 2018-06-18T23:55:13Z

I agree that this shouldn't have been merged as is. I was intending to discuss this PR at the next dev meeting and reaching a consensus.

At this point, we should be suggesting necessary changes (maybe in #197) so we can make them happen.

telephon · 2018-06-19T07:51:20Z

Sorry, my misunderstanding. I had understood that the cmake option is off by default, and the changes in themselves were very well maintained with a clear responsibility.

So as far as I can tell, what is really missing is a few sentences of documentation of this option and its limitations.

florian-grond · 2018-06-19T09:07:48Z

As Brian has pointed out, almost everything compiles except for a few Ugens HOADecBinaural3/4/5, 1) they either cause a problem on Windows because of the deep bracket nesting 2) they cause a problem on small platforms like the RPi because compilation is resource intensive. These two problems will likely come back as new plugins are being developed in HOA. Not for many but for some plugins. At some point in the future, the Faust compiler might not create these big expressions anymore, but we should not wait for that. On platforms like the RPi, and other small boards, it is very unlikely that one would do Ambisonics work with very high orders. I believe it would be possible to detect MSVC but to detect the RPi seems more tricky. Things like HOADecBinaural3/4/5 are useful but not absolutely necessary. I would go for Brian's suggestion: Let's explicitly exclude certain orders of UGens from the official built with an OFF flag so that it runs on all supported platforms but for those who compile the plugins everything is in the public repo and can be included if desired. for now: if (X in HOADecBinauralX.ccp > 2) -> do not compile or more general: if (X in HOAWhateverX.ccp > 2) -> do not compile If anyone could suggest a way to do that correctly in the CMakeList file that would be great. Florian www.grond.at

…

On Tue, Jun 19, 2018 at 9:51 AM, Julian Rohrhuber ***@***.***> wrote: Sorry, my misunderstanding. I had understood that the cmake option is off by default, and the changes in themselves were very well maintained with a clear responsibility. So as far as I can tell, what is really missing is a few sentences of documentation of this option and its limitations. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#146 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABVJxroWNawVEoTbfGTbdgHda2fEXWicks5t-K16gaJpZM4Nuqz7> .

patrickdupuis · 2018-06-19T13:33:13Z

@florian-grond let's leave this thread and move the discussion to #197.

nhthn · 2018-06-20T02:22:56Z

i, for one, am fine with this merge. since the ugens are turned off by the default in the build system, it won't cause problems for users who don't specifically install them.

added HOAUgens

0fd70e1

nhthn requested changes Jun 2, 2017

View reviewed changes

added Readme

2aea881

nhthn approved these changes Jun 2, 2017

View reviewed changes

bagong assigned bagong and unassigned bagong Jun 2, 2017

bagong previously requested changes Jun 2, 2017

View reviewed changes

compiled HOAUgens with most recent Faust

c49ebfc

also added them to the CMakeLists.txt where I also added: set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fbracket-depth=512") since the Faust autogenerated code creates deep bracket nestings for higher orders

telephon reviewed Jun 5, 2017

View reviewed changes

patrickdupuis added 2 commits June 12, 2018 15:28

improve the option's description (#6)

6a039c4

Add a basic README (#7)

55cd622

* add basic README * small fix

telephon approved these changes Jun 18, 2018

View reviewed changes

telephon merged commit 98df066 into supercollider:master Jun 18, 2018

mossheim removed the waiting for consensus label Jun 18, 2018

added HOAUgens #146

added HOAUgens #146

Conversation

florian-grond commented Jun 2, 2017

nhthn left a comment

Choose a reason for hiding this comment

florian-grond commented Jun 2, 2017

nhthn commented Jun 2, 2017

florian-grond commented Jun 2, 2017

nhthn left a comment

Choose a reason for hiding this comment

bagong left a comment

Choose a reason for hiding this comment

florian-grond commented Jun 5, 2017

telephon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bagong commented Jun 5, 2017 • edited Loading

nhthn commented Jun 5, 2017

telephon commented Jun 5, 2017

telephon commented Jun 5, 2017

miguel-negrao commented Jun 5, 2017

florian-grond commented Jun 5, 2017

telephon commented Jun 5, 2017

nhthn commented Jun 5, 2017

LFSaw commented Jun 5, 2017 • edited Loading

florian-grond commented Jun 5, 2017

LFSaw commented Jun 5, 2017

bagong commented Jun 5, 2017

florian-grond commented Jun 5, 2017

telephon commented Jun 5, 2017

bagong commented Jun 5, 2017 • edited Loading

LFSaw commented Jun 5, 2017 via email

bagong commented Jun 5, 2017

djiamnot commented Jun 12, 2018

patrickdupuis commented Jun 18, 2018

telephon commented Jun 18, 2018

florian-grond commented Jun 18, 2018 via email

patrickdupuis commented Jun 18, 2018

LFSaw commented Jun 18, 2018

telephon commented Jun 18, 2018

telephon commented Jun 18, 2018

LFSaw commented Jun 18, 2018

telephon commented Jun 18, 2018

florian-grond commented Jun 18, 2018 via email

telephon commented Jun 18, 2018

patrickdupuis commented Jun 18, 2018

telephon commented Jun 18, 2018

patrickdupuis commented Jun 18, 2018

mossheim commented Jun 18, 2018

patrickdupuis commented Jun 18, 2018

mossheim commented Jun 18, 2018

jamshark70 commented Jun 18, 2018

patrickdupuis commented Jun 18, 2018

telephon commented Jun 19, 2018

florian-grond commented Jun 19, 2018 via email

patrickdupuis commented Jun 19, 2018

nhthn commented Jun 20, 2018

bagong commented Jun 5, 2017 •

edited

Loading

LFSaw commented Jun 5, 2017 •

edited

Loading

bagong commented Jun 5, 2017 •

edited

Loading