Improve client list synchronization or stop audio access during connect

[hoffie]

Describe the bug

Client list updates are only sent when a ChanInfo protocol message has been received.
When some other event triggers an update (another client connects/disconnects/renames), state will be synchronized again.

To Reproduce

channels.h:107

Expected behavior

The list of participating (send/receive audio) clients should be transparent to all clients in all circumstances.

Version of Jamulus

3.7.0rc1

Additional context

I see two approaches for fixing this:

1. Make currently connecting clients transparent. This may lead to clients without a name showing up for some very short time (ideally). This might make it infeasable to continue running @softins's welcome message feature in Jamulus Explorer. :(
2. Do not accept audio from or send audio to unnamed clients.

@softins @pljones Thoughts?

[gilgongo]

If this is has got a release tag, shouldn't it be on the backlog?

[softins]

I see two approaches for fixing this:

1. Make currently connecting clients transparent. This may lead to clients without a name showing up for some very short time (ideally). This might make it infeasable to continue running @softins's welcome message feature in Jamulus Explorer. :(
   2, Do not accept audio from or send audio to unnamed clients.

I would favour approach 2.

Obviously, with the current protocol, the audio must be accepted from the client, and causes the client session to be created, but until the client is identified, the audio should be discarded instead of mixed. And no audio should be returned to the client while in that state.

"Unnamed" clients needs to distinguish between a client that hasn't yet sent a clientinfo message, and a client that has sent a clientinfo message with no name specified. The former should not be connected to audio, but the latter should, and should be displayed in all the clients, as at present.

And a client in the former state should not show up on anyone's client list. I think it was stated that currently, it would, if an additional client connected and identified.

[gene96817]

Does "unnamed" client also include clients that have not set their profile?

[pljones]

I think 2 can be ruled out totally.

There's no requirement to set a client name or even send a protocol message with an empty name. (There are defaults which would apply.)

That must not stop use of the server. (At least, not without changing the protocol version to 4.x and declaring it incompatible with anything older.)

[hoffie]

I think 2 can be ruled out totally.

There's no requirement to set a client name or even send a protocol message with an empty name. (There are defaults which would apply.)

That must not stop use of the server. (At least, not without changing the protocol version to 4.x and declaring it incompatible with anything older.)

I don't think there is a formal specification of the Jamulus protocol, is there? There is inline documentation of the individual messages in src/protocol.cpp, but there is no description how those messages work together. The protocol is implicitly defined by the reference implementation, which is Jamulus itself.

I don't think there is even an explicit protocol version but rather explicit checks for specific features or the partner's Jamulus version, right?

If this is the case, I'd argue that we could indeed adjust this part of the protocol without mandating Jamulus 4.x. Previous Jamulus versions will continue to work.
The only risk I see is that it could break alternative Jamulus clients which process audio, but behave different to official Jamulus releases by not sending a CHANNEL_INFOS message.
Are there such alternative (full) clients?
I'm not sure if this is a realistical assumption and if this should be the deciding factor for us.

[pljones]

The point is that it would break existing clients. Not clients not based on the official code. Clients of older versions of the official code that are currently compatible. So official clients. Officially supported clients. Taking away that support is a breaking change.

[hoffie]

Uh, yes, I agree.

I was thinking about tieing this to CHANNEL_INFOS, which was introduced in 0f976c4. It seems like Jamulus 3.3.0 was the first version which included this.
This means, what I proposed would break official Jamulus before version 3.3.0.

I just ran a quick test independent of this issue: I tried connecting a 3.3.0 client to a 3.7.0dev server. This worked, but no faders were shown, so I'd say that this combination does not work properly, it seems?

1. Is CHANNEL_INFOS what you meant regarding compatibility?
2. What are the compatibility guarantees, e.g. what is the oldest supported client version? I assumed it must be 3.0.0, but it seems like even 3.3.0 is no longer properly supported?

[hoffie]

6466a4a looks like it removed CHANNEL_NAME support in favor of CHANNEL_INFOS. This was part of 3.4.5, so I guess 3.4.4 was the last version which was compatible with 3.3.0 in this regard?

[pljones]

There are still 3.4.4 servers, unfortunately... (Later clients may have trouble connecting to them...) Version numbering hasn't been consistent, it has to be said :).

We've said now that the versioning scheme is that 3.x.x remains compatible with all 3.x.x versions and we go to 4.x.x if we break compatibility. That's not been the case in the past - mostly work has been put in to retain compatibility as much as possible by introducing new protocol messages (which get silently ignored by older versions and aren't mandatory in newer versions).

A couple of changes have been made (one being whichever version introduced small network buffers - if that's turned on, you can't connect to a server that doesn't support it; I don't like it, it didn't need to fail that way) that have caused "partly breaking" changes. The faders one is another. It "sort of" still works... And there are old servers still out there that can register with a directory server, so think they're still available public servers - so we can't say they're not supported until we drop support for everything older than 3.6.1 (which is the first version that tells the directory server its version).

That's why I'm concerned...

[hoffie]

@gene96817

Does "unnamed" client also include clients that have not set their profile?

No, it's about clients which do not send channel information at all.

@pljones

There are still 3.4.4 servers, unfortunately... (Later clients may have trouble connecting to them...) Version numbering hasn't been consistent, it has to be said :).

What matters more would be 3.4.4 clients as both suggested changes would affect the server. In other words: When someone runs a 3.4.4 server, they will not be affected by the bugfix. If they do update, they can profit from the bugfix.

I have looked at the code regarding the amount of changes. Option 1 seems way simpler than Option 2, which would require changes in audio mixing, audio sending and channel state tracking. It would also have to touch performance-critical paths. It's certainly possible, but seems heavier both implementation- and performance-wise.