From 92bace1fafed21e9db19c7ce2daa6575d0ca6bcc Mon Sep 17 00:00:00 2001 From: "mallinath@webrtc.org" Date: Sat, 27 Aug 2011 00:37:58 +0000 Subject: [PATCH] Hi, This CL will support negotiation of RTCP Mux feature. Earlier we were by default enabling and assuming remote end point will support this feature as well. This will also remove the maintaining of transport channels in WebRtcSession. Its left to cricket::Transport Review URL: http://webrtc-codereview.appspot.com/131005 git-svn-id: http://webrtc.googlecode.com/svn/trunk@472 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../source/talk/app/webrtc/webrtc_json.cc | 34 ++++-- .../source/talk/app/webrtc/webrtc_json.h | 2 + .../source/talk/app/webrtc/webrtcsession.cc | 103 ++---------------- .../source/talk/app/webrtc/webrtcsession.h | 6 - 4 files changed, 34 insertions(+), 111 deletions(-) diff --git a/third_party_mods/libjingle/source/talk/app/webrtc/webrtc_json.cc b/third_party_mods/libjingle/source/talk/app/webrtc/webrtc_json.cc index 26f6a707cd..72dcc56d3f 100644 --- a/third_party_mods/libjingle/source/talk/app/webrtc/webrtc_json.cc +++ b/third_party_mods/libjingle/source/talk/app/webrtc/webrtc_json.cc @@ -145,6 +145,13 @@ bool BuildMediaMessage( Append(params, "label", 1); // always audio 1 } + const cricket::MediaContentDescription* media_info = + static_cast ( + content_info.description); + if (media_info->rtcp_mux()) { + Append(params, "rtcp_mux", std::string("supported")); + } + std::vector rtpmap; if (!BuildRtpMapParams(content_info, video, &rtpmap)) { return false; @@ -214,8 +221,10 @@ bool BuildAttributes(const std::vector& candidates, std::vector::const_iterator iter_end = candidates.end(); for (; iter != iter_end; ++iter) { - if ((video && !iter->name().compare("video_rtp")) || - (!video && !iter->name().compare("rtp"))) { + if ((video && (!iter->name().compare("video_rtcp") || + (!iter->name().compare("video_rtp")))) || + (!video && (!iter->name().compare("rtp") || + (!iter->name().compare("rtcp"))))) { Json::Value candidate; Append(&candidate, "component", kIceComponent); Append(&candidate, "foundation", kIceFoundation); @@ -270,10 +279,8 @@ bool ParseJSONSignalingMessage(const std::string& signaling_message, cricket::AudioContentDescription* audio_content = new cricket::AudioContentDescription(); ParseAudioCodec(mlines[i], audio_content); - - // enabling RTCP mux by default at both ends, without - // exchanging it through signaling message. - audio_content->set_rtcp_mux(true); + + audio_content->set_rtcp_mux(ParseRTCPMux(mlines[i])); audio_content->SortCodecs(); sdp->AddContent(cricket::CN_AUDIO, cricket::NS_JINGLE_RTP, audio_content); ParseICECandidates(mlines[i], candidates); @@ -282,9 +289,7 @@ bool ParseJSONSignalingMessage(const std::string& signaling_message, new cricket::VideoContentDescription(); ParseVideoCodec(mlines[i], video_content); - // enabling RTCP mux by default at both ends, without - // exchanging it through signaling message. - video_content->set_rtcp_mux(true); + video_content->set_rtcp_mux(ParseRTCPMux(mlines[i])); video_content->SortCodecs(); sdp->AddContent(cricket::CN_VIDEO, cricket::NS_JINGLE_RTP, video_content); ParseICECandidates(mlines[i], candidates); @@ -293,6 +298,16 @@ bool ParseJSONSignalingMessage(const std::string& signaling_message, return true; } +bool ParseRTCPMux(const Json::Value& value) { + Json::Value rtcp_mux(ReadValue(value, "rtcp_mux")); + if (!rtcp_mux.empty()) { + if (rtcp_mux.asString().compare("supported") == 0) { + return true; + } + } + return false; +} + bool ParseAudioCodec(const Json::Value& value, cricket::AudioContentDescription* content) { std::vector rtpmap(ReadValues(value, "rtpmap")); @@ -421,6 +436,7 @@ void Append(Json::Value* object, const std::string& key, bool value) { void Append(Json::Value* object, const std::string& key, char * value) { (*object)[key] = Json::Value(value); } + void Append(Json::Value* object, const std::string& key, double value) { (*object)[key] = Json::Value(value); } diff --git a/third_party_mods/libjingle/source/talk/app/webrtc/webrtc_json.h b/third_party_mods/libjingle/source/talk/app/webrtc/webrtc_json.h index a9bb95c45a..5f0920b1c2 100644 --- a/third_party_mods/libjingle/source/talk/app/webrtc/webrtc_json.h +++ b/third_party_mods/libjingle/source/talk/app/webrtc/webrtc_json.h @@ -29,6 +29,7 @@ #define TALK_APP_WEBRTC_WEBRTC_JSON_H_ #include +#include #ifdef WEBRTC_RELATIVE_PATH #include "json/json.h" @@ -100,6 +101,7 @@ bool ParseVideoCodec(const Json::Value& value, cricket::VideoContentDescription* content); bool ParseICECandidates(const Json::Value& value, std::vector* candidates); +bool ParseRTCPMux(const Json::Value& value); Json::Value ReadValue(const Json::Value& value, const std::string& key); std::string ReadString(const Json::Value& value, const std::string& key); double ReadDouble(const Json::Value& value, const std::string& key); diff --git a/third_party_mods/libjingle/source/talk/app/webrtc/webrtcsession.cc b/third_party_mods/libjingle/source/talk/app/webrtc/webrtcsession.cc index 0a9249359c..15fa07aa96 100644 --- a/third_party_mods/libjingle/source/talk/app/webrtc/webrtcsession.cc +++ b/third_party_mods/libjingle/source/talk/app/webrtc/webrtcsession.cc @@ -128,7 +128,7 @@ bool WebRtcSession::CreateVoiceChannel(const std::string& stream_id) { // RTCP disabled cricket::VoiceChannel* voice_channel = - channel_manager_->CreateVoiceChannel(this, stream_id, false); + channel_manager_->CreateVoiceChannel(this, stream_id, true); ASSERT(voice_channel != NULL); stream_info->channel = voice_channel; @@ -145,7 +145,7 @@ bool WebRtcSession::CreateVideoChannel(const std::string& stream_id) { // RTCP disabled cricket::VideoChannel* video_channel = - channel_manager_->CreateVideoChannel(this, stream_id, false, NULL); + channel_manager_->CreateVideoChannel(this, stream_id, true, NULL); ASSERT(video_channel != NULL); stream_info->channel = video_channel; @@ -170,17 +170,6 @@ cricket::TransportChannel* WebRtcSession::CreateChannel( cricket::TransportChannel* transport_channel = transport_->CreateChannel(name, type); ASSERT(transport_channel != NULL); - transport_channels_[name] = transport_channel; - - StreamMap::iterator iter; - for (iter = streams_.begin(); iter != streams_.end(); ++iter) { - StreamInfo* stream_info = (*iter); - if (stream_info->stream_id.compare(content_name) == 0) { - ASSERT(!stream_info->channel); - stream_info->transport = transport_channel; - break; - } - } return transport_channel; } @@ -189,13 +178,7 @@ cricket::TransportChannel* WebRtcSession::GetChannel( if (!transport_) return NULL; - StreamMap::iterator iter; - for (iter = streams_.begin(); iter != streams_.end(); ++iter) { - if (content_name.compare((*iter)->stream_id) == 0) { - return (*iter)->transport; - } - } - return NULL; + return transport_->GetChannel(name); } void WebRtcSession::DestroyChannel( @@ -204,14 +187,6 @@ void WebRtcSession::DestroyChannel( return; transport_->DestroyChannel(name); - - StreamMap::iterator iter; - for (iter = streams_.begin(); iter != streams_.end(); ++iter) { - if (content_name.compare((*iter)->stream_id) == 0) { - (*iter)->transport = NULL; - break; - } - } } void WebRtcSession::OnMessage(talk_base::Message* message) { @@ -496,68 +471,6 @@ bool WebRtcSession::OnRemoteDescription( return true; } -bool WebRtcSession::OnStreamDeleteMessage( - const cricket::SessionDescription* desc, - const std::vector& candidates) { - // This will be called when session is in connected state - // In this state session expects signaling message for any removed - // streamed by the peer. - // check for candidates port, if its equal to 0, remove that stream - // and provide callback OnRemoveStream else keep as it is - - SetState(STATE_RECEIVEDTERMINATE); - for (size_t i = 0; i < candidates.size(); ++i) { - if (candidates[i].address().port() == 0) { - RemoveStreamOnRequest(candidates[i]); - } - } - - const cricket::ContentInfo* video_content = GetFirstVideoContent(desc); - if (video_content) { - SignalRemoveStream(video_content->name, true); - } else { - const cricket::ContentInfo* audio_content = GetFirstAudioContent(desc); - if (audio_content) { - SignalRemoveStream(audio_content->name, false); - } - } - return true; -} - -void WebRtcSession::RemoveStreamOnRequest( - const cricket::Candidate& candidate) { - // 1. Get Transport corresponding to candidate name - // 2. Get StreamInfo for the transport found in step 1 - // 3. call ChannelManager Destroy voice/video method -// - TransportChannelMap::iterator iter = - transport_channels_.find(candidate.name()); - if (iter == transport_channels_.end()) { - return; - } - - cricket::TransportChannel* transport = iter->second; - std::vector::iterator siter; - for (siter = streams_.begin(); siter != streams_.end(); ++siter) { - StreamInfo* stream_info = (*siter); - if (stream_info->transport == transport) { - if (!stream_info->video) { - cricket::VoiceChannel* channel = static_cast ( - stream_info->channel); - channel->Enable(false); - channel_manager_->DestroyVoiceChannel(channel); - } else { - cricket::VideoChannel* channel = static_cast ( - stream_info->channel); - channel->Enable(false); - channel_manager_->DestroyVideoChannel(channel); - } - streams_.erase(siter); - break; - } - } -} - cricket::SessionDescription* WebRtcSession::CreateOffer() { cricket::SessionDescription* offer = new cricket::SessionDescription(); StreamMap::iterator iter; @@ -626,9 +539,8 @@ cricket::SessionDescription* WebRtcSession::CreateAnswer( } } - // enabling RTCP mux by default at both ends, without - // exchanging it through signaling message. - audio_accept->set_rtcp_mux(true); + // RTCP mux is set based on what present in incoming offer + audio_accept->set_rtcp_mux(audio_offer->rtcp_mux()); audio_accept->SortCodecs(); answer->AddContent(audio_content->name, audio_content->type, audio_accept); } @@ -656,9 +568,8 @@ cricket::SessionDescription* WebRtcSession::CreateAnswer( } } - // enabling RTCP mux by default at both ends, without - // exchanging it through signaling message. - video_accept->set_rtcp_mux(true); + // RTCP mux is set based on what present in incoming offer + video_accept->set_rtcp_mux(video_offer->rtcp_mux()); video_accept->SortCodecs(); answer->AddContent(video_content->name, video_content->type, video_accept); } diff --git a/third_party_mods/libjingle/source/talk/app/webrtc/webrtcsession.h b/third_party_mods/libjingle/source/talk/app/webrtc/webrtcsession.h index 9c56241806..2e8d712dde 100644 --- a/third_party_mods/libjingle/source/talk/app/webrtc/webrtcsession.h +++ b/third_party_mods/libjingle/source/talk/app/webrtc/webrtcsession.h @@ -143,16 +143,13 @@ class WebRtcSession : public cricket::BaseSession { struct StreamInfo { explicit StreamInfo(const std::string stream_id) : channel(NULL), - transport(NULL), video(false), stream_id(stream_id) {} StreamInfo() : channel(NULL), - transport(NULL), video(false) {} cricket::BaseChannel* channel; - cricket::TransportChannel* transport; bool video; std::string stream_id; }; @@ -202,9 +199,6 @@ class WebRtcSession : public cricket::BaseSession { typedef std::map TransportChannelMap; bool SetVideoCapture(bool capture); - bool OnStreamDeleteMessage(const cricket::SessionDescription* desc, - const std::vector& candidates); - void RemoveStreamOnRequest(const cricket::Candidate& candidate); void EnableAllStreams(); cricket::Transport* transport_;