Group Calls (WIP) #1902
Closed
Group Calls (WIP) #1902
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
robertlong
commented
Sep 9, 2021
src/webrtc/groupCall.ts
Outdated
| } | ||
|
|
||
| const CONF_ROOM = "me.robertlong.conf"; | ||
| const CONF_PARTICIPANT = "me.robertlong.conf.participant"; |
There was a problem hiding this comment.
TODO: I need to move this to the proper MSC event once we've created it.
src/webrtc/groupCall.ts
Outdated
| */ | ||
|
|
||
| // TODO: move this elsewhere or get rid of the retry logic. Do we need it? | ||
| sendStateEventWithRetry( |
There was a problem hiding this comment.
I don't actually know that we still want this for this PR. I don't think it was actually making calls any more stable, but I don't know for sure.
robertlong
commented
Sep 9, 2021
There was a problem hiding this comment.
Generally, this looking absolutely awesome! I'll do a more thorough review when this isn't a draft but I've got a few more general points:
- It might be beneficial for
localParticipantto be a getter the same waylocalUserMediaStreamand similar are -
GroupCallshould emitfeeds_changedevents and have agetFeeds()method which is more friendly for makingCallViewcomponents. It might even allow us to make the already existingCallViewin thereact-sdkwork without many changes - The TypeScript could be improved a bit (params and return types)
-
MediaHandlerfrom ImproveMatrixCallmedia handling and internally removeCallTypes #1911 should be somehow used when it gets merged
| @@ -630,6 +630,11 @@ export class MatrixCall extends EventEmitter { | |||
| * @param {boolean} addToPeerConnection whether to add the tracks to the peer connection | |||
| */ | |||
| public pushLocalFeed(callFeed: CallFeed, addToPeerConnection = true): void { | |||
| if (this.feeds.some((feed) => callFeed.stream.id === feed.stream.id)) { | |||
There was a problem hiding this comment.
Does the same stream id always mean the same feed?
No particular reason to worry about old user agents here
Seems chrome at least will give you a disabled audio track if you already had another user media audio track and disabled it, so make sure our tracks are enabled when we add them. We already did this on one code path but it didn't get moved over when a new code path was added. On the plus side, we now know the reason for the ancient code that had the comment asking what it was for, so update that.
If a renogotiation ends up with one track being removed, we removed the whole stream, which would cause us to lose, for example, audio rather than just video.
* Add PTT call mode & mute by default in PTT calls (#2311) No other parts of PTT calls implemented yet * Make the tests pass again (#2316) 3280394 made call use a bunch of methods that weren't mocked in the tests. * Add maximum trasmit time for PTT (#2312) on sender side by muting mic after the max transmit time has elapsed. * Don't allow user to unmute if another user is speaking (#2313) * Add maximum trasmit time for PTT on sender side by muting mic after the max transmit time has elapsed. * Don't allow user to unmute if another user is speaking Based on #2312 For element-hq/element-call#298 * Fix createGroupCall arguments (#2325) Comma instead of a colon...
Closed
|
Closing in favour of #2355 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Group Calls
This PR expands upon Matrix calls and adds support for establishing calls to multiple members in a room. The code stems from my work on matrix-video-chat.
New Classes
GroupCallCreated by calling
client.createGroupCall(roomId, type)where type can be either"voice"or"video". This class essentially sets up a bunch ofMatrixCalls to room members who have set a participant state event.GroupCallParticipantThis is a wrapper around
MatrixCallthat represents the state for a participant who has entered aGroupCall.New Features
Addition of the
inviteefield tom.call.inviteThis
inviteefield allows the recipients of them.call.inviteevent to filter out invites not intended for them.Ice Disconnected Timeout
Rather than wait on the Ice
failedconnection state, I added a 30 second timeout which ensures that calls hang up if you've been disconnected for too long. If the connection state changes within this 30 second period, this timeout resets.Active Speaker
@SimonBrandner recently added speaking and volume events to
CallFeeds, this PR uses that work to track the "active speaker" in a group call. This is primarily intended to be used to show what user should be focused in a group call.WebRTC Datachannels
Adding datachannels to calls is currently possible by calling
call.peerConn.createDataChannel(). This PR just formalizes that process by adding a public API method for doing so and adatachannelevent for listening for new datachannels on aMatrixCallorGroupCall.To Do:
getLocalAudioStream/getLocalVideoStreamCallFeedexportinviteefield.This PR currently has no changelog labels, so will not be included in changelogs.
A reviewer can add one of:
T-Deprecation,T-Enhancement,T-Defect,T-Taskto indicate what type of change this is, or addType: [enhancement/defect/task]to the description and I'll add them for you.