-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Use adapter.js #940
Use adapter.js #940
Conversation
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 PR is really interesting!! I still need to play a little bit with it, but here I left a minor comments.
We should remove these lines in Connection.js:
} else if (that.browser === 'bowser') {
L.Logger.debug('Bowser Stack');
that = Erizo.BowserStack(spec);
@@ -167,7 +172,7 @@ Erizo.Stream = (specInput) => { | |||
const options = optionsInput || {}; | |||
that.elementID = elementID; | |||
let player; | |||
if (that.hasVideo() || this.hasScreen()) { | |||
if (that.hasVideo() || that.hasScreen()) { |
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.
good catch!
|
||
that.pcConfig = { | ||
iceServers: [], |
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.
nice! 👍
} | ||
result += 'a=x-google-flag:conference\r\n'; | ||
return sdp.replace(matchGroup[0], result); | ||
}; | ||
|
||
const setMaxBW = (sdpInput) => { |
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.
bye bye!
}; | ||
|
||
that.processSignalingMessage = (msgInput) => { | ||
// L.Logger.info("Process Signaling Message", msg); |
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.
Consider removing commented code.
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.
👍
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.
LGTM!
Description
This PR updates most of the webrtc API calls and adds adapter.js to simplify supporting different browser.
baseStack
that is inherited by bothChromeStableStack
andFirefoxStack
and takes care of most of the functionality. Only small differences are now implemented in each browsers' stack.[] It needs and includes Unit Tests
Changes in Client or Server public APIs
No changes to the API
[] It includes documentation for these changes in
/doc
.