multichannel support

[ben-wes]

and another discussion disguised as an issue. :)

---

it would be nice to have multichannel support for signals in pdlua. the (probably?) obvious way might be to have a table of tables in that case, where #in for perform(in) would tell the number of channels. but this is not backward compatible and possibly also not very efficient? ... although there could also be a new type like MC_SIGNAL for the inlets/outlets definitions.

another option might be to just have a flat table with all samples for all channels? the blocksize can be obtained through the dsp() function - so the number of channels could be derived in other ways (maybe also written somewhere from dsp() function?).

for outlets, the number of channels would need to be defined some way though, i guess ... not sure what the syntax there should be and what perform() should return.

[agraef]

I'm not sure what's the best way to do this. I still need to look into Pd's multi-channel implementation.

[agraef]

Ok, since I just ported multi-channels to purr-data, my understanding improved a little, but not by much. 😉 But it's good enough that I have an informed opinion about the proposed feature now.

So basically you add CLASS_MULTICHANNEL to the class_new call. You get the number of input channels for each signal in the dsp callback, and you can use signal_setmultiout to set up multi-channel output signals. That part seems easy. But I'm not so sure about the perform routines. There must be some serious magic going on behind the scenes in the engine, as these seem to be virtually the same as before on the C side. So I'm still not sure how we'd implement a multi-channel perform callback in Lua land. It surely can be done, but then there's the issue with representing the multi-channel sample data in a backward-compatible way in Lua that you mentioned already. It also complicates the API, because you now need to specify not only the signature of signal and data inlets and outlets, but also the number of channels in each of the signal inlets and outlets.

Can you please remind me again why we should go through all that hassle? 😕 Because to me this seems to be a mere convenience for something that could easily be achieved using a combination of snake~ out to unpack the multi-channel signals, a standard Lua dsp object with multiple signal ins and outs, and snake~ in to re-pack the signals again. Which would offer the additional benefit that you can configure the number of channels in the multi-signals any way you like. And you can also just package those three objects in a neat little abstraction if needed. Wouldn't that give you everything you want, and more, with the currently available facilities already?

Sorry if this comes across as just me being too lazy to implement that feature -- which is true, I'm a lazy bum indeed, but that's also because I work on so many different projects. 😛 But IMHO, if someone is going to invest a substantial amount of time implementing this feature, it should add some real value, and not complicate things too much. Thoughts?

[ben-wes]

TL;DR (before you read the other comments below): i'm not completely convinced that we need this. i'm also a bit ambivalent there since Lua is certainly not the right place to deal with signals of really high channel counts. so the use cases i'm think of are only applications like scope3d~, where it would just be aesthetically more pleasing to have a single input connection for 3 channels.

ah - flexible channel counts would be a thing, too, of course (that are not managed in initialize, but in dsp).

But I'm not so sure about the perform routines. There must be some serious magic going on behind the scenes in the engine, as these seem to be virtually the same as before on the C side.

i'm certainly not well versed when it comes to Pd's engine - but isn't the magic in that case achieved by representing multichannel signals in the same vectors, but with size blocksize * n_channels, where the samples of additional channels are expected at signal vector pointer + n * blocksize?

[...] but then there's the issue with representing the multi-channel sample data in a backward-compatible way in Lua that you mentioned already.

the obvious way here would probably be to also have a flat table that includes n * blocksize samples instead of the current blocksize samples. that would also be backward compatible?

It also complicates the API, because you now need to specify not only the signature of signal and data inlets and outlets, but also the number of channels in each of the signal inlets and outlets.

similarly to the Pd side, this would certainly have to be defined for the signal outlets, yes. for the inlets, it could be derived in the dsp method, right?

Sorry if this comes across as just me being too lazy to implement that feature -- which is true [...]

all good - and that's definitely a good protection against feature creep here. :)
thanks for the thoughts and detailed responses on this!

[agraef]

ah - flexible channel counts would be a thing, too, of course

I agree.

but isn't the magic in that case achieved by representing multichannel signals in the same vectors, but with size blocksize * n_channels

Ah yes, you're right, it's even documented in the t_signal declaration in m_pd.h. This makes a lot more sense now. :)

the obvious way here would probably be to also have a flat table that includes n * blocksize samples instead of the current blocksize samples. that would also be backward compatible?

Correct. I now think that this should be doable without too much effort.

• The dsp method would have an extra third table argument with the channel counts of all input signals, that could just be ignored by objects which don't use that information.
• We'd have to add some wrapper for signal_setmultiout() to make it possible to set up multi-channel outputs in the dsp method.
• The perform method would deliver input signals and receive output signals as flat Lua sample tables as before. We'd just have to make sure that the C perform callback knows about the proper sample counts of each argument and result table, depending on the number of channels for each of those vectors.

Yes, that sounds like a plan to me. Ben, do you want to give it a go yourself?

[porres]

I really hope you people get to implement this. I'm sure it'll be very very powerful, but I'm not doing a good job here selling the thing out. Or let me try it... I am currently adding MC support to most of the signal externals in ELSE. I never thought about that until MC was a reality in Pd and now I can see the benefit and it'd be great if one could code externals in lua that explore this.

Just build it and they will come...

[ben-wes]

• The dsp method would have an extra third table argument with the channel counts of all input signals, that could just be ignored by objects which don't use that information.
• We'd have to add some wrapper for signal_setmultiout() to make it possible to set up multi-channel outputs in the dsp method.

sounds good to me and i don't see alternatives here.

• The perform method would deliver input signals and receive output signals as flat Lua sample tables as before.

yep - that sounds like the right way to adopt Pd's mechanism. a table of tables representing a multichannel signal might be more convenient, but possibly also not ideal performance-wise and overcomplicate things?

do you want to give it a go yourself?

i'd be happy to - but this might be a bit over my head still (or possibly forever, haha). i'll see how far i get and let you know!

before doing so, i'd really love to hear @timothyschoen's opinion on the API, too!

[agraef]

a table of tables representing a multichannel signal might be more convenient, but possibly also not ideal performance-wise and overcomplicate things?

I don't think that it would have much impact on the performance. The main issue is that a table of tables is not compatible with the existing API, which breaks existing code. With a flat table the user callbacks don't even need to be aware of multi-channel input and still do something sensible with the sample data. Also, many existing perform callbacks would still work fine with multi-channel data without any changes. So I think that this is the preferable solution. If we document this properly and with a nice example, then having to work with the flat tables shouldn't be much harder than with single-channel data. It's just that you need to calculate the indices in a row-major fashion (something like i = (k-1)*blocksize+j where k is the 1-based channel index). Like we did in the old days when programming in Assembler or dialects of Basic and Fortran without multi-dimensional arrays. 😉

i'll see how far i get and let you know!

That's the right spirit! 👍 There's a lot of examples of little Lua-callable functions and how to call Lua code from C in the code already, so I hope that you'll find your way. If not then don't hesitate to ask.

[agraef]

Also, https://www.lua.org/manual/5.4/ is your friend. If you scroll down a bit to the Index, all the Lua C API functions can be found in the two columns on the right. The reference documentation can be a bit terse and hard to understand if you never used that API before, but again, just ask if you need help -- I'm well familiar with the Lua C API by now.

[agraef]

before doing so, i'd really love to hear @timothyschoen's opinion on the API, too!

I'd say that you can just go ahead with this, I'm sure that Tim will agree that it's the most sensible approach. And we can always massage the code later, or Tim might do that himself if he has ideas on how to improve the API. If we don't have any code, there's nothing to massage and this will be stuck in the idea stage forever, I'm afraid. I'm paraphrasing Alexandre's remark. 😄

[agraef]

Also, I'm currently busy with #57 and will possibly tackle #59 afterwards, so I probably won't be able to tend to this ticket myself for quite some time. And I'm doing the finishing touches of the upcoming purr-data release, which is a really big one. The winter semester is also approaching, and I still have to prepare lots of stuff for that as well. Long story short, just embark on it or it may take an eternity. 😅

[timothyschoen]

before doing so, i'd really love to hear @timothyschoen's opinion on the API, too!

I'd say that you can just go ahead with this, I'm sure that Tim will agree that it's the most sensible approach. And we can always massage the code later, or Tim might do that himself if he has ideas on how to improve the API. If we don't have any code, there's nothing to massage and this will be stuck in the idea stage forever, I'm afraid. I'm paraphrasing Alexandre's remark. 😄

Yep, sounds good, I think a flat table is the best way to do it.

@ben-wes Good luck, let me know if you need any help! I also think we'll need a new function to set the number of output channels?

[agraef]

That's actually part of the proposal above:

• We'd have to add some wrapper for signal_setmultiout() to make it possible to set up multi-channel outputs in the dsp method.

[agraef]

And we might also want to skip all those shiny new features for older Pd versions (testing for PD_MINOR_VERSION >= 54).

[ben-wes]

sooo ... i fiddled around a bit to achieve some kind of result. it's certainly still far from mergeable - but maybe the api can be finalized at this point.

this is missing the version checks for multichannel capabilities and probably also many other improvements and i'm honestly not sure if i can finalize it.

anyway - here's a little lua test outputting the 3 channels of 2 inlets in inverted order to its 3 outlets. let me know if this looks like the right approach to you:

local testmc = pd.Class:new():register("testmc")

function testmc:initialize(sel, atoms)
  self.inlets = {SIGNAL, SIGNAL}
  self.outlets = {SIGNAL, SIGNAL, SIGNAL}
  return true
end

function testmc:dsp(samplerate, blocksize, inchans)
  self.blocksize = blocksize
  pd.post(string.format("samplerate: %d, blocksize: %d, inchans: %s",
    samplerate, blocksize, table.concat(inchans, " ")))
  self:signal_setmultiout(1, 2)
  self:signal_setmultiout(2, 2)
  self:signal_setmultiout(3, 2)
end

function testmc:perform(in1, in2)
  out1 = {}
  out2 = {}
  out3 = {}
  for i = 1, self.blocksize do
    out1[i]                  = in2[i + self.blocksize * 2]
    out1[i + self.blocksize] = in2[i + self.blocksize]
    out2[i]                  = in2[i]
    out2[i + self.blocksize] = in1[i + self.blocksize * 2]
    out3[i]                  = in1[i + self.blocksize]
    out3[i + self.blocksize] = in1[i]
  end
  return out1, out2, out3
end

the current draft:
https://github.com/agraef/pd-lua/compare/master...ben-wes:feature/multichannel?expand=1

[ben-wes]

made a few more changes and additions now. should i create a draft PR from this?

[ben-wes]

btw, i realized that we didn't add prototypes for the newly added functions in pdlua.c ... and then was wondering if we should do that and move them all to pdlua.h?

[ben-wes]

... here's a patch and 2 lua objects (one multichannel and one single channel) for testing this: test-lua-mc.zip

[ben-wes]

and one more question ... the version checks here are obviously only compile time checks.

do we want the version on Deken to be compatible also with older Pd versions? in that case, we'd have to deal in a different way with signal_setmultiout - i could give it a try with this dlsym mechanism, which is already in the pdlua code.

i'm not yet sure how to properly deal with sp[i]->nchans yet though - would need to check first.

[agraef]

btw, i realized that we didn't add prototypes for the newly added functions in pdlua.c ... and then was wondering if we should do that and move them all to pdlua.h?

Not sure what functions you're talking about (sorry, short on time, didn't look at your changeset yet), but if they are functions to be called only from Lua then they should actually be static, and not go into the header.

and one more question ... the version checks here are obviously only compile time checks.

Good point.

do we want the version on Deken to be compatible also with older Pd versions? in that case, we'd have to deal in a different way with signal_setmultiout - i could give it a try with this dlsym mechanism, which is already in the pdlua code.

Yeah, I've used that to detect purr-data which obviously uses different gui calls (JS instead of Tcl). But that's a much simpler situation which can be accommodated quite easily.

If your code uses internal structs in any way, then this gets a lot messier. It's not just the nchans field; the whole layout of the struct has been changed in an incompatible way IIRC. So I'm afraid that pdlua will just crash when used with an old version thinking it has the new data structure because it has been compiled against 0.54 or later. :( I just had to deal with a similar case in purr-data where moonlib/dinlet~ was still using the old vinlet struct leading to inconsistencies which then caused crashes (kind of the reverse situation, but I think that the other way round won't fare much better).

I'd have to look at the code, though, to see whether this might be case here. As I said, short on time right now, but I'll have a look asap.

[agraef]

Quickly glanced at the code and yes, I think that we're in trouble. :(

Not sure what to do about this. Either we need to tell people to upgrade to 0.54+, or we need to make this optional and compile the Deken version without the feature to be on the safe side for now. Or have two separate versions of pdlua on Deken, one for pd-0.53 and earlier, and one for pd-0.54 and later. Which is probably going to be a maintenance and support nightmare. :(

[ben-wes]

but if they are functions to be called only from Lua then they should actually be static, and not go into the header.

ah ... nevermind - that was a stupid suggestion. but i might add the prototypes for consistency in pdlua.c then.

Not sure what to do about this.

i'll give it a try. i had copied christof's mechanism of vstplugin for pmpd when i added multichannel support there. maybe we can do the same here if that looks good to you? see for example:

https://github.com/ch-nry/pd-pmpd/blob/03c3494bafbc3af1ab8fe145221ad0cdea202124/pmpd~.c#L520-L535

[agraef]

i had copied christof's mechanism of vstplugin for pmpd when i added multichannel support there. maybe we can do the same here if that looks good to you?

That might do the trick. At least it's worth to give it a go. But please make sure that you actually test with both new and old pd versions. :)

If that doesn't pan out -- I already started formulating the approach below before I saw your reply, so this might be our "plan B":

I think that it would be a good idea to add some #ifdef NO_MULTICHANNEL or so, which lets us force a build of the non-mc version by invoking make with -DNO_MULTICHANNEL, independently of the PD_MINOR_VERSION.

For the Deken version, I think that we can build the new mc version by default, but you then need to add a check which makes sure that people don't try to run that version on old pd versions. That is, check the runtime pd version at startup, and tell people in the sign-on message that they need to grab an older pdlua version if they're still running pd 0.53 or older, and then just disable the signal Lua API. Just making people's old pd versions crash if they try to do signal stuff is not an acceptable way to inform the user that something isn't working. ;-)

That would require, though, that older pdlua versions remain available on Deken if we upload a new version, and that people know where to find them. Do you know whether that's possible? Otherwise, we'd have to tell people to grab the older versions on GH.

In purr-data this is much easier, because pd-lua is included so I can just use whatever is available (which would be to enable mc in purr-data 2.19.4 and later).

[ben-wes]

But please make sure that you actually test with both new and old pd versions. :)

will do! and thanks for your thoughts on this!

That would require, though, that older pdlua versions remain available on Deken if we upload a new version, and that people know where to find them. Do you know whether that's possible? Otherwise, we'd have to tell people to grab the older versions on GH.

old versions stay available in vanilla through deken as well - just not too obvious maybe. can't check right now and might be dependent on a deken setting.

[ben-wes]

adding to my last comment: installing previous versions (and many of them!) is quite easy if the setting "Only show the newest version of a library" isn't checked (not sure about the default there right now):

[ben-wes]

ddb4435 should now allow building pdlua with pre-multichannel Pd versions and running both builds with different Pd versions. Tested:

Pd build src	Pd test version	result
0.53-2	0.53-2	✅
0.55-1	0.53-2	✅
0.53-2	0.55-1	✅
0.55-1	0.55-1	✅

EDIT: adding here ... i'm not really sure if i properly handled all cases and also not quite sure if that's the proper way now. it becomes a bit confusing (at least for me when looking at it after a long day) with the combinations of compile time checks and runtime checks. :)

[agraef]

installing previous versions (and many of them!) is quite easy if the setting "Only show the newest version of a library" isn't checked

Thanks, that's good to know! Not sure why such an important option isn't readily visible in the main dialog, but hidden away in that convoluted Preferences menu.

it becomes a bit confusing (at least for me when looking at it after a long day) with the combinations of compile time checks and runtime checks. :)

Yes, these things make my head hurt, too. I'll have a look asap.

Unfortunately, it might take a while until I get to this, because at present I'm fighting some bugs in my upcoming purr-data 2.19.4 release, and the bugs are getting weirder -- like the fiddle~ external crashing which seems to be due to a compiler or linker bug on Arch Linux. Apparently, the Arch maintainers thought that it would be a good idea to base (and bet) their distro on an unreleased git version of gcc 14.

[agraef]

Apparently, the Arch maintainers thought that it would be a good idea to base (and bet) their distro on an unreleased git version of gcc 14.

OTOH, if they don't give the very latest gcc a thorough shakeout then who will? Even Debian testing and unstable are still on gcc 14.1.0 AFAICT.

[ben-wes]

at present I'm fighting some bugs in my upcoming purr-data 2.19.4 release, and the bugs are getting weirder

thanks for the update! hope, you're making some progress there! - i'm afraid that i can't contribute any ideas or experience in that context.

about the multichannel stuff: i added a few comments in #60 ... those are the points where i'm hesitant at them moment. but obviously, the whole thing will need a review unfortunately.

[ben-wes]

@timothyschoen @agraef : i stumbled upon one little detail here where i'd appreciate your opinions - currently, pdlua's dsp function passes the blocksize based on this line:

pd-lua/pdlua.c

Line 1129 in 1d8ea81

lua_pushnumber(__L(), sys_getblksize());

... this gets the configured blocksize of Pd. i'm now wondering whether this is what we want in all cases like a reblocked or resampled subpatch/abstraction - which can be different from Pd's blocksize. we could possibly read this local size from sp[0]->s_n - but i'm honestly not sure which one is more appropriate here?

[agraef]

Well, in the dsp method sys_getblksize is all we got, I guess. Or is there a canvas-local block size in the Pd API that we could tap into?

[ben-wes]

is there a canvas-local block size in the Pd API that we could tap into?

as long as there is at least 1 iolet, sp[0]->s_n is holding the local blocksize. my previous comment was partly wrong though: resampling doesn't change the local blocksize, but reblocking obviously does. i tested that and it looks good - so the question is which one we want here?

EDIT: i pushed 5963279 for this now. feels right to me.

[agraef]

That commit doesn't seem to be in the PR anymore. Can you please update the hash so that I can take a look at what you did there?

as long as there is at least 1 iolet, sp[0]->s_n is holding the local blocksize

Ok, that seems reasonable. But is that value always guaranteed to be nonzero, or do we handle that condition?

[ben-wes]

Can you please update the hash so that I can take a look at what you did there?

ah, sorry - sure. it's here:
5963279#diff-be05f819a0338ff6475de5edabb88815ee6956c1944dd154823801493d4b4539R1154

is that value always guaranteed to be nonzero, or do we handle that condition?

AFAICT, it is guaranteed if there's a SIGNAL in/outlet. i also checked with only a DATA iolet and without iolets and in those cases, the dsp function isn't called - which makes sense, of course. i'm not 100% sure if this is always guaranteed, since i would need to prove it with the dsp logic in pd/pdlua, which i admittedly didn't do.

[agraef]

Ok, looks good to me. BTW, did you test with a float input instead of signal?

[agraef]

Never mind, just tested it myself using your test patch, works fine.