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

Convert call.js to Typescript & update WebRTC APIs #1487

Merged
merged 11 commits into from
Oct 6, 2020
Merged

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Sep 30, 2020

Two large commits here: a78b590 which converts to typescript and cc4656b which updates to changes in the WebRTC API (mostly using promises instead of callbacks and not using addStream().

This PR also should make the output setting a bit more reliable, as this appears to be quite sensitive on Chrome (it's still not implemented on Firefox).

This does a fairly basic rewriting of the whole file to bring it up
to a vaguely modern standard of code. Nothing should have changed
fundamentally, but it probably should do: as per the global type
overrides, we use a lot of deprecated WebRTC APIs and should probably
update.
Various flags that may or may not have been part of the poorly defined
external API before. Neither were used internally. Make direction a
public export but remove didConnect.
and some other related & semi-related refactoring
Otherwise we end up with separate, unbundled 'streamless tracks'
which is not what we want.
As per comment, this seems to be the way to make setSinkId work reliably on Chrome.

Also don't make our own mediastreams if we don't need to (ie. in a normal call
our tracks should come with their owen MediaStreams).
@dbkr dbkr changed the title Convert call.js to Typescript Convert call.js to Typescript & update WebRTC APIs Oct 5, 2020
@dbkr dbkr requested a review from a team October 5, 2020 15:11
@dbkr dbkr marked this pull request as ready for review October 5, 2020 15:11
src/webrtc/call.ts Outdated Show resolved Hide resolved
src/webrtc/call.ts Outdated Show resolved Hide resolved
Copy link
Member

@t3chguy t3chguy left a comment

Choose a reason for hiding this comment

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

Looks sane otherwise

src/webrtc/call.ts Show resolved Hide resolved
src/webrtc/call.ts Outdated Show resolved Hide resolved
src/webrtc/call.ts Outdated Show resolved Hide resolved
src/@types/global.d.ts Outdated Show resolved Hide resolved
@dbkr dbkr requested a review from t3chguy October 6, 2020 10:31
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 this pull request may close these issues.

2 participants