jamulus icon indicating copy to clipboard operation
jamulus copied to clipboard

Add ctrlmidich option for "own channel" regardless of number

Open pljones opened this issue 1 year ago • 22 comments

Short description of changes

From @AndersGoran.

This adds a ;z option to --ctrlmidich parsing that adds control of the client channel, regardless of its channel number. This applies to all the channel-specific controls used. It takes up one extra MIDI controller number for each.

CHANGELOG: Add ctrlmidich option for "own channel" regardless of number

Context: Fixes an issue?

Raised in https://github.com/orgs/jamulussoftware/discussions/2220#discussioncomment-10811813

Does this change need documentation? What needs to be documented and how?

Yes, but it needs reviewing to see if this is how we want to do it first.

Status of this Pull Request

Proof of concept.

What is missing until this pull request can be merged?

Needs reviewing and considering whether "own client id" concept should apply to all controls (e.g. use ;z as a switch to bump lock the first CC number for each control to "own client id").

Checklist

  • [x] I've verified that this Pull Request follows the general code principles
  • [ ] I tested my code and it does what I want
  • [x] My code follows the style guide
  • [ ] I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • [ ] I've filled all the content above

AUTOBUILD: Please build all targets

pljones avatar Oct 02 '24 18:10 pljones

There may currently be a problem with CSoundBase::ParseCommandLineArgument in that it doesn't necessarily set every element of aMidiCtls for each sMidiCtlChar but it doesn't check in CSoundBase::ParseMIDIMessage that the element exists.

So I'd like the ParseCommandLineArgument changed to ensure each possible option gets set to some "unset" default value and then overridden by the command line argument.

Separately, I'd like ;z<n> changed to just ;z and have that toggle "Use Own Channel Id In First CC For Control" mode -- so it's not just Fader that gets the benefit, it's Pan and Mute, too. Or have ;z<n> with <n> being the offset into the usual range of CC numbers for each control.

pljones avatar Oct 02 '24 18:10 pljones

OK, so I decided to make the change I suggested.

Once the build is complete for Windows, I'll give it a try here.


The legacy definition has just one or two numbers that only provides a definition for the controller offset of the fader controllers (default 70 for the sake of Behringer X-Touch)

[MIDI channel];[offset for first fader]

The more verbose new form is a sequence of offsets and counts for various controllers:

[MIDI channel];[control letter][offset]*[count](;...)

Currently, the following control letters are defined:

Control control letter
Fader f
Pan p
Solo s
Mute m
  • offset is the base MIDI CC number for the control.
  • count is the number of CC values for the control (i.e. the number Jamulus channels that can be controlled).

Additionally, o has a single offset (i.e. count is ignored and taken as 1) and controls Mute Myself.

In addition, z reserves the first CC number for a control to mean "my Jamulus channel". For example

--ctrlmidich '1;f0*9;z'

would mean MIDI CC0 controlled my Jamulus channel fader, with CC1 to CC8 for Jamulus channels 1 to 8 (so if you were channel 6, you get two MIDI CC controls).

An example for a Korg nanoKONTROL2 with 8 fader controllers starting at offset 0 and 8 pan controllers starting at offset 16 would be

[MIDI channel];f0*8;p16*8

However, at the current point of time only 'f' and 'p' controllers are actually implemented. // may not be true...

pljones avatar Oct 04 '24 19:10 pljones

Ugh, failure due to qt5 -> qt6. Investigating.


Fixed (more a compiler issue, I think).

pljones avatar Oct 05 '24 08:10 pljones

OK, I can't get JACK to run without crashing on Windows any more for some reason. So I can't actually test this (as Jamulus ASIO doesn't support MIDI, as far as I know, and I've no MIDI - or audio - on my Linux box).

pljones avatar Oct 05 '24 11:10 pljones

@AndersGoran could you check whether the latest build is useable?

It should work like this:

--ctrlmidich 0;z;f0*9 gives you MIDI CC 0 controlling "my" fader, regardless of server-assigned Jamulus channel number, MIDI CC 1 controlling Jamulus channel 0 through to MIDI CC 8 controlling Jamulus channel 7.

It should work for all the Jamulus controls.

--ctrlmidich 0;z;f0;p1;s2;m3;o4 should give you control only over your own channel for fader, pan, mute (um, no, don't) and mute myself (better). --ctrlmidich 0;z;f0*9;p1*9;s2*9;m3*9;o4 gives you your own channel, plus additional controls for up to eight Jamulus channels, which could include your own channel. (If you were on Jamulus channel 3, you'd get MIDI CC 0 and MIDI CC 4 controlling your fader, for example; if you were on Jamulus channel 17, you'd still get control over your own channel and the first eight Jamulus channels.)

Note: remember, having control over your own channel only affects what you hear, not what others hear.

pljones avatar Oct 05 '24 16:10 pljones

I cloned https://github.com/pljones/jamulus.git, current HEAD is 3f0983e2, compiles fine (I'm on macOS) but I'm afraid the "z" flag makes no difference. As before, my controller sends CC 100-103 from four separate pedals, so I use "0;z;f100*4" and the 100 pedal controls channel 0, 101 control 1, and so on. When I'm on e.g. 1, I have to use the pedal that sends 101 to move my fader.

Is there a pre-built executable somewhere that I should be using?

AndersGoran avatar Oct 05 '24 17:10 AndersGoran

Yes. Checkout the Checks tab of this PR. There you'll find the macOS artifact for this PR.

ann0see avatar Oct 05 '24 19:10 ann0see

So I suppose it is the DMG linked here that I want: https://github.com/jamulussoftware/jamulus/actions/runs/11194714268/artifacts/2019359055

I'm afraid this one does nothing at all related to MIDI. Not with the usual string without "z", nor with "z".

Here's how I invoke it:

[~/Desktop]$ exec /Applications/Jamulus.app/Contents/MacOS/Jamulus --ctrlmidich "0;z;f100*4" "$@"
- MIDI controller settings: 0;z;f100*4
- allocated port number: 22134
QString::arg: Argument missing: fpsmoz,

That last QString::arg looks a bit off - fpsmoz - aren't those exactly the char's we allow in the midi ctrl argument?

AndersGoran avatar Oct 05 '24 19:10 AndersGoran

Okay, thanks, let me take a look.

pljones avatar Oct 06 '24 10:10 pljones

OK, new build there.

(I can't get mt Jamulus for Linux build to see Pipewire -- PW is running, qpwgraph and both pavucontrol are fine but Jamulus runs Jack and doesn't appear as a PW app.)

pljones avatar Oct 06 '24 15:10 pljones

Sorry, same with this one. The "fpsmoz" thing is gone, though. No MIDI action at all. Official 3.11 client is fine (but of course not with "z" flag), but this one does nothing with or without "z". Installed from https://github.com/jamulussoftware/jamulus/actions/runs/11202345633/artifacts/2020757377

AndersGoran avatar Oct 06 '24 18:10 AndersGoran

Thanks. Hm. Clearly I'm not understanding the code... I'll back out my changes for now.

pljones avatar Oct 07 '24 17:10 pljones

OK, once that's built, can you check it works?

pljones avatar Oct 07 '24 17:10 pljones

Extra debugging logging in my version:

$ ./Jamulus --ctrlmidich "0;z;p1;f2;m3"
- MIDI controller settings: 0;z;p1;f2;m3
- allocated port number: 22134
"sParm: 1*1*1"
"eTyp: 1; iFirst: 1; iNum: 1; bMyChannel: 1"
"aMidiCtls[1 + 0] = { (eType) 1, (iChannel) -1 }"
"sParm: 0*2*1"
"eTyp: 0; iFirst: 0; iNum: 2; bMyChannel: 1"
"aMidiCtls[0 + 0] = { (eType) 0, (iChannel) -1 }"
"aMidiCtls[0 + 1] = { (eType) 0, (iChannel) 0 }"
"sParm: 3*3*1"
"eTyp: 3; iFirst: 3; iNum: 3; bMyChannel: 1"
"aMidiCtls[3 + 0] = { (eType) 3, (iChannel) -1 }"
"aMidiCtls[3 + 1] = { (eType) 3, (iChannel) 0 }"
"aMidiCtls[3 + 2] = { (eType) 3, (iChannel) 1 }"

Well, that's wrong, isn't it! It's getting the type, offset and count right in sParm and then my loop populates aMidiCtls wrongly.

"aMidiCtls[0 + 1] = { (eType) 0, (iChannel) 0 }"

comes along and overwrites

"aMidiCtls[1 + 0] = { (eType) 1, (iChannel) -1 }"

for example. At least now I can see something is wrong...


Edit: And once I had the above, it was fairly easy to spot...

pljones avatar Oct 07 '24 18:10 pljones

OK: (this is now out of date, of course)

Branch (includes this PR) https://github.com/jamulussoftware/jamulus/compare/main...pljones:jamulus:ctrlmidich-for-own-channel-all-controls
Just the extra diff https://github.com/pljones/jamulus/commit/3ba65c611c1692c61ef3b6b4ab4fb77cb9e50c4a
Builds https://github.com/pljones/jamulus/actions/runs/11221638882

pljones avatar Oct 07 '24 18:10 pljones

Now we're talking! I've tested briefly on a server where "0:" is someone else, and I am "1:".

If I start this latest build (macOS non-legacy is what I use) with 0;f100*4 - without z - then everything is as it used to be, my first pedal CC100 controls "someone else" and CC101 controls my fader. As expected.

When I add z, i.e. 0;f100*4;z, then the first pedal, CC100, controls my fader, CC101 controls "0:", i.e. the first non-me channel. But I also notice that CC102 controls my fader too. Maybe that's expected, maybe not.

But at least, now the z flag is recognised and it does what I believe we want it to do. Great!

AndersGoran avatar Oct 07 '24 20:10 AndersGoran

I think we should test this a bit more on sessions with many participants. Now I connected to a server where I got channel 0 - there had been lots of people on it earlier but many had disconnected, so the few ones left were 6, 7, 8 or something like that. As I have 4 CC controllers configured, all I could control was my own fader because I had the "z" flag.

But that's not really relevant for this PR, is it?

I can certainly do some more digging, but there's only so many hours in a day... it seems I should be able to clone the branch mentioned above and do some testing, right?

Anyway, thanks for the effort of getting it to where it is now!

AndersGoran avatar Oct 07 '24 20:10 AndersGoran

Yes, you'll have your own channel number. Start out setting something like 0;f100*2;z, that should give you MIDI Ctrl 100 for your channel, whatever number it is and 101 for Jamulus channel 0. If you're not channel 0, you'll still be able to control your fader. If you are channel 0, you'll have 100 as the "sticky" number and 101 just because you're on Jamulus channel 0 and that's what 101 controls.

Are you trying the build here or from the link I posted, by the way?

pljones avatar Oct 08 '24 18:10 pljones

I was trying the macOS non-legacy build from https://github.com/pljones/jamulus/actions/runs/11221638882

I think the main purpose of the PR is fulfilled, that one can be sure to have control over one's own fader.

AndersGoran avatar Oct 08 '24 18:10 AndersGoran

Coding Style Check / Verify Python coding style (pull_request)

Why did that fail?


https://github.com/jamulussoftware/jamulus/issues/3408 raised.

pljones avatar Oct 12 '24 11:10 pljones

I have the feeling that documentation is lacking on explaining the code. I feel unsure about the exact functionality - especially the cli parsing.

Which documentation?

What are you unsure about?

pljones avatar Oct 15 '24 17:10 pljones

I think I need to go over the code in my editor. The diff on GitHub doesn't show me enough.

ann0see avatar Oct 17 '24 21:10 ann0see

I know I'm rather late to the party here, due to limited time, but have today been studying and thinking about this solution to @AndersGoran's original issue.

I don't like the fact that when using the z option, there will be two faders that control the user's own channel: the first fader (by virtue of the z option), and the fader assigned to the user's actual channel. This seems user-unfriendly, wasteful of a physical fader, and with the potential for confusion, if the two faders, pans or mute/solo buttons are set to different states.

I would like to propose a different solution (apologies for not doing so until now!):

  • The user's own channel always gets given the internal fader channel 0, whatever client ID it is on. So my fader would always look like 0:TonyM.
  • Any other channels with a client ID less than my own client ID will be given the internal fader channel of ID+1.
  • Any other channels with a client ID greater than my own client ID will be given the internal fader channel equal to their ID as at present.

So a session where I am TonyM might look like this:

Client ID on server Name Fader ID Tag on mixer
0 Tom 1 1:Tom
1 Dick 2 2:Dick
2 TonyM 0 0:TonyM
3 Harry 3 3:Harry
4 Jim 4 4:Jim

That way, there is only one fader channel per client, and my own fader is always the first one. These fader IDs are local to my own Jamulus client, and don't need to be the same as those on any other user's client.

Since the user never has any control over which client IDs are assigned by the server, it doesn't matter that the other user's fader number doesn't necessarily equal their client ID, and the above technique would ensure the first MIDI fader is always the user's own, since they always get fader channel 0. This wouldn't even need the extra z flag in the --ctrlmidich string; there would be no downside in making the above always the case.

This would also work well with @pljones recent "Sort by Channel" option in #3418 (where Channel is the fader ID).

Comments?

softins avatar Oct 30 '24 17:10 softins

It's probably best if @AndersGoran comments.

pljones avatar Oct 30 '24 17:10 pljones

@softins I agree.

The user's own channel always gets given the internal fader channel 0, whatever client ID it is on. So my fader would always look like 0:TonyM.

This could potentially also simply the own fader first implementation

ann0see avatar Oct 30 '24 18:10 ann0see

Thank you @softins, for looking and thinking about this. Your suggestion sounds good to me.

My use case has always been such that I want a predictable way to assign one single MIDI CC to always control my fader. I don't really care about any other faders than my own, in any of the groups I play in on Jamulus.

I just want to be able to mute myself in my own mix with MIDI CC. If I could trust that the first CC given in --ctrlmidich always controls my fader, that's all I need.

AndersGoran avatar Oct 30 '24 19:10 AndersGoran

So if I've got five sliders assigned and I always want slide 1 to be "me", I'd expect 2, 3, 4 and 5 to be someone else, agreed.

I'd also expect them to be in order on the mixer board as my hardware controller, with "me" on the left -- which is what "Sort by Server Channel" would do, normally, if you "overwrote" the Server channel with the renumbered value like this.

What I wouldn't want is for the mixer and hardware order to mismatch - 2 3 0 4 5 vs 1 2 3 4 5

(And, of course, if channel 4 disconnects, controller 4 wouldn't control anything - I'd have to see "5" and know what it meant.)

Indeed, "Own channel first" could always do this -- after numbering the channels according to the sort, renumber with the proposed rule, before displaying. (Currently it moves the channel from where it is to the first position, regardless - same effect, different method.)

pljones avatar Oct 31 '24 17:10 pljones

I'm in the middle of doing a proof of concept for my suggestion, which we will be able to try out in a few days. It's quite complicated working out the best way, due to the onion-like many layers in the software structure: Client; ClientDlg; MainMixerBoard; Faders; with many signal/slot connections between them.

One thing we can't do is overwrite the iChanID, since that by definition must match the channel ID on the server. So the MIDI channel numbers must be stored or calculated independently on the client side, and the mapping between them and channel IDs done either algorithmically or via lookup tables. I'm currently trying the former approach as per my suggestion further up.

softins avatar Oct 31 '24 23:10 softins

Lookup table sounds more performant and less messy.

ann0see avatar Nov 01 '24 07:11 ann0see

Lookup table sounds more performant and less messy.

Yup - the current version uses a lookup table.

However you do it, you need to cater for both the situation where the user wants the reserved channel and when they don't want it. (It's perfectly reasonable for someone to want controller three, or whatever, to control channel 3, regardless of whether it's their own channel or someone else's.)

But at the time you parse the controller values, you can't populate the map from controller to channel for "my channel", as you don't know what number it will be, so if you don't want to reserve a slot for it, you need another map on top of the existing one, I think, remapping the numbers every time someone joins (or leaves, though that's not important). There's currently something happening in the UI control to keep the mixer channels in order -- but, really, I'd rather MIDI control of the Client was not dependent upon the UI being enabled.

It's getting to the point I'm thinking "We've got CClientDlg and CClientRpc both controlling CClient, why not get CClientMidi, too, for MIDI control, and give it MIDI output, too, to reflect changes?" Overhaul CClientDlg, CClientRpc and CClient to communicate properly over the signal/slot interfaces (only, as far as I'm concerned -- CClient would define public slots for controls and emit signals with changes in state - that it held -- and would have public read-only calls to read state).

pljones avatar Nov 01 '24 19:11 pljones