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.

Sign up for GitHub

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

Initial support to negotiate SDPs in Single Peer Connection #1167

Merged
merged 12 commits into from
Mar 20, 2018
Merged

Initial support to negotiate SDPs in Single Peer Connection #1167

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1440682
Add option to publish only in basic example
jcague Mar 15, 2018
cba96e9
Safe call to media sinks from MediaStream
jcague Mar 15, 2018
889773a
Remove unused code
jcague Mar 15, 2018
bb41069
Temporary avoid issues when managing multiple streams in WebRTCConnec…
jcague Mar 15, 2018
06d2e27
Remove warnings when building ErizoAPI
jcague Mar 15, 2018
179ac02
Fix path of singlePC attribute.
jcague Mar 15, 2018
8afa5e7
Initial support to negotiation of Single Peer Connection between clie…
jcague Mar 15, 2018
2eb2b8e
Fix lint and unit tests
jcague Mar 15, 2018
07ea345
Fix Erizo Unit Tests
jcague Mar 15, 2018
bfc713f
Fix some errors in gcc
jcague Mar 15, 2018
576f893
Change method name to maybeCloseConnection
jcague Mar 19, 2018
0f35324
Fix more things for spine
jcague Mar 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions erizo/src/erizo/MediaStream.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,12 @@ DEFINE_LOGGER(MediaStream, "MediaStream");

MediaStream::MediaStream(std::shared_ptr<Worker> worker,
std::shared_ptr<WebRtcConnection> connection,
const std::string& media_stream_id) :
const std::string& media_stream_id,
const std::string& media_stream_label) :
audio_enabled_{false}, video_enabled_{false},
connection_{connection},
stream_id_{media_stream_id},
mslabel_ {media_stream_label},
bundle_{false},
pipeline_{Pipeline::create()},
worker_{worker},
Expand Down Expand Up @@ -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()]);
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 I like the ssrc[label] map.

setAudioSourceSSRC(remote_sdp_->audio_ssrc_map[getLabel()]);

audio_enabled_ = remote_sdp_->hasAudio;
video_enabled_ = remote_sdp_->hasVideo;
Expand Down Expand Up @@ -279,26 +281,26 @@ void MediaStream::read(std::shared_ptr<DataPacket> packet) {
if (bundle_) {
// Check incoming SSRC
// Deliver data
if (isVideoSourceSSRC(recvSSRC)) {
if (isVideoSourceSSRC(recvSSRC) && video_sink_) {
parseIncomingPayloadType(buf, len, VIDEO_PACKET);
video_sink_->deliverVideoData(std::move(packet));
} else if (isAudioSourceSSRC(recvSSRC)) {
} else if (isAudioSourceSSRC(recvSSRC) && audio_sink_) {
parseIncomingPayloadType(buf, len, AUDIO_PACKET);
audio_sink_->deliverAudioData(std::move(packet));
} else {
ELOG_DEBUG("%s read video unknownSSRC: %u, localVideoSSRC: %u, localAudioSSRC: %u",
toLog(), recvSSRC, this->getVideoSourceSSRC(), this->getAudioSourceSSRC());
}
} else {
if (packet->type == AUDIO_PACKET && audio_sink_ != nullptr) {
if (packet->type == AUDIO_PACKET && audio_sink_) {
parseIncomingPayloadType(buf, len, AUDIO_PACKET);
// Firefox does not send SSRC in SDP
if (getAudioSourceSSRC() == 0) {
ELOG_DEBUG("%s discoveredAudioSourceSSRC:%u", toLog(), recvSSRC);
this->setAudioSourceSSRC(recvSSRC);
}
audio_sink_->deliverAudioData(std::move(packet));
} else if (packet->type == VIDEO_PACKET && video_sink_ != nullptr) {
} else if (packet->type == VIDEO_PACKET && video_sink_) {
parseIncomingPayloadType(buf, len, VIDEO_PACKET);
// Firefox does not send SSRC in SDP
if (getVideoSourceSSRC() == 0) {
Expand Down
4 changes: 3 additions & 1 deletion erizo/src/erizo/MediaStream.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class MediaStream: public MediaSink, public MediaSource, public FeedbackSink,
* Constructs an empty MediaStream without any configuration.
*/
MediaStream(std::shared_ptr<Worker> worker, std::shared_ptr<WebRtcConnection> connection,
const std::string& media_stream_id);
const std::string& media_stream_id, const std::string& media_stream_label);
/**
* Destructor.
*/
Expand Down Expand Up @@ -117,6 +117,7 @@ class MediaStream: public MediaSink, public MediaSource, public FeedbackSink,
std::shared_ptr<Worker> getWorker() { return worker_; }

std::string& getId() { return stream_id_; }
std::string& getLabel() { return mslabel_; }

bool isSourceSSRC(uint32_t ssrc);
bool isSinkSSRC(uint32_t ssrc);
Expand All @@ -143,6 +144,7 @@ class MediaStream: public MediaSink, public MediaSource, public FeedbackSink,
private:
std::shared_ptr<WebRtcConnection> connection_;
std::string stream_id_;
std::string mslabel_;
bool should_send_feedback_;
bool slide_show_mode_;
bool sending_;
Expand Down
105 changes: 0 additions & 105 deletions erizo/src/erizo/SdpInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ namespace erizo {
static const char *sendonly = "a=sendonly";
static const char *ice_user = "a=ice-ufrag";
static const char *ice_pass = "a=ice-pwd";
static const char *ssrctag = "a=ssrc:";
static const char *ssrcgrouptag = "a=ssrc-group";
static const char *rid = "a=rid";
static const char *savpf = "SAVPF";
static const char *rtpmap = "a=rtpmap:";
Expand All @@ -59,7 +57,6 @@ namespace erizo {
hasAudio = false;
hasVideo = false;
profile = SAVPF;
audio_ssrc = 0;
videoCodecs = 0;
audioCodecs = 0;
videoSdpMLine = -1;
Expand Down Expand Up @@ -306,16 +303,6 @@ namespace erizo {
}
}
}

if (audio_ssrc == 0) {
audio_ssrc = 44444;
}
if (audioDirection != RECVONLY) {
sdp << "a=ssrc:" << audio_ssrc << " cname:o/i14u9pJrxRKAsu" << endl <<
"a=ssrc:"<< audio_ssrc << " msid:"<< msidtemp << " a0"<< endl <<
"a=ssrc:"<< audio_ssrc << " mslabel:"<< msidtemp << endl <<
"a=ssrc:"<< audio_ssrc << " label:" << msidtemp << "a0" << endl;
}
}

if (printedVideo && this->hasVideo) {
Expand Down Expand Up @@ -426,9 +413,6 @@ namespace erizo {
}
}
}
if (video_ssrc_list.empty()) {
video_ssrc_list.push_back(55543);
}

if (!rids().empty()) {
sdp << "a=simulcast: " << rids()[0].direction << " rid=";
Expand All @@ -440,25 +424,6 @@ namespace erizo {
}
sdp << '\n';
}

if (videoDirection != RECVONLY) {
std::for_each(video_ssrc_list.begin(), video_ssrc_list.end(),
[&sdp, &msidtemp](uint32_t &video_ssrc){
sdp << "a=ssrc:" << video_ssrc << " cname:o/i14u9pJrxRKAsu" << endl <<
"a=ssrc:" << video_ssrc << " msid:"<< msidtemp << " v0"<< endl <<
"a=ssrc:" << video_ssrc << " mslabel:"<< msidtemp << endl <<
"a=ssrc:" << video_ssrc << " label:" << msidtemp << "v0" << endl;
});
/* TODO(pedro) properly encode FID groups in sdp when supported
std::for_each(video_rtx_ssrc_map.begin(), video_rtx_ssrc_map.end(),
[&sdp, &msidtemp](uint32_t &video_rtx_ssrc){
sdp << "a=ssrc:" << video_rtx_ssrc << " cname:o/i14u9pJrxRKAsu" << endl <<
"a=ssrc:" << video_rtx_ssrc << " msid:"<< msidtemp << " v0"<< endl <<
"a=ssrc:" << video_rtx_ssrc << " mslabel:"<< msidtemp << endl <<
"a=ssrc:" << video_rtx_ssrc << " label:" << msidtemp << "v0" << endl;
});
*/
}
}
ELOG_DEBUG("sdp local \n %s", sdp.str().c_str());
return sdp.str();
Expand Down Expand Up @@ -603,8 +568,6 @@ namespace erizo {
size_t isCrypt = line.find(crypto);
size_t isUser = line.find(ice_user);
size_t isPass = line.find(ice_pass);
size_t isSsrc = line.find(ssrctag);
size_t isSsrcGroup = line.find(ssrcgrouptag);
size_t isRid = line.find(rid);
size_t isSAVPF = line.find(savpf);
size_t isRtpmap = line.find(rtpmap);
Expand Down Expand Up @@ -767,55 +730,6 @@ namespace erizo {
}
}

if (isSsrc != std::string::npos) {
Copy link
Contributor

Choose a reason for hiding this comment

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

💯x 👍 To removing the SSRC parsing from here

std::vector<std::string> parts = stringutil::splitOneOf(line, " :", 2);
// FIXME add error checking
if (mtype == VIDEO_TYPE) {
uint32_t parsed_ssrc = strtoul(parts[1].c_str(), nullptr, 10);
ELOG_DEBUG("message: maybeAdd video in isSsrc, ssrc: %u", parsed_ssrc);
maybeAddSsrcToList(parsed_ssrc);
} else if ((mtype == AUDIO_TYPE) && (audio_ssrc == 0)) {
audio_ssrc = strtoul(parts[1].c_str(), nullptr, 10);
ELOG_DEBUG("audio ssrc: %u", audio_ssrc);
}
}

if (isSsrcGroup != std::string::npos) {
if (mtype != VIDEO_TYPE) {
continue;
}
std::vector<std::string> parts = stringutil::splitOneOf(line, " :", 10);
if (parts.size() < 4) {
continue;
}
if (parts[1] == kSimulcastGroup) {
ELOG_DEBUG("Detected SIM group, size: %lu", parts.size());
std::vector<uint32_t> old_video_ssrc_list;
if (video_ssrc_list.size() > 0) {
old_video_ssrc_list = video_ssrc_list;
video_ssrc_list.clear();
}
std::for_each(parts.begin() + 2, parts.end(), [this] (std::string &part){
uint32_t parsed_ssrc = strtoul(part.c_str(), nullptr, 10);
ELOG_DEBUG("maybeAddSsrc video SIM, ssrc %u", parsed_ssrc);
maybeAddSsrcToList(parsed_ssrc);
});
for (uint32_t ssrc : old_video_ssrc_list) {
maybeAddSsrcToList(ssrc);
}
} else if (parts[1] == kFidGroup) {
int number_of_ssrcs = parts.size() - 2;
if (number_of_ssrcs != 2) {
ELOG_DEBUG("FID Group with wrong number of SSRCs, ignoring");
continue;
}
uint32_t original_ssrc = strtoul(parts[2].c_str(), nullptr, 10);
uint32_t rtx_ssrc = strtoul(parts[3].c_str(), nullptr, 10);
video_rtx_ssrc_map[rtx_ssrc] = original_ssrc;
ELOG_DEBUG("message: parsed FID group, original_src: %u, rtx_ssrc: %u", original_ssrc, rtx_ssrc);
}
}

if (isRid != std::string::npos) {
std::vector<std::string> parts = stringutil::splitOneOf(line, ":", 2);
if (mtype == VIDEO_TYPE) {
Expand Down Expand Up @@ -973,10 +887,6 @@ namespace erizo {
}
}

if (video_ssrc_list.empty()) {
video_ssrc_list.push_back(0);
}

// go through the payload_map_ and match it with internalPayloadVector_
// generate rtpMaps and payloadVector
std::vector<RtpMap> rtx_maps;
Expand Down Expand Up @@ -1214,21 +1124,6 @@ namespace erizo {
s[len] = 0;
}

void SdpInfo::maybeAddSsrcToList(uint32_t ssrc) {
auto find_rt = video_rtx_ssrc_map.find(ssrc);
if (find_rt != video_rtx_ssrc_map.end()) {
// Its a rtx ssrc
return;
}
auto value = std::find_if(video_ssrc_list.begin(), video_ssrc_list.end(), [ssrc](uint32_t current_ssrc) {
return ssrc == current_ssrc;
});
if (value == video_ssrc_list.end()) {
ELOG_DEBUG("message: Adding ssrc to list, ssrc: %u", ssrc);
video_ssrc_list.push_back(ssrc);
}
}

bool operator==(const Rid& lhs, const Rid& rhs) {
return lhs.id == rhs.id && lhs.direction == rhs.direction;
}
Expand Down
6 changes: 3 additions & 3 deletions erizo/src/erizo/SdpInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,9 +257,9 @@ class SdpInfo {
/**
* The audio and video SSRCs for this particular SDP.
*/
unsigned int audio_ssrc;
std::vector<uint32_t> video_ssrc_list;
std::map<uint32_t, uint32_t> video_rtx_ssrc_map;
std::map<std::string, unsigned int> audio_ssrc_map;
std::map<std::string, std::vector<uint32_t>> video_ssrc_map;
std::map<std::string, std::map<uint32_t, uint32_t>> video_rtx_ssrc_map;
/**
* Is it Bundle
*/
Expand Down
46 changes: 38 additions & 8 deletions erizo/src/erizo/WebRtcConnection.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,14 @@ bool WebRtcConnection::createOffer(bool video_enabled, bool audioEnabled, bool b

if (video_enabled_) {
forEachMediaStream([this] (const std::shared_ptr<MediaStream> &media_stream) {
local_sdp_->video_ssrc_list.push_back(media_stream->getVideoSinkSSRC());
std::vector<uint32_t> video_ssrc_list = std::vector<uint32_t>();
video_ssrc_list.push_back(media_stream->getVideoSinkSSRC());
local_sdp_->video_ssrc_map[media_stream->getLabel()] = video_ssrc_list;
});
}
if (audio_enabled_) {
forEachMediaStream([this] (const std::shared_ptr<MediaStream> &media_stream) {
local_sdp_->audio_ssrc = media_stream->getAudioSinkSSRC();
local_sdp_->audio_ssrc_map[media_stream->getLabel()] = media_stream->getAudioSinkSSRC();
});
}

Expand Down Expand Up @@ -159,10 +161,15 @@ void WebRtcConnection::addMediaStream(std::shared_ptr<MediaStream> media_stream)
}

void WebRtcConnection::removeMediaStream(const std::string& stream_id) {
ELOG_DEBUG("%s message: removing mediaStream, id: %s", toLog(), stream_id.c_str())
ELOG_DEBUG("%s message: removing mediaStream, id: %s", toLog(), stream_id.c_str());
media_streams_.erase(std::remove_if(media_streams_.begin(), media_streams_.end(),
[stream_id](const std::shared_ptr<MediaStream> &stream) {
return stream->getId() == stream_id;
[stream_id, this](const std::shared_ptr<MediaStream> &stream) {
bool isStream = stream->getId() == stream_id;
if (isStream) {
local_sdp_->video_ssrc_map.erase(local_sdp_->video_ssrc_map.find(stream->getLabel()));
local_sdp_->audio_ssrc_map.erase(local_sdp_->audio_ssrc_map.find(stream->getLabel()));
}
return isStream;
}), media_streams_.end());
}

Expand All @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agree, I'll make sure we don't need this by running some test

std::vector<uint32_t> video_ssrc_list = std::vector<uint32_t>();
video_ssrc_list.push_back(media_stream->getVideoSinkSSRC());
local_sdp_->video_ssrc_map[media_stream->getLabel()] = video_ssrc_list;
local_sdp_->audio_ssrc_map[media_stream->getLabel()] = media_stream->getAudioSinkSSRC();
});
if (local_sdp_->audio_ssrc_map.size() + local_sdp_->video_ssrc_map.size() > 2) {
if (local_sdp_->audioDirection == erizo::RECVONLY) {
local_sdp_->audioDirection = erizo::SENDRECV;
local_sdp_->videoDirection = erizo::SENDRECV;
}
}
return local_sdp_;
}

Expand All @@ -203,6 +222,10 @@ bool WebRtcConnection::processRemoteSdp() {
forEachMediaStream([this] (const std::shared_ptr<MediaStream> &media_stream) {
media_stream->setRemoteSdp(remote_sdp_);
});
std::string object = this->getLocalSdp();
if (conn_event_listener_) {
conn_event_listener_->notifyEvent(CONN_SDP, object);
}
return true;
}

Expand All @@ -212,8 +235,10 @@ bool WebRtcConnection::processRemoteSdp() {
local_sdp_->updateSupportedExtensionMap(extension_processor_.getSupportedExtensionMap());

forEachMediaStream([this] (const std::shared_ptr<MediaStream> &media_stream) {
local_sdp_->video_ssrc_list.push_back(media_stream->getVideoSinkSSRC());
local_sdp_->audio_ssrc = media_stream->getAudioSinkSSRC();
std::vector<uint32_t> video_ssrc_list = std::vector<uint32_t>();
video_ssrc_list.push_back(media_stream->getVideoSinkSSRC());
local_sdp_->video_ssrc_map[media_stream->getLabel()] = video_ssrc_list;
local_sdp_->audio_ssrc_map[media_stream->getLabel()] = media_stream->getAudioSinkSSRC();
});

if (remote_sdp_->dtlsRole == ACTPASS) {
Expand Down Expand Up @@ -405,7 +430,12 @@ void WebRtcConnection::onTransportData(std::shared_ptr<DataPacket> packet, Trans
if (chead->isRtcp() && chead->packettype != RTCP_Sender_PT) { // Sender Report
ssrc = chead->getSourceSSRC();
}
forEachMediaStream([packet, transport, ssrc] (const std::shared_ptr<MediaStream> &media_stream) {
int index = 0;
forEachMediaStream([&index, packet, transport, ssrc] (const std::shared_ptr<MediaStream> &media_stream) {
if (index == 1) {
return;
}
index++;
if (media_stream->isSourceSSRC(ssrc) || media_stream->isSinkSSRC(ssrc)) {
media_stream->onTransportData(packet, transport);
}
Expand Down
2 changes: 1 addition & 1 deletion erizo/src/erizo/rtp/RtcpNackGenerator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ bool RtcpNackGenerator::handleRtpPacket(std::shared_ptr<DataPacket> packet) {
if (!initialized_) {
highest_seq_num_ = seq_num;
initialized_ = true;
return 0;
return false;
}
if (seq_num == highest_seq_num_) {
return false;
Expand Down
Loading