Add Polyphony to the SimpleOSC classes

[baconpaul]

I had seen on community.vcvrack that you had asked if anyone would help introducing polyphony. I did this with both the surge and baconplugs at the 0.6 -> 1.0 transition and it's not that hard. Especially now you are GPL3 happy to toss in a bit of help here and there.

This pull request adds polyphony to all 4 of your simple oscillators (saw, square, tri, sin). If you want I'm happy to look at others. I'm also happy for you to throw out this commit altogether of course!

A few comments

1. The source code seemed a mix of tabs and spaces. Which would you rather have? As a result some of the diff looks bigger than it actually is

2. I'm not sure what your test regimen is. I pulled your oscillator in with the BaconPlugs module "PolyGenerator" and listened to beethoven and back polyphonically with no problem with all 4 modules

3. Not sure which if any of your other modules would benefit from polyphony.

Hope this helps. All yours if you want it! And again no hard feelings i you don't like this or want it changed or some such.

[baconpaul]

Oh also this is a pretty simple polyphony implementation; just make N instances of your oscillator class. Didn't do any cross-voice SIMD or any such things at this point. CPU load still seems very low.

[baconpaul]

The + and Simplex oscillators look like the could be done exactly the same way. The wavetable may be a bit trickier. The logic and math modules are simply adding a loop. Let me know if you are interested in me looking at any of those also.

[thomassidor]

Hi Paul

Thank you so much for contributing with this - you rock! :)

It would be awesome to implement polyphony in the other modules as well, so feel free to look into it if you want.

Regarding your comments:

1. I have no strong opinions on this, but think I use tabs mostly. Any spaces are probably leftovers from copying+pasting.

2. Honestly I have no structured test regimen. I try out a few things and if things seems to work, then it's being released. So what you did seems to be well in line with that :)

3. The other oscillators would definitely as well. And TT-L and TT-A too. You're right that it's perhaps a bit tricky with WAVE as it is now. Doing a thorough refactor and rewrite of it is on the todo-list though. After that it might be easier to approach. The current implementation is horrible.

I'm thinking we can do an aggregated polyphony release with them all, if you're up for it?

I just started work on a "Noise Wrangler" which isn't going to touch any of the other modules, so the branching and merging should be quite trivial.

But perhaps I should look into refactoring WAVE first so we can add polyphony to that as well?

[thomassidor]

I've started refactoring WAVE :)

[baconpaul]

Hi and thanks

So cool. Adding to the other OSC+ and stuff is easy andI will leave wave alone

Why don’t I just force push it all to this pull request. I think I may even be able to bang that out today!

And I’ll set my emacs to “tab == 2 spaces” which seems to be what you are using.

Oh also, we should think about whether you would like me to assign copyright to you, or if you want a blended copyright GPL3 repo. I’m fine with either in this case and I don’t think we need to be all that formal, but you should decide which one you prefer and we should at least indicate it either in the license file (if you want blended) or this issue (if you want assignment).

[thomassidor]

Sounds great!

It might take me a few days to finish refactoring WAVE since I don't have too much time to spare.

As for copyright - I'm not that well versed in the different options. I guess blended copyright just mean that we're both listed in the main copyright statement, right. But how does assignment copyright work practically?

[baconpaul]

OK I just force pushed the branch with all the modules done. If you merge it you will get polyphony on all the oscillators except wave as well as the math and logic. Here's how I tested it

Copyright assignment usually is a contributors agreement. For now I'm happy to just state that I assign the copyright on these (trivial) changes to you in this issue and we can not worry about it. It's only really an issue if you decide to unGPL3 and sell the code. When you hold sole copyright you can; if you do not you need the consent of all the copyright holders. But since this is just me helping a hand for an hour, and the code is GPL3 and you want it open, I am fine to either be listed as an author or have you be OK with the simple statement here. Your choice!

[baconpaul]

Oh and that "PolyGenerator" modules is from "BaconPlugs" which you can grab in the plugin manager.

[thomassidor]

Great work!

I've pushed a refactored and improved (I hope) version of WAVE to dev-wave-refactor. You can have a look at it and see if it's more straightforward to implement polyphony in it now.

Could we possibly merge this into that branch? I'm not sure how to do that.

As for the copyright, let's do as you suggest. I'll add a an author section to the readme.md before pushing to master. Also I have no plans to unGPL3 it.

[baconpaul]

I took a peek. The trick is that the dumb way of doing it (vectorize on oscillators) doesn't share the capture state between them so may be a bit more refactor. But I'm happy to look.

As to merge - if you are amenable perhaps we could do this. Why don't you merge your branch to master, then I can rebase this without wav polyphony and you can then merge this, and then I can take a deeper look at the wavetable oscillator over the long weekend here in the states. Sound good?

[thomassidor]

Perhaps we could just add polyphony to out and modulations - but leave the waveform and the capturing non-polyphonic? Maybe that's easier to start with.

Not sure if I understand what you're suggesting in terms of merging. Usually I don't merge things to master until it's completely done and ready to push to the library.

[baconpaul]

Oh ok cool. In surge land we use master a bit more liberally and make release branches so ignore my suggestion.

It is super easy for me to grab your commute for the wav into my branch and make a suggested change. I’ll take a peek

My comment really meant: you have 16 voices with the same wavetable but different modulations. So playback and modulate is polyphonic but capture is monophonic. But your data structure captures into the oscillator object so either all the poly oscillators need to share a state or when you capture you need to make 16 copies. One of those two. I’ll look at the code and figure it out. If you have a test network you use to exercise the module that would be super handy too!

[thomassidor]

Sorry, I don't really have a test setup for that module either. But let me know if there's any other questions.

By the way, I just finished up a new random/noise module in dev-noisewrangler (https://github.com/thomassidor/tinytricks/tree/dev-noisewrangler).

I'm thinking we'll release that together with your changes as 1.5.0 when you're done.

[baconpaul]

Great. Sorry for the delay - got pulled into a couple of things with the holiday weekend. I'll get to this shortly - maybe even today!

[baconpaul]

OK I just merged your dev-wavetable branch into my poly branch and before I make any changes it crashes quite a lot, mostly it seems around the scope object. I'll debug and see where I get but just want to be sure that dev-wavetable is the latest still?

Ignore that comment. The fault was mine.

[baconpaul]

OK cool I just did a push here which does a couple of things

1. It merges your dev-wavetable-refactor branch into this poly-osc branch
2. It deletes your WaveTable object in the destructor of the WAVE module and
3. It adds polyphony to Wave in a way which works in my tests.

I got it playing beethoven no problem!

So I think (think) we are either good to go or at least good to release to testers with this pull request.

Let me know how you'd like to proceed. Happy to help with git mechanics and stiff if you need that or also happy to leave it up to you.

Oh also, don't know if you ever saw this, but on my rack modules (both surge and bacon plugs) I use azure pipelines to automatically build releases and the like I outlined how to do it here https://community.vcvrack.com/t/azure-pipelines-for-github-allows-build-of-plugin/2651 and that thread has other folks who have done it also. Microsoft gives away free build and CI compute resources to open source projects on GitHub, and it's super powerful!

Have a lovely weekend.

[thomassidor]

Awesome Paul!

Just had a look through it and saw this comment in simplex-oscillator.cpp: "// Mirror. FIXME this becomes an array". Is this something we need to worry about for now?

As for merging - ideally I want this pull request and dev-noisewrangler to be joined as one pull request into master and then released as 1.5.0. Other than one small commit (https://github.com/thomassidor/tinytricks/commit/49033ec078c7ad6067f8e83b9000d050f1519e9e) they should share the same code base, so it shouldn't be too hard.

Before it going all the way we just need to remove the mention of v1.4.3 from your pull, and add the notes from there into a 1.5.0 section.

What do you think the best strategy for doing that would be?

You are very welcome to just go ahead with what you feel is right.

[thomassidor]

Oh, and thanks for the tip about Azure. Will look into that.

[baconpaul]

Oh that fixme is wrong! I can knock that out now and repush if you want.

Do you want to merge the pull requests or do you want me to merge dev-noisewrangler into this one also?

[baconpaul]

OK pushed with those two FIXME comments removed or cleaned up.

Lemme know if you want to pull this into your branch or want me to pull it into yours.

Alternately you could merge this into master now and then merge yours in, and then tag as 1.5.0.

[thomassidor]

If you can pull it into dev-noisewrangler that would be great. Then I'll clean up the docs and pull to master and tag as 1.5.0.

[thomassidor]

I should be able to do this later today.

[baconpaul]

I can pull dev wrangler into here but can’t push to dev wrangler

I’m offline for a bit also driving sorry!

[thomassidor]

No worries. I'll pull it into dev-noisewrangler, clean up and then into master 👍

[thomassidor]

Ok, I've merged the whole thing to dev-noisewrangler, and fixed a few small things (the scope in SN-OSC wasn't working great). See commit https://github.com/thomassidor/tinytricks/commit/04fc317e7e277a4ca523412307c68d1f6e52ac6d - btw. seems we still use different whitespace :)

However there's still some issues that I'm yet to figure out the exact scenarios for:

1. Rack sometimes freezes when trying to add a new instance of WAVE.
2. Recapturing wavetable in WAVE sometimes cause crash.

I can't look into it right now, and probably not until Monday. So you're more than welcome to do so if you feel like it.

[baconpaul]

Yeah ok I think there may be a bad scope interaction and object lifetime. I’ll take a look and send a fresh pr if I find it!

[baconpaul]

warn src/widget/FramebufferWidget.cpp:74] Framebuffer of size (nan, nan) * 1.000000 could not be created for FramebufferWidget.

Definitely something going wrong somewhere. I'll see if I can find out more about that message, of which I get one per WAVE right now, and an occasional crash on exit.