-
Notifications
You must be signed in to change notification settings - Fork 167
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
Add fallback for addTrack
if addTransceiver
is not supported by a device
#403
Conversation
🦋 Changeset detectedLatest commit: 97b14d1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great cleanup!
src/room/RTCEngine.ts
Outdated
return this.createTransceiverRTCRtpSender(track, opts, encodings); | ||
} | ||
if (supportsAddTrack()) { | ||
console.log('using add-track fallback'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
switch to our logger?
@@ -577,7 +567,7 @@ export default class LocalParticipant extends Participant { | |||
this.engine.negotiate(); | |||
|
|||
// store RTPSender | |||
track.sender = transceiver.sender; | |||
track.sender = await this.engine.createSender(track, opts, encodings); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will need to happen before this.engine.negotiate()
and setTrackCodecBitrate
. tho today this works because of the debounce logic that we have in place.
the work in this PR is largely based upon the work in #373 (thank you @cscherban !)
main difference implementation wise is that there's no support for
addStream
.apart from that it's mostly refactoring.
To increase compatibility further, we will have to add some more specific browser targets to the babel transpilation (will open a separate PR for that).
note: using
addTrack
currently breaks dynacast as this check is failing https://github.com/livekit/client-sdk-js/blob/main/src/room/track/LocalVideoTrack.ts#L322