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

Remove video tracks on video mute without renegotiating #3091

Merged
merged 8 commits into from
Jan 25, 2023

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Jan 24, 2023

Builds on #3028 but without removing the sender and causing a renegotiation.

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

Here's what your changelog entry will look like:

✨ Features

  • Remove video tracks on video mute without renegotiating (#3091).

src/webrtc/call.ts Show resolved Hide resolved
Copy link
Contributor

@SimonBrandner SimonBrandner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first, I had trouble understanding what this is exactly trying to do but I think I am starting to see now but a closer PR description could be helpful here

src/webrtc/call.ts Outdated Show resolved Hide resolved
src/webrtc/call.ts Outdated Show resolved Hide resolved
src/webrtc/call.ts Outdated Show resolved Hide resolved
return this.hasLocalUserMediaVideoTrack || this.hasRemoteUserMediaVideoTrack ? CallType.Video : CallType.Voice;
// we may want to look for a video receiver here rather than a track to match the
// sender behaviour, although in practice they should be the same thing
return this.hasUserMediaVideoSender || this.hasRemoteUserMediaVideoTrack ? CallType.Video : CallType.Voice;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sender will be there even if the track isn't, so this could return CallType.Video even if we have no video in the call, afaict

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd argue this is correct - a video call is still a video call if everyone has their video muted, although really the lines of what is a voice and video call are very blurry.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd argue that the call type can change during the call by you muting/unmuting but yeah the lines are blurry and I am in favour of getting rid of 1:1 call types as soon as we can

src/webrtc/call.ts Outdated Show resolved Hide resolved
@dbkr dbkr merged commit cb61345 into develop Jan 25, 2023
@dbkr dbkr deleted the dbkr/video_mute_no_renegotiate branch January 25, 2023 15:38
su-ex added a commit to SchildiChat/matrix-js-sdk that referenced this pull request Feb 28, 2023
* Element-R: implement encryption of outgoing events ([\matrix-org#3122](matrix-org#3122)).
* Poll model - page /relations results ([\matrix-org#3073](matrix-org#3073)). Contributed by @kerryarchibald.
* Poll model - validate end events ([\matrix-org#3072](matrix-org#3072)). Contributed by @kerryarchibald.
* Handle optional last_known_event_id property in m.predecessor ([\matrix-org#3119](matrix-org#3119)). Contributed by @andybalaam.
* Add support for stable identifier for fixed MAC in SAS verification ([\matrix-org#3101](matrix-org#3101)).
* Provide eventId as well as roomId from Room.findPredecessor ([\matrix-org#3095](matrix-org#3095)). Contributed by @andybalaam.
* MSC3946 Dynamic room predecessors ([\matrix-org#3042](matrix-org#3042)). Contributed by @andybalaam.
* Poll model ([\matrix-org#3036](matrix-org#3036)). Contributed by @kerryarchibald.
* Remove video tracks on video mute without renegotiating ([\matrix-org#3091](matrix-org#3091)).
* Introduces a backwards-compatible API change. `MegolmEncrypter#prepareToEncrypt`'s return type has changed from `void` to `() => void`. ([\matrix-org#3035](matrix-org#3035)). Contributed by @clarkf.
* Stop the ICE disconnected timer on call terminate ([\matrix-org#3147](matrix-org#3147)).
* Clear notifications when we can infer read status from receipts ([\matrix-org#3139](matrix-org#3139)). Fixes element-hq/element-web#23991.
* Messages sent out of order after one message fails ([\matrix-org#3131](matrix-org#3131)). Fixes element-hq/element-web#22885 and element-hq/element-web#18942. Contributed by @justjanne.
* Element-R: fix a bug which prevented encryption working after a reload ([\matrix-org#3126](matrix-org#3126)).
* Element-R: Fix invite processing ([\matrix-org#3121](matrix-org#3121)).
* Don't throw with no `opponentDeviceInfo` ([\matrix-org#3107](matrix-org#3107)).
* Remove flaky megolm test ([\matrix-org#3098](matrix-org#3098)). Contributed by @clarkf.
* Fix "verifyLinks" functionality of getRoomUpgradeHistory ([\matrix-org#3089](matrix-org#3089)). Contributed by @andybalaam.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants