Feature set cl message#3727
Conversation
| { | ||
| uint32_t iFeatures = 0; | ||
|
|
||
| iFeatures |= ( !bUseDoubleSystemFrameSize << FS_SMALL_NETW_BUF ); |
There was a problem hiding this comment.
Don't conflate "small network buffers" in the client with "double frame size" in the server -- you've looked at this now 😄.
| enum EFeatureSet | ||
| { | ||
| // used for protocol -> enum values must be fixed | ||
| FS_SMALL_NETW_BUF = 0, |
There was a problem hiding this comment.
This should be FS_FAST_UPDATE and be true if bUseDoubleSystemFrameSize is false.
| FS_MULTITHREADING = 1, | ||
| FS_RECORDER_INIT = 2, | ||
| FS_DISABLE_RECORDING = 3, | ||
| FS_IS_RECORDING = 4, |
There was a problem hiding this comment.
I don't think we need three flags:
FS_RECORDER_ENABLED- true ifGetRecorderInitialised() && !GetDisableRecording()FS_IS_RECORDING- true ifJamController.GetRecorderState() == RS_RECORDING(you had!=?)
| FS_DELAY_PAN = 5, | ||
| FS_ENABLE_IPV6 = 6, | ||
| FS_RAW_AUDIO = 7 | ||
| }; |
There was a problem hiding this comment.
Might be worth adding:
FS_DISCONONQUIT-- true if the connection will drop if the server exits
|
Could you please also check which protocol messages could be replaced by this? |
| (standard re-registration timeout). | ||
|
|
||
|
|
||
| - PROTMESSID_CLM_SERVER_INFO: Bitmask of enabled server features |
There was a problem hiding this comment.
I initially phrased it as "capabilities". FYI.
There were old versions which didn't support panning I think. Assuming this message was already implemented at this time, We would have used this message to advertise pan support.
In think we should make it very clear what this message can and cannot encode
There was a problem hiding this comment.
It's a new message being added for this new feature.
There was a problem hiding this comment.
Yes. I know. But we should specify what this feature can and what it can't expose. I'm thinking of a specification/architectural viewpoint
| iFeatures |= ( (JamController.GetRecorderState() != RS_RECORDING) << FS_IS_RECORDING ); | ||
| iFeatures |= ( bDelayPan << FS_DELAY_PAN ); | ||
| iFeatures |= ( bEnableIPv6 << FS_ENABLE_IPV6 ); | ||
| // iFeatures |= ( bDisableRaw << FS_RAW_AUDIO ); |
|
My view is that it's exposing the operating state of the Server. If you're running with the UI or using JSONRPC, some state can change (one day, it'll all be controllable, hopefully). However, it's not for large blocks of data: client list, welcome message, etc. I see it as providing answers to questions a Client user might have about Server capabilities (I also favour using that term) - so on/off states. That's why I think it should be stuff that gets shown on the Connect Dialog (although I'm thinking of a "toggle extra connect info" option in Advanced Settings). |
|
Ok. Can it be used by the client to to enable/disable certain features such as raw audio? |
3 participants
Short description of changes
Adds two new connection-less protocol messages, one to request and one to return server features.
CHANGELOG: Request and receive server features by connection-less message
Context: Fixes an issue?
Fixes 3710
Does this change need documentation? What needs to be documented and how?
Yes. See below. Will also need updates to Client and Server documentation.
Status of this Pull Request
Proof of concept. Needs:
What is missing until this pull request can be merged?
See above.
Checklist