Initial support to negotiate SDPs in Single Peer Connection #1167
Conversation
| @@ -95,6 +96,9 @@ window.onload = function () { | |||
| room = Erizo.Room({token: token}); | |||
|
|
|||
| var subscribeToStreams = function (streams) { | |||
| if (onlyPublish) { | |||
There was a problem hiding this comment.
👍 This is useful. I did exactly the same for the SinglePC prototype and never actually added it to master.
| @@ -767,55 +730,6 @@ namespace erizo { | |||
| } | |||
| } | |||
|
|
|||
| if (isSsrc != std::string::npos) { | |||
There was a problem hiding this comment.
💯x 👍 To removing the SSRC parsing from here
| connection.close(); | ||
| if (this.ErizoConnectionsMap.get(connection.erizoId) !== undefined) { | ||
| delete this.ErizoConnectionsMap.get(connection.erizoId)[connection.sessionId]; | ||
| if (connection.streamsMap.size() === 0) { |
There was a problem hiding this comment.
I think this is a good place to check if the connection really has to be closed 👍. Consider changing the method name to maybeCloseConnection and update the log accordingly.
| @@ -127,8 +129,8 @@ bool MediaStream::setRemoteSdp(std::shared_ptr<SdpInfo> sdp) { | |||
|
|
|||
| bundle_ = remote_sdp_->isBundle; | |||
|
|
|||
| setVideoSourceSSRCList(remote_sdp_->video_ssrc_list); | |||
| setAudioSourceSSRC(remote_sdp_->audio_ssrc); | |||
| setVideoSourceSSRCList(remote_sdp_->video_ssrc_map[getLabel()]); | |||
There was a problem hiding this comment.
👍 I like the ssrc[label] map.
| @@ -183,6 +190,18 @@ bool WebRtcConnection::setRemoteSdpInfo(std::shared_ptr<SdpInfo> sdp) { | |||
|
|
|||
| std::shared_ptr<SdpInfo> WebRtcConnection::getLocalSdpInfo() { | |||
| ELOG_DEBUG("%s message: getting local SDPInfo", toLog()); | |||
| forEachMediaStream([this] (const std::shared_ptr<MediaStream> &media_stream) { | |||
There was a problem hiding this comment.
I don't get why this is added here... Aren't we adding the SinkSSRCs to the local_sdp in both createOffer and processRemoteSdp. So in both cases of the negotiation we should have the local_sdp correctly filled in.
There was a problem hiding this comment.
agree, I'll make sure we don't need this by running some test
Description
More work towards having Single Peer Connection in Lynckia. With this PR we start to have negotiation working in some cases with Chrome. I had to touch multiple files and how the negotiation is performed, so I don't want to add and fix more things to Single Peer Connection in this PR and test that it doesn't break anything.
[] It needs and includes Unit Tests
Changes in Client or Server public APIs
Nothing changes in the public APIs.
I added a thing to be considered by Native Clients: a new attribute called
labelthat is needed to identify the stream in the web clients.[] It includes documentation for these changes in
/doc.