-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
scsynth: adding BelaScopeUGen #75
Conversation
I see now that my diff for BELAUGens.cpp is big.. it's actually full of trailing space correction that I haven't done, I guess they happened automatically. The only changes I made are about BelaScopeUGen, near the end of the file. |
thanks! I know mostly nothing about Sc's ugens, I have only contributed to another one before. Is |
Thank you!
As far as I understand, Scope is supposed to plot at audio rate, i.e. to call |
yes and yes!
so that you can write a more C++-style constructor/destructor for If you are resubmitting a patch, please do not include the whitespace changes that your text editor added and also fix the indentation where appropriate. It seems that you are mixing tabs and spaces somewhere, e.g.:
|
2da2ebc
to
1a424fd
Compare
thanks, is there anything that could be added to prevent instantiating more than one instance of the class? There can be only one |
For a start, what do you think about giving a single Scope object to the server itself? I think we could add it to
I think safety concerns about calling setup() multiple times can be mitigated (actually eliminated IMO) by wrapping Scope.setup() in a function that always calls stop() and cleanup() first if scope is running. What do you think? |
I'd rather avoid that. We are aiming to merge upstream, so the more we can keep our changes out of the way of regular Sc code, the better, so the approach of using UGens seems fine to me. What I was thinking is more having a static flag that can be used to check the number of instantiated objects. This is easy to do in C++, but I was wondering how to communicate the failure back to the language.
One could just delete the Another interesting feature would be to have a trigger input to the scope for custom triggering. |
Ok, I see your point, but wouldn't the addition of a Scope object inside the BELA ifdef in SC_World still be compatible with the idea of merging? Then maybe we skip creating new primitives and server methods, and handle everything else in the UGen. On the other hand it looks relatively easy to put the limit on sclang-side, but this would be unreliable if you have multiple clients (e.g. one on Bela and the other on host) |
Is there a way for the
You are right, I didn't realise we already have an
Sounds good! Would you be able to do the above? * I guess it's a sensible idea to have it there then. I am a bit concerned about having anything at all in |
I'm on it... but I'm a bit confused about linking. If SC_World requires Scope, does it mean we have to link libbelaextra to scsynth, and add Bela "root folder" to include_directories? |
That seems right yes, and undo your earlier change to |
I added a commit where we create and store a Scope() object in SC_World, before the AudioDriver starts, and we call the scope's Now we have a single Scope object per server, but I haven't yet figured out a way to call It seems to me that everything a UGen does is done on the audio thread (that's why RTAlloc for frameData in the constructor), so it's ok to call scope->log(), but apparently not setup(), maybe because of WSServer or AuxTask allocations? |
Thanks for your new commit. I seem to understand that now we have a fixed number of channels, but it would be a better option to have them set up in the server options as you previously suggested (or better
yes! If I understand correctly, one can still instantiate multiple copies of the ugen, which is going to cause trouble (because calling One other alternative is to hook into Sc's own |
An alternative is to use the bus system as it is, reserve a bus language-side, and put in some guards for not doing that twice. The bus is read by a BelaScopeUGen, which calls |
abecc8b
to
67d1095
Compare
Extra question: sometimes I get this warning in the IDE (now it happened after I left the tab in the background, with a test project running for a long time, like more than one hour). And of course scoping doesn't work anymore afterwards.
I got it sporadically since the first version of this PR... can you give me any clue about what is it? |
Update: I implemented a ServerOption ( -O belaMaxScopeChannels ), and single-instance-guards for sclang and BelaScopeUGen. Apart from that sporadic WSClient memory error I reported above, I feel good about this implementation :)
Let me know what you think, when you have time of course. |
this essentially means that you are sending too much data to the audio-to-websocket pipe, that is not sent via the websocket to the in-browser Scope fast enough. In practice, it often boils down to using too many Scope channels with a too high refresh rate for the CPU available. A common solution is increase the holdoff in the Scope GUI. Could you please provide a Is there a way to add another "trigger" input channel to the scope? The server should be notified at setup time that a trigger channel is active, so that it can switch the A couple more issues:
Here's a patch that should fix these two issues: diff --git a/SCClassLibrary/Common/Control/Server.sc b/SCClassLibrary/Common/Control/Server.sc
index 396c944a7..87f432f78 100644
--- a/SCClassLibrary/Common/Control/Server.sc
+++ b/SCClassLibrary/Common/Control/Server.sc
@@ -60,7 +60,7 @@ ServerOptions {
var <>adcLevel;
var <>numMultiplexChannels;
var <>belaPRU;
- var <>belaMaxScopeChannels = 2;
+ var <>belaMaxScopeChannels = 0;
var <>recHeaderFormat="aiff";
var <>recSampleFormat="float";
diff --git a/server/plugins/BELAUGens.cpp b/server/plugins/BELAUGens.cpp
index b9e72384d..8fd7b4f62 100644
--- a/server/plugins/BELAUGens.cpp
+++ b/server/plugins/BELAUGens.cpp
@@ -1379,11 +1379,11 @@ void BelaScopeUGen_Ctor(BelaScopeUGen *unit)
uint32 numInputs = unit->mNumInputs;
uint32 maxScopeChannels = unit->mWorld->mBelaMaxScopeChannels;
if(numInputs > maxScopeChannels) {
- rt_printf("BelaScopeUGen warning: can't scope %i channels, maxBelaScopeChannels is set to %i\n", numInputs, maxScopeChannels);
+ rt_fprintf(stderr, "BelaScopeUGen warning: can't initialise scope %i channels, maxBelaScopeChannels is set to %i\n", numInputs, maxScopeChannels);
}
BelaScopeUGen::instanceCount++;
if(BelaScopeUGen::instanceCount > 1) {
- rt_printf("BelaScopeUGen warning: creating a new instance when one is already active. This one will do nothing.\n");
+ rt_fprintf(stderr, "BelaScopeUGen warning: creating a new instance when one is already active. This one will do nothing.\n");
SETCALC(BelaScopeUGen_noop);
return;
};
diff --git a/server/scsynth/SC_Bela.cpp b/server/scsynth/SC_Bela.cpp
index 4730530fd..9355f75bd 100644
--- a/server/scsynth/SC_Bela.cpp
+++ b/server/scsynth/SC_Bela.cpp
@@ -148,7 +148,7 @@ bool sc_belaSetup(BelaContext* belaContext, void* userData)
// cast void pointer
SC_BelaDriver *belaDriver = (SC_BelaDriver*) userData;
gBelaSampleRate = belaContext->audioSampleRate;
- belaDriver->mBelaScope->setup(8, gBelaSampleRate);
+ belaDriver->mBelaScope->setup(mBelaMaxScopeChannels, belaContext->audioSampleRate);
return true;
}
diff --git a/server/scsynth/scsynth_main.cpp b/server/scsynth/scsynth_main.cpp
index 451933847..600d7eefe 100644
--- a/server/scsynth/scsynth_main.cpp
+++ b/server/scsynth/scsynth_main.cpp
@@ -184,7 +184,7 @@ int main(int argc, char* argv[])
options.mBelaDACLevel = 0;
options.mBelaNumMuxChannels = 0;
options.mBelaPRU = 1;
- options.mBelaMaxScopeChannels = 2;
+ options.mBelaMaxScopeChannels = 0;
#endif
for (int i=1; i<argc;) { There are still a few whitespace errors (which you can see with |
also, I'd be interested in hearing if @sensestage has any suggestion on this |
Thanks for looking at it!
Here is a simple sketch that scopes stereo inputs + 6 random sine waves: s = Server.default;
s.options.numAnalogInChannels = 8;
s.options.numAnalogOutChannels = 8;
s.options.numDigitalChannels = 16;
s.options.blockSize = 64;
s.options.numInputBusChannels = 2;
s.options.numOutputBusChannels = 2;
s.options.maxLogins = 4;
s.options.postln;
s.options.belaMaxScopeChannels = 8;
s.waitForBoot({
SynthDef("help-scope",{ arg out=0;
var in = SoundIn.ar([0,1]).belaScope(0);
var sin = SinOsc.ar(TExpRand.kr(50,1000,Dust.kr(1!5)).lag2(1)) * LFNoise2.ar(1!5).exprange(0.01,1);
sin.belaScope(2);
Out.ar(out, Pan2.ar(sin,[-1,1])+in);
}).play;
}); |
012d3c0
to
2d93d82
Compare
correct
I am actually thinking: if one wants to use "custom" triggering, they shall just send a trigger audio-rate signal into one scope channel, and select that as the triggering channel from the Scope frontend. No need to add any extra support for it. I think we are good to go (or as good as we can get with the current design). I will wait for a couple of days to see if @sensestage has any comments. In the meantime I will merge this on top of my mainlining attempt and build a beta release. |
great, yes please
Test after I build the release. I will continue the conversation in #74 |
This is a squash and reformatting of several commits that originally made up #75 BelaScope: create in SC_World, setup in SC_Bela ServerOptions: -O belaMaxScopeChannels BelaScope: single instance and valid inputs checks BelaScopeUGen: single instance guard BelaScope: add .belaScope to Server, Bus and Function BelaScope: maxScopeChannels defaults to 0
Great I squashed and reformatted this here. Here's a build, please test it (I enjoyed the scope example, thanks!): https://github.com/BelaPlatform/supercollider/releases/tag/v3.11-bela2 Note: if you want to build this yourself, you need the very latest Bela master, as I changed a couple of things in EDIT: updated link to bela2 |
This is a squash and reformatting of several commits that originally made up #75 BelaScope: create in SC_World, setup in SC_Bela ServerOptions: -O belaMaxScopeChannels BelaScope: single instance and valid inputs checks BelaScopeUGen: single instance guard BelaScope: add .belaScope to Server, Bus and Function BelaScope: maxScopeChannels defaults to 0
Updated the release to include Ableton Link support and Xenomai IPC: https://github.com/BelaPlatform/supercollider/releases/tag/v3.11-bela3 |
Just FYI: I will be unable to test this properly until at least next thursday. Sorry, I'm drowning in other works |
This is a squash and reformatting of several commits that originally made up #75 BelaScope: create in SC_World, setup in SC_Bela ServerOptions: -O belaMaxScopeChannels BelaScope: single instance and valid inputs checks BelaScopeUGen: single instance guard BelaScope: add .belaScope to Server, Bus and Function BelaScope: maxScopeChannels defaults to 0
Hi there! Yes, I do have suggestions. (Admittedly, I haven't read through the whole thread, so just sharing my initial thoughts) My initial go at this ugen was to have one ugen BelaScopeChannel, to scope just one channel, and one UGen that can just be instantiated once that refers to the BelaScope, and that sets the amount of channels to use. So it does not have to be a startup parameter. Back in the day I did not get around to making it all work out, but this was my initial start. It avoids having to add things to the World struct.
|
@sensestage
Overall, I am not sure the added complexity is worth the trouble, if the only gain you get from this is to have a variable number of scope channels, but I'd be happy to hear your thoughts on this. Remember that I know very little about the inner workings of
A good summary of the discussion is in the current implementation, which is all contained in 2fc8ce6 and can be tested with the code in #75 (comment) . |
No worries. Make sure you always test the latest release. |
@sensestage another thing about your implementation: would there be need for a |
@sensestage
So we ended up giving a What do you think? |
@elgiano any luck in testing this? |
@giuliomoro I'm giving it a go now! Tried the oscilloscope test script above and it all works, I did some back and forth between the latest release and my version from git, and I can't see any noticeable difference.
after "SuperCollider 3 server ready."
Hope that can be useful for you, and other than this, I don't really have projects on which I can test this release, so I'm open for any suggestion about testing it :) |
thanks, the first few lines are expected. The others not so much. Have you updated your Bela to the latest |
Got me, I had forgot that. With the latest Bela master I only get these
"warnings" indeed
```
-- cold init from program
-- cobalt->init()
-- connected to Cobalt
Initialize_xenomai
-- memory locked
-- memory heaps mapped
-- boilerplate->init()
-- program bootstrap done
```
And everything still works :) Meaning that I get sound out and Oscilloscope
is plotting. I've tested only with my Oscilloscope test patch and
SuperCollider's default project.
…On Sat, Nov 7, 2020 at 1:00 PM giuliomoro ***@***.***> wrote:
thanks, the first few lines are expected. The others not so much. Have you
updated your Bela to the latest master and run make -C ~/Bela lib to
update libbela which is used by Sc ?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#75 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACNMWWRPNDGED7GVCSSJP3SOUZFHANCNFSM4SPEML3Q>
.
|
Great, I think I will submit for upstream merge soon |
although first I'd like to fix a few more outstanding issues: |
Great, although I still have to submit documentation for this! I'm almost done with it, shall I make another PR or shall we reopen this? Also, if you have time, please consider this (BelaPlatform/Bela#645) contribution to the main Bela repo, where I added syntax highlighting for SC code :) I'll look into the other issues as well! |
Open a new one against the
Thanks very much, I shall do that soon. I will also need an example for the scope to add to the main repo, so you can submit a separate PR there for that if you want. On that note, I wanted to add the |
This is a squash and reformatting of several commits that originally made up #75 BelaScope: create in SC_World, setup in SC_Bela ServerOptions: -O belaMaxScopeChannels BelaScope: single instance and valid inputs checks BelaScopeUGen: single instance guard BelaScope: add .belaScope to Server, Bus and Function BelaScope: maxScopeChannels defaults to 0
This is a squash, reformatting and rebase of several commits that originally made up #75 and #83 Co-authored-by: Giulio Moro <[email protected]>
This is a squash and reformatting of several commits that originally made up #75 BelaScope: create in SC_World, setup in SC_Bela ServerOptions: -O belaMaxScopeChannels BelaScope: single instance and valid inputs checks BelaScopeUGen: single instance guard BelaScope: add .belaScope to Server, Bus and Function BelaScope: maxScopeChannels defaults to 0
This is a squash, reformatting and rebase of several commits that originally made up #75 and #83 and #84 Co-authored-by: Giulio Moro <[email protected]>
This is a squash, reformatting and rebase of several commits that originally made up #75 and #83 and #84 Co-authored-by: Giulio Moro <[email protected]>
Purpose and Motivation
Addressing issue #6, I'm proposing a bus-based implementation for accessing Bela's Scope from SuperCollider.
Not sure if it's the best implementation, even less sure if the way I link Scope through CMake is optimal. It seems to work fine on my system though.
Known issue: there is a frame drop when BelaScopeUGen is created. Could it be because a new Scope() is created on the server, and that allocation is not real-time safe?
Types of changes
Checklist
Remaining Work