zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Some cleanup in BaseChannel RTCP mux code.

Browse Source 
Removing a redundant variable used to track whether or not RTCP mux has
been fully negotiated. It's RtcpMuxFilter's job to do that, and it
already had the state, it just wasn't exposed.

Review-Url: https://codereview.webrtc.org/2260963002
Cr-Commit-Position: refs/heads/master@{#13856}
This commit is contained in:
deadbeef
2016-08-22 16:00:30 -07:00
committed by Commit bot
parent b3f1c5d2fe
commit 23d947dc98
5 changed files with 74 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

35
webrtc/pc/channel.cc
View File

@ -169,22 +169,8 @@ BaseChannel::BaseChannel(rtc::Thread* worker_thread,
content_name_(content_name),
transport_controller_(transport_controller),
rtcp_transport_enabled_(rtcp),
transport_channel_(nullptr),
rtcp_transport_channel_(nullptr),
rtp_ready_to_send_(false),
rtcp_ready_to_send_(false),
writable_(false),
was_ever_writable_(false),
has_received_packet_(false),
dtls_keyed_(false),
secure_required_(false),
rtp_abs_sendtime_extn_id_(-1),
media_channel_(media_channel),
enabled_(false),
local_content_direction_(MD_INACTIVE),
remote_content_direction_(MD_INACTIVE) {
rtcp_enabled_(rtcp),
media_channel_(media_channel) {
ASSERT(worker_thread_ == rtc::Thread::Current());
if (transport_controller) {
RTC_DCHECK_EQ(network_thread, transport_controller->network_thread());
@ -268,7 +254,7 @@ bool BaseChannel::InitNetwork_n(const std::string* bundle_transport_name) {
if (!SetDtlsSrtpCryptoSuites_n(transport_channel_, false)) {
return false;
}
if (rtcp_transport_enabled() &&
if (rtcp_transport_channel_ &&
!SetDtlsSrtpCryptoSuites_n(rtcp_transport_channel_, true)) {
return false;
}
@ -308,10 +294,12 @@ bool BaseChannel::SetTransport_n(const std::string& transport_name) {
srtp_filter_.ResetParams();
}
// TODO(guoweis): Remove this grossness when we remove non-muxed RTCP.
if (rtcp_transport_enabled()) {
// If this BaseChannel uses RTCP and we haven't fully negotiated RTCP mux,
// we need an RTCP channel.
if (rtcp_enabled_ && !rtcp_mux_filter_.IsFullyActive()) {
LOG(LS_INFO) << "Create RTCP TransportChannel for " << content_name()
<< " on " << transport_name << " transport ";
// TODO(deadbeef): Remove this grossness when we remove non-muxed RTCP.
SetRtcpTransportChannel_n(
transport_controller_->CreateTransportChannel_n(
transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTCP),
@ -328,12 +316,11 @@ bool BaseChannel::SetTransport_n(const std::string& transport_name) {
return false;
}
// TODO(guoweis): Remove this grossness when we remove non-muxed RTCP.
if (rtcp_transport_enabled()) {
// TODO(deadbeef): Remove this grossness when we remove non-muxed RTCP.
if (rtcp_transport_channel_) {
// We can only update the RTCP ready to send after set_transport_channel has
// handled channel writability.
SetReadyToSend(
true, rtcp_transport_channel_ && rtcp_transport_channel_->writable());
SetReadyToSend(true, rtcp_transport_channel_->writable());
}
transport_name_ = transport_name;
return true;
@ -1208,7 +1195,6 @@ void BaseChannel::ActivateRtcpMux_n() {
if (!rtcp_mux_filter_.IsActive()) {
rtcp_mux_filter_.SetActive();
SetRtcpTransportChannel_n(nullptr, true);
rtcp_transport_enabled_ = false;
}
}
@ -1232,7 +1218,6 @@ bool BaseChannel::SetRtcpMux_n(bool enable,
<< " by destroying RTCP transport channel for "
<< transport_name();
SetRtcpTransportChannel_n(nullptr, true);
rtcp_transport_enabled_ = false;
}
break;
case CA_UPDATE:

33
webrtc/pc/channel.h
View File

@ -70,6 +70,7 @@ class BaseChannel
public MediaChannel::NetworkInterface,
public ConnectionStatsGetter {
public:
// |rtcp| represents whether or not this channel uses RTCP.
BaseChannel(rtc::Thread* worker_thread,
rtc::Thread* network_thread,
MediaChannel* channel,
@ -197,7 +198,6 @@ class BaseChannel
rtc::Thread* signaling_thread() {
return transport_controller_->signaling_thread();
}
bool rtcp_transport_enabled() const { return rtcp_transport_enabled_; }
void ConnectToTransportChannel(TransportChannel* tc);
void DisconnectFromTransportChannel(TransportChannel* tc);
@ -342,34 +342,37 @@ class BaseChannel
// Transport related members that should be accessed from network thread.
TransportController* const transport_controller_;
std::string transport_name_;
bool rtcp_transport_enabled_;
TransportChannel* transport_channel_;
// Is RTCP used at all by this type of channel?
// Expected to be true (as of typing this) for everything except data
// channels.
const bool rtcp_enabled_;
TransportChannel* transport_channel_ = nullptr;
std::vector<std::pair<rtc::Socket::Option, int> > socket_options_;
TransportChannel* rtcp_transport_channel_;
TransportChannel* rtcp_transport_channel_ = nullptr;
std::vector<std::pair<rtc::Socket::Option, int> > rtcp_socket_options_;
SrtpFilter srtp_filter_;
RtcpMuxFilter rtcp_mux_filter_;
BundleFilter bundle_filter_;
bool rtp_ready_to_send_;
bool rtcp_ready_to_send_;
bool writable_;
bool was_ever_writable_;
bool has_received_packet_;
bool dtls_keyed_;
bool secure_required_;
bool rtp_ready_to_send_ = false;
bool rtcp_ready_to_send_ = false;
bool writable_ = false;
bool was_ever_writable_ = false;
bool has_received_packet_ = false;
bool dtls_keyed_ = false;
bool secure_required_ = false;
rtc::CryptoOptions crypto_options_;
int rtp_abs_sendtime_extn_id_;
int rtp_abs_sendtime_extn_id_ = -1;
// MediaChannel related members that should be access from worker thread.
MediaChannel* const media_channel_;
// Currently enabled_ flag accessed from signaling thread too, but it can
// be changed only when signaling thread does sunchronious call to worker
// thread, so it should be safe.
bool enabled_;
bool enabled_ = false;
std::vector<StreamParams> local_streams_;
std::vector<StreamParams> remote_streams_;
MediaContentDirection local_content_direction_;
MediaContentDirection remote_content_direction_;
MediaContentDirection local_content_direction_ = MD_INACTIVE;
MediaContentDirection remote_content_direction_ = MD_INACTIVE;
};
// VoiceChannel is a specialization that adds support for early media, DTMF,

12
webrtc/pc/rtcpmuxfilter.cc
View File

@ -17,10 +17,16 @@ namespace cricket {
RtcpMuxFilter::RtcpMuxFilter() : state_(ST_INIT), offer_enable_(false) {
}
bool RtcpMuxFilter::IsFullyActive() const {
return state_ == ST_ACTIVE;
}
bool RtcpMuxFilter::IsProvisionallyActive() const {
return state_ == ST_SENTPRANSWER || state_ == ST_RECEIVEDPRANSWER;
}
bool RtcpMuxFilter::IsActive() const {
return state_ == ST_SENTPRANSWER ||
state_ == ST_RECEIVEDPRANSWER ||
state_ == ST_ACTIVE;
return IsFullyActive() || IsProvisionallyActive();
}
void RtcpMuxFilter::SetActive() {

14
webrtc/pc/rtcpmuxfilter.h
View File

@ -21,10 +21,20 @@ class RtcpMuxFilter {
public:
RtcpMuxFilter();
// Whether the filter is active, i.e. has RTCP mux been properly negotiated.
// Whether RTCP mux has been negotiated with a final answer (not provisional).
bool IsFullyActive() const;
// Whether RTCP mux has been negotiated with a provisional answer; this means
// a later answer could disable RTCP mux, and so the RTCP transport should
// not be disposed yet.
bool IsProvisionallyActive() const;
// Whether the filter is active, i.e. has RTCP mux been properly negotiated,
// either with a final or provisional answer.
bool IsActive() const;
// Make the filter active, regardless of the current state.
// Make the filter active (fully, not provisionally) regardless of the
// current state. This should be used when an endpoint *requires* RTCP mux.
void SetActive();
// Specifies whether the offer indicates the use of RTCP mux.

26
webrtc/pc/rtcpmuxfilter_unittest.cc
View File

@ -82,12 +82,18 @@ TEST(RtcpMuxFilterTest, IsActiveSender) {
cricket::RtcpMuxFilter filter;
// Init state - not active
EXPECT_FALSE(filter.IsActive());
EXPECT_FALSE(filter.IsProvisionallyActive());
EXPECT_FALSE(filter.IsFullyActive());
// After sent offer, demux should not be active.
filter.SetOffer(true, cricket::CS_LOCAL);
EXPECT_FALSE(filter.IsActive());
EXPECT_FALSE(filter.IsProvisionallyActive());
EXPECT_FALSE(filter.IsFullyActive());
// Remote accepted, filter is now active.
filter.SetAnswer(true, cricket::CS_REMOTE);
EXPECT_TRUE(filter.IsActive());
EXPECT_FALSE(filter.IsProvisionallyActive());
EXPECT_TRUE(filter.IsFullyActive());
}
// Test that we can receive provisional answer and final answer.
@ -96,27 +102,39 @@ TEST(RtcpMuxFilterTest, ReceivePrAnswer) {
filter.SetOffer(true, cricket::CS_LOCAL);
// Received provisional answer with mux enabled.
EXPECT_TRUE(filter.SetProvisionalAnswer(true, cricket::CS_REMOTE));
// We are now active since both sender and receiver support mux.
// We are now provisionally active since both sender and receiver support mux.
EXPECT_TRUE(filter.IsActive());
EXPECT_TRUE(filter.IsProvisionallyActive());
EXPECT_FALSE(filter.IsFullyActive());
// Received provisional answer with mux disabled.
EXPECT_TRUE(filter.SetProvisionalAnswer(false, cricket::CS_REMOTE));
// We are now inactive since the receiver doesn't support mux.
EXPECT_FALSE(filter.IsActive());
EXPECT_FALSE(filter.IsProvisionallyActive());
EXPECT_FALSE(filter.IsFullyActive());
// Received final answer with mux enabled.
EXPECT_TRUE(filter.SetAnswer(true, cricket::CS_REMOTE));
EXPECT_TRUE(filter.IsActive());
EXPECT_FALSE(filter.IsProvisionallyActive());
EXPECT_TRUE(filter.IsFullyActive());
}
TEST(RtcpMuxFilterTest, IsActiveReceiver) {
cricket::RtcpMuxFilter filter;
// Init state - not active.
EXPECT_FALSE(filter.IsActive());
EXPECT_FALSE(filter.IsProvisionallyActive());
EXPECT_FALSE(filter.IsFullyActive());
// After received offer, demux should not be active
filter.SetOffer(true, cricket::CS_REMOTE);
EXPECT_FALSE(filter.IsActive());
EXPECT_FALSE(filter.IsProvisionallyActive());
EXPECT_FALSE(filter.IsFullyActive());
// We accept, filter is now active
filter.SetAnswer(true, cricket::CS_LOCAL);
EXPECT_TRUE(filter.IsActive());
EXPECT_FALSE(filter.IsProvisionallyActive());
EXPECT_TRUE(filter.IsFullyActive());
}
// Test that we can send provisional answer and final answer.
@ -126,12 +144,18 @@ TEST(RtcpMuxFilterTest, SendPrAnswer) {
// Send provisional answer with mux enabled.
EXPECT_TRUE(filter.SetProvisionalAnswer(true, cricket::CS_LOCAL));
EXPECT_TRUE(filter.IsActive());
EXPECT_TRUE(filter.IsProvisionallyActive());
EXPECT_FALSE(filter.IsFullyActive());
// Received provisional answer with mux disabled.
EXPECT_TRUE(filter.SetProvisionalAnswer(false, cricket::CS_LOCAL));
EXPECT_FALSE(filter.IsActive());
EXPECT_FALSE(filter.IsProvisionallyActive());
EXPECT_FALSE(filter.IsFullyActive());
// Send final answer with mux enabled.
EXPECT_TRUE(filter.SetAnswer(true, cricket::CS_LOCAL));
EXPECT_TRUE(filter.IsActive());
EXPECT_FALSE(filter.IsProvisionallyActive());
EXPECT_TRUE(filter.IsFullyActive());
}
// Test that we can enable the filter in an update.