Skip to content
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

Make client-side channels independent of server-side channels #3423

Closed
softins opened this issue Nov 11, 2024 · 6 comments · Fixed by #3426
Closed

Make client-side channels independent of server-side channels #3423

softins opened this issue Nov 11, 2024 · 6 comments · Fixed by #3426
Assignees

Comments

@softins
Copy link
Member

softins commented Nov 11, 2024

I'm opening this issue so that implementation can be discussed independently of #3419, which will be superseded by this approach. I'll copy the relevant comments across with attribution.

Originally posted by @softins in #3419 (comment)

I still disagree. I'm effectively virtualising the channel numbers so that own channel is always 0.

If that's the intent, discussing "MIDI" throughout the approach is highly misleading.

Well, yes, but that concept only came to mind when you started also relating JSON-RPC to it.

OK, let's take this concept and run with it, although without the "virtual" term.

Currently, there is no distinction between "client-side" channel and "server-side" channel. The iChannelID or iChannelIdx, and also the iMyChannelID, currently refer to the channel index allocated within the connected server and sent to the client. The Jamulus protocol by definition uses the server-side channel number to identify channels.

Let's decouple the client-side and server-side channel numbers, and make "client-side channel" a concept local to an individual client, mapping it within the client at the lowest level possible to the "server-side channel". Something like this within client.h:

class CClientChannel {
  ...
  int iServerChannelID;
  ...
};

CClientChannel clientChannels[MAX_NUM_CHANNELS];

Maybe we put other items in there that are currently just arrays indexed by server channel index. Or maybe not, but they would now be indexed by client-side channel ID.

  • When the client connects to a server (unless the server is ancient), it receives a CLIENT_ID message before anything else, containing the server-side channel allocated to that client. We assign that server channel to client channel 0, always, and put the received ID into clientChannels[0].iServerChannelID.

  • When we receive a list of connected clients from the server, for any new server channel in the list, we allocated the lowest free client channel to it and again store the server channel in iServerChannelID. For existing known server channels, we just look up the associated client channel to update its data if necessary.

  • If connected to an ancient server that doesn't send CLIENT_ID, the channels will just be allocated as received, so the "own" channel could be anywhere. That's unavoidable, but hopefully very rare. In that case, we don't reserve client channel 0, and it will get used by the first server channel to be seen.

  • Any time we pass an iChannelIdx or iChannelID around within the client hierarchy, it is referring only to a client channel.

  • Server channel numbers only get used when processing or sending protocol messages. If we have a server channel ID and need to find the associated client channel, we can either do a quick linear search for matching iServerChannelID or can maintain a lookup table of CClientChannel* pointers integer client channel IDs, indexed by server channel ID. Obviously, to convert from a client channel ID to a server channel ID for sending to the server, we just index the CClientChannel array/vector by client channel ID and fetch iServerChannelID. Both of these operations should be local to the CClient.

  • Once a server channel has been assigned to a specific client channel, that assignment will not be changed until that server channel disappears again. This will avoid things like fader assignments jumping unexpectedly. When a server-side channel disappears due a remote client disconnecting, our client-side channel that it was associated with gets marked as free for re-use (iServerChannelID = INVALID_INDEX;), but other channels do not get moved to occupy the freed client channel.

  • Midi controller number offsets can then be used unchanged to index into client-side channels, and channel 0 will always be the client's own channel (except with an ancient server). There will be no need for the kludgy z option in --ctrlmidich, nor for a duplicate fader.

  • Other potential operations such as a JSON-RPC method would also identify channels only by client-side channel ID. As would the GUI Audio Mixer Board.

  • "No sorting" will by default be by client-side channel ID, so "Sort by Server Channel" would no longer be required. I can't see any reason to sort by actual server-side channel number. Actually on second thoughts, it might still be required for the same reason it was introduced, due to the current allocation strategy of the faders in the GUI. But it could just be renamed to “Sort by Channel”, as it would be sorting on the client-side channel number.

  • This technique would also overcome the observation of @AndersGoran that when he connected to a server where the only other user had a high-numbered server channel, he could not control it with a MIDI fader.

@softins
Copy link
Member Author

softins commented Nov 11, 2024

Originally posted by @pljones in #3419 (comment)

OK, from a GUI perspective - all the existing sort options would continue to work as is, except "Sort by Channel", where "My Channel" would automatically be first (and thus adding "Own channel first" would have no visible effect; still useful on the other options). --ctrlmidich would show "client channel" and those would always start in range. The client would no long "care" what server channel was used. So:

A server with one person on where they got server channel 10 allocated when they joined.
I join:

  • the server allocates me server channel 0 (as it's now free) and I store that in clientChannels[0].iServerChannelID
  • the server tells me there's someone else on server channel 10 and I store that in clientChannels[1].iServerChannelID (because that's my next free channel as I'm just joining)
  • someone else joins and gets server channel 1 (that's also now free) and I store that in clientChannels[2].iServerChannelID

Now two different scenarios - not problems, just to think through:

  • the person who was on leaves, freeing server channel 10 and I free clientChannels[1].
  • Scenario 1:
    • someone else new joins, gets server channel 2 and I store that in clientChannels[1].iServerChannelID because it's the lowest free
    • the person who was on and left rejoins almost immediately and gets server channel 3, that in clientChannels[3].iServerChannelID
  • Scenario 2:
    • the person who was on and left rejoins almost immediately and gets server channel 2, that goes in clientChannels[1].iServerChannelID
    • someone else new joins, gets server channel 3 and I store that in clientChannels[3].iServerChannelID because it's the lowest free

OK, I think that makes sense, too -- if you leave and rejoin, the likelihood is you'll get the same clientChannels index, right? If you leave and someone else joins before you get back, hard luck, you're in a different place on the desk now.


EDIT:

  • For "No User Sorting" (which probably shouldn't say "User" according to the Style guide), the definition is that we always add a newly entering person to the highest mixer position. This is achieved internally by assigning the display vector a sort count number. I'm not keen on the current implementation. If we could add "join sequence" to CClientChannel and have that automatically set when a channel is allocated to from a constantly incrementing number (per server session "I" join), then sort by that, I think it would make the View->Sort process easier, as it would always be getting channel information.

@softins
Copy link
Member Author

softins commented Nov 11, 2024

Originally posted by @pljones in #3419 (comment)

From there, GUI, JSON-RPC and MIDI control of CClient messaging to the server to adjust the mix:

  • I think CClient needs to have the channel volume level stored (so the GUI fader just tells CClient to adjust that, and CClient knows to tell the server to adjust the mix, JSON-RPC and MIDI make the same call); if the value is changed, CClient emits a "volume changed" signal (the GUI fader tracks this to follow changes from JSON-RPC and MIDI but ignores it whilst doing its own update to avoid "stuttering").
  • Same for group, mute and solo settings -- currently these are embedded in the GUI but should be managed in CClient to allow JSON-RPC and MIDI control not to rely on the GUI.

So I think this is a good first step (and a discrete change from the above).

@softins
Copy link
Member Author

softins commented Nov 11, 2024

Originally posted by @pljones in #3419 (comment)

I'm wondering if we need to avoid making CClient (or, rather, client.cpp) too complicated -- should the concept of the mixer (which spans audio I/O and network I/O conceptually) be handled by CClient? Should CClient get pared down to managing the client protocol message requests and responses?

That would mean the existing "GUI-only" mixing desk would be split into a class that was independent of CClient and the GUI that just managed all the interaction between CClient and the CClientChannel[]. The GUI would then be a visual presentation layer over that, and there would also be JSON-RPC and MIDI "presentation" layers.

I'd almost not mind all of that in one file with a bunch of #ifndef ... for each of GUI, JSON-RPC and MIDI exclusion options.

@softins
Copy link
Member Author

softins commented Nov 11, 2024

Originally posted by @ann0see in #3419 (comment)

That would mean the existing "GUI-only" mixing desk would be split into a class that was independent of CClient and the GUI that just managed all the interaction between CClient and the CClientChannel[]. The GUI would then be a visual presentation layer over that, and there would also be JSON-RPC and MIDI "presentation" layers.

I was honestly also thinking about this for a while. From the perspective of good separation it would make sense:

  1. MIDI handling gets less messy
  2. JSON-RPC can get feature parity with the GUI
  3. Mobile GUI could be another "GUI Client" which is native to the mobile OS

The biggest downside would be that this may be hard to get through a review.

@softins
Copy link
Member Author

softins commented Nov 11, 2024

Originally posted by @softins in #3419 (comment)

These are all good thoughts. But I think we should go incrementally, rather than for a big bang which might take ages to finish or not even get around to being completed.

So I would think of implementing what I suggested above within CClient and not yet making any changes to the GUI and Audio Mixer. That could then be done separately afterwards. I don't actually think the mixer is within CClient anyway. When running with a GUI, we have CClientDlg above it, and CAudioMixerBoard above that, with signals from below being passed to slots above. It should all work unchanged and won't care that iChannelIdx is now a client-side channel instead of a server-side channel.

I'm aware too that it will be necessary to translate from server channel IDs to client channel IDs within CChanInfo, when passing them up to the mixer.

@softins
Copy link
Member Author

softins commented Nov 11, 2024

Originally posted by @pljones in #3419 (comment)

Yep, not suggesting doing it in one go. But having these goals laid out clearly as incremental planned steps would be useful, I think, so we can assess progress to a better implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants