From b78d4d13835f628e722a57abae2bf06ba3655921 Mon Sep 17 00:00:00 2001 From: nisse Date: Fri, 17 Feb 2017 08:34:35 -0800 Subject: [PATCH] Delete class SSRCDatabase, and its global ssrc registry, and the method RTPSender::GenerateNewSSRC. It's now mandatory for higher layers to call SetSSRC, RTPSender no longer allocates any ssrc by default. BUG=webrtc:4306,webrtc:6887 Review-Url: https://codereview.webrtc.org/2644303002 Cr-Commit-Position: refs/heads/master@{#16670} --- .../logging/rtc_event_log/rtc_event_log.proto | 2 + webrtc/modules/rtp_rtcp/BUILD.gn | 2 - .../modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 37 ------- .../modules/rtp_rtcp/source/rtp_rtcp_impl.h | 1 - webrtc/modules/rtp_rtcp/source/rtp_sender.cc | 104 +++++++----------- webrtc/modules/rtp_rtcp/source/rtp_sender.h | 12 +- .../rtp_rtcp/source/rtp_sender_unittest.cc | 8 +- .../modules/rtp_rtcp/source/ssrc_database.cc | 51 --------- .../modules/rtp_rtcp/source/ssrc_database.h | 62 ----------- webrtc/voice_engine/channel.cc | 2 +- webrtc/voice_engine/channel_manager.cc | 9 +- webrtc/voice_engine/channel_manager.h | 4 + .../test/auto_test/standard/rtp_rtcp_test.cc | 7 +- 13 files changed, 71 insertions(+), 230 deletions(-) delete mode 100644 webrtc/modules/rtp_rtcp/source/ssrc_database.cc delete mode 100644 webrtc/modules/rtp_rtcp/source/ssrc_database.h diff --git a/webrtc/logging/rtc_event_log/rtc_event_log.proto b/webrtc/logging/rtc_event_log/rtc_event_log.proto index 0da910a29f..b1ece47b6d 100644 --- a/webrtc/logging/rtc_event_log/rtc_event_log.proto +++ b/webrtc/logging/rtc_event_log/rtc_event_log.proto @@ -102,6 +102,8 @@ message RtcpPacket { } message AudioPlayoutEvent { + // TODO(ivoc): Rename, we currently use the "remote" ssrc, i.e. identifying + // the receive stream, while local_ssrc identifies the send stream, if any. // required - The SSRC of the audio stream associated with the playout event. optional uint32 local_ssrc = 2; } diff --git a/webrtc/modules/rtp_rtcp/BUILD.gn b/webrtc/modules/rtp_rtcp/BUILD.gn index da742c5480..f2e53bbbfa 100644 --- a/webrtc/modules/rtp_rtcp/BUILD.gn +++ b/webrtc/modules/rtp_rtcp/BUILD.gn @@ -143,8 +143,6 @@ rtc_static_library("rtp_rtcp") { "source/rtp_sender_video.h", "source/rtp_utility.cc", "source/rtp_utility.h", - "source/ssrc_database.cc", - "source/ssrc_database.h", "source/time_util.cc", "source/time_util.h", "source/tmmbr_help.cc", diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index f74ebba0c0..482a7418bb 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -100,7 +100,6 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) this), clock_(configuration.clock), audio_(configuration.audio), - collision_detected_(false), last_process_time_(configuration.clock->TimeInMilliseconds()), last_bitrate_process_time_(configuration.clock->TimeInMilliseconds()), last_rtt_process_time_(configuration.clock->TimeInMilliseconds()), @@ -112,11 +111,6 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) remote_bitrate_(configuration.remote_bitrate_estimator), rtt_stats_(configuration.rtt_stats), rtt_ms_(0) { - // Make sure that RTCP objects are aware of our SSRC. - uint32_t SSRC = rtp_sender_.SSRC(); - rtcp_sender_.SetSSRC(SSRC); - SetRtcpReceiverSsrcs(SSRC); - // Make sure rtcp sender use same timestamp offset as rtp sender. rtcp_sender_.SetTimestampOffset(rtp_sender_.TimestampOffset()); @@ -355,19 +349,6 @@ int32_t ModuleRtpRtcpImpl::SetSendingStatus(const bool sending) { if (rtcp_sender_.SetSendingStatus(GetFeedbackState(), sending) != 0) { LOG(LS_WARNING) << "Failed to send RTCP BYE"; } - - collision_detected_ = false; - - // Generate a new SSRC for the next "call" if false - rtp_sender_.SetSendingStatus(sending); - - // Make sure that RTCP objects are aware of our SSRC (it could have changed - // Due to collision) - uint32_t SSRC = rtp_sender_.SSRC(); - rtcp_sender_.SetSSRC(SSRC); - SetRtcpReceiverSsrcs(SSRC); - - return 0; } return 0; } @@ -794,24 +775,6 @@ void ModuleRtpRtcpImpl::SetRemoteSSRC(const uint32_t ssrc) { // Inform about the incoming SSRC. rtcp_sender_.SetRemoteSSRC(ssrc); rtcp_receiver_.SetRemoteSSRC(ssrc); - - // Check for a SSRC collision. - if (rtp_sender_.SSRC() == ssrc && !collision_detected_) { - // If we detect a collision change the SSRC but only once. - collision_detected_ = true; - uint32_t new_ssrc = rtp_sender_.GenerateNewSSRC(); - if (new_ssrc == 0) { - // Configured via API ignore. - return; - } - if (RtcpMode::kOff != rtcp_sender_.Status()) { - // Send RTCP bye on the current SSRC. - SendRTCP(kRtcpBye); - } - // Change local SSRC and inform all objects about the new SSRC. - rtcp_sender_.SetSSRC(new_ssrc); - SetRtcpReceiverSsrcs(new_ssrc); - } } void ModuleRtpRtcpImpl::BitrateSent(uint32_t* total_rate, diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h index f1af359137..c15515dc48 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -333,7 +333,6 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp { bool TimeToSendFullNackList(int64_t now) const; const bool audio_; - bool collision_detected_; int64_t last_process_time_; int64_t last_bitrate_process_time_; int64_t last_rtt_process_time_; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc index 47ec31b918..7283663529 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc @@ -111,10 +111,8 @@ RTPSender::RTPSender( send_packet_observer_(send_packet_observer), bitrate_callback_(bitrate_callback), // RTP variables - ssrc_db_(SSRCDatabase::GetSSRCDatabase()), remote_ssrc_(0), sequence_number_forced_(false), - ssrc_forced_(false), last_rtp_timestamp_(0), capture_time_ms_(0), last_timestamp_time_ms_(0), @@ -128,11 +126,6 @@ RTPSender::RTPSender( send_side_bwe_with_overhead_( webrtc::field_trial::FindFullName( "WebRTC-SendSideBwe-WithOverhead") == "Enabled") { - ssrc_ = ssrc_db_->CreateSSRC(); - RTC_DCHECK(ssrc_ != 0); - ssrc_rtx_ = ssrc_db_->CreateSSRC(); - RTC_DCHECK(ssrc_rtx_ != 0); - // This random initialization is not intended to be cryptographic strong. timestamp_offset_ = random_.Rand(); // Random start, 16 bits. Can't be 0. @@ -157,12 +150,6 @@ RTPSender::~RTPSender() { // variables but we grab them in all other methods. (what's the design?) // Start documenting what thread we're on in what method so that it's easier // to understand performance attributes and possibly remove locks. - if (remote_ssrc_ != 0) { - ssrc_db_->ReturnSSRC(remote_ssrc_); - } - ssrc_db_->ReturnSSRC(ssrc_); - - SSRCDatabase::ReturnSSRCDatabase(); while (!payload_type_map_.empty()) { std::map::iterator it = payload_type_map_.begin(); @@ -210,7 +197,7 @@ int32_t RTPSender::RegisterRtpHeaderExtension(RTPExtensionType type, return rtp_header_extension_map_.Register(type, id); case kRtpExtensionNone: case kRtpExtensionNumberOfExtensions: - LOG(LS_ERROR) << "Invalid RTP extension type for registration"; + LOG(LS_ERROR) << "Invalid RTP extension type for registration."; return -1; } return -1; @@ -334,12 +321,13 @@ int RTPSender::RtxStatus() const { void RTPSender::SetRtxSsrc(uint32_t ssrc) { rtc::CritScope lock(&send_critsect_); - ssrc_rtx_ = ssrc; + ssrc_rtx_.emplace(ssrc); } uint32_t RTPSender::RtxSsrc() const { rtc::CritScope lock(&send_critsect_); - return ssrc_rtx_; + RTC_DCHECK(ssrc_rtx_); + return *ssrc_rtx_; } void RTPSender::SetRtxPayloadType(int payload_type, @@ -348,7 +336,7 @@ void RTPSender::SetRtxPayloadType(int payload_type, RTC_DCHECK_LE(payload_type, 127); RTC_DCHECK_LE(associated_payload_type, 127); if (payload_type < 0) { - LOG(LS_ERROR) << "Invalid RTX payload type: " << payload_type; + LOG(LS_ERROR) << "Invalid RTX payload type: " << payload_type << "."; return; } @@ -360,7 +348,7 @@ int32_t RTPSender::CheckPayloadType(int8_t payload_type, rtc::CritScope lock(&send_critsect_); if (payload_type < 0) { - LOG(LS_ERROR) << "Invalid payload_type " << payload_type; + LOG(LS_ERROR) << "Invalid payload_type " << payload_type << "."; return -1; } if (payload_type_ == payload_type) { @@ -401,7 +389,9 @@ bool RTPSender::SendOutgoingData(FrameType frame_type, { // Drop this packet if we're not sending media packets. rtc::CritScope lock(&send_critsect_); - ssrc = ssrc_; + RTC_DCHECK(ssrc_); + + ssrc = *ssrc_; sequence_number = sequence_number_; rtp_timestamp = timestamp_offset_ + capture_timestamp; if (transport_frame_id_out) @@ -521,7 +511,14 @@ size_t RTPSender::SendPadData(size_t bytes, int probe_cluster_id) { if (!audio_configured_ && !last_packet_marker_bit_) { break; } - ssrc = ssrc_; + if (!ssrc_) { + LOG(LS_ERROR) << "SSRC unset."; + return 0; + } + + RTC_DCHECK(ssrc_); + ssrc = *ssrc_; + sequence_number = sequence_number_; ++sequence_number_; payload_type = payload_type_; @@ -545,7 +542,12 @@ size_t RTPSender::SendPadData(size_t bytes, int probe_cluster_id) { (now_ms - last_timestamp_time_ms_) * kTimestampTicksPerMs; capture_time_ms += (now_ms - last_timestamp_time_ms_); } - ssrc = ssrc_rtx_; + if (!ssrc_rtx_) { + LOG(LS_ERROR) << "RTX SSRC unset."; + return 0; + } + RTC_DCHECK(ssrc_rtx_); + ssrc = *ssrc_rtx_; sequence_number = sequence_number_rtx_; ++sequence_number_rtx_; payload_type = rtx_payload_type_map_.begin()->second; @@ -645,7 +647,7 @@ bool RTPSender::SendPacketToNetwork(const RtpPacketToSend& packet, "sent", bytes_sent); // TODO(pwestin): Add a separate bitrate for sent bitrate after pacer. if (bytes_sent <= 0) { - LOG(LS_WARNING) << "Transport failed to send packet"; + LOG(LS_WARNING) << "Transport failed to send packet."; return false; } return true; @@ -675,7 +677,7 @@ void RTPSender::OnReceivedNack( if (bytes_sent < 0) { // Failed to send one Sequence number. Give up the rest in this nack. LOG(LS_WARNING) << "Failed resending RTP packet " << seq_no - << ", Discard rest of packets"; + << ", Discard rest of packets."; break; } } @@ -919,7 +921,9 @@ void RTPSender::UpdateDelayStatistics(int64_t capture_time_ms, int64_t now_ms) { int max_delay_ms = 0; { rtc::CritScope lock(&send_critsect_); - ssrc = ssrc_; + if (!ssrc_) + return; + ssrc = *ssrc_; } { rtc::CritScope cs(&statistics_crit_); @@ -959,7 +963,9 @@ void RTPSender::ProcessBitrate() { uint32_t ssrc; { rtc::CritScope lock(&send_critsect_); - ssrc = ssrc_; + if (!ssrc_) + return; + ssrc = *ssrc_; } rtc::CritScope lock(&statistics_crit_); @@ -993,7 +999,8 @@ std::unique_ptr RTPSender::AllocatePacket() const { rtc::CritScope lock(&send_critsect_); std::unique_ptr packet( new RtpPacketToSend(&rtp_header_extension_map_, max_packet_size_)); - packet->SetSsrc(ssrc_); + RTC_DCHECK(ssrc_); + packet->SetSsrc(*ssrc_); packet->SetCsrcs(csrcs_); // Reserve extensions, if registered, RtpSender set in SendToNetwork. packet->ReserveExtension(); @@ -1010,7 +1017,7 @@ bool RTPSender::AssignSequenceNumber(RtpPacketToSend* packet) { rtc::CritScope lock(&send_critsect_); if (!sending_media_) return false; - RTC_DCHECK_EQ(packet->Ssrc(), ssrc_); + RTC_DCHECK(packet->Ssrc() == ssrc_); packet->SetSequenceNumber(sequence_number_++); // Remember marker bit to determine if padding can be inserted with @@ -1042,23 +1049,6 @@ bool RTPSender::UpdateTransportSequenceNumber(RtpPacketToSend* packet, return true; } -void RTPSender::SetSendingStatus(bool enabled) { - if (!enabled) { - rtc::CritScope lock(&send_critsect_); - if (!ssrc_forced_) { - // Generate a new SSRC. - ssrc_db_->ReturnSSRC(ssrc_); - ssrc_ = ssrc_db_->CreateSSRC(); - RTC_DCHECK(ssrc_ != 0); - } - // Don't initialize seq number if SSRC passed externally. - if (!sequence_number_forced_ && !ssrc_forced_) { - // Generate a new sequence number. - sequence_number_ = random_.Rand(1, kMaxInitRtpSeqNumber); - } - } -} - void RTPSender::SetSendingMediaStatus(bool enabled) { rtc::CritScope lock(&send_critsect_); sending_media_ = enabled; @@ -1079,29 +1069,14 @@ uint32_t RTPSender::TimestampOffset() const { return timestamp_offset_; } -uint32_t RTPSender::GenerateNewSSRC() { - // If configured via API, return 0. - rtc::CritScope lock(&send_critsect_); - - if (ssrc_forced_) { - return 0; - } - ssrc_ = ssrc_db_->CreateSSRC(); - RTC_DCHECK(ssrc_ != 0); - return ssrc_; -} - void RTPSender::SetSSRC(uint32_t ssrc) { // This is configured via the API. rtc::CritScope lock(&send_critsect_); - if (ssrc_ == ssrc && ssrc_forced_) { + if (ssrc_ == ssrc) { return; // Since it's same ssrc, don't reset anything. } - ssrc_forced_ = true; - ssrc_db_->ReturnSSRC(ssrc_); - ssrc_db_->RegisterSSRC(ssrc); - ssrc_ = ssrc; + ssrc_.emplace(ssrc); if (!sequence_number_forced_) { sequence_number_ = random_.Rand(1, kMaxInitRtpSeqNumber); } @@ -1109,7 +1084,8 @@ void RTPSender::SetSSRC(uint32_t ssrc) { uint32_t RTPSender::SSRC() const { rtc::CritScope lock(&send_critsect_); - return ssrc_; + RTC_DCHECK(ssrc_); + return *ssrc_; } rtc::Optional RTPSender::FlexfecSsrc() const { @@ -1189,6 +1165,8 @@ std::unique_ptr RTPSender::BuildRtxPacket( if (!sending_media_) return nullptr; + RTC_DCHECK(ssrc_rtx_); + // Replace payload type. auto kv = rtx_payload_type_map_.find(packet.PayloadType()); if (kv == rtx_payload_type_map_.end()) @@ -1199,7 +1177,7 @@ std::unique_ptr RTPSender::BuildRtxPacket( rtx_packet->SetSequenceNumber(sequence_number_rtx_++); // Replace SSRC. - rtx_packet->SetSsrc(ssrc_rtx_); + rtx_packet->SetSsrc(*ssrc_rtx_); } uint8_t* rtx_payload = diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.h b/webrtc/modules/rtp_rtcp/source/rtp_sender.h index 09884b374d..8f71b48899 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.h @@ -32,7 +32,6 @@ #include "webrtc/modules/rtp_rtcp/source/rtp_packet_history.h" #include "webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h" #include "webrtc/modules/rtp_rtcp/source/rtp_utility.h" -#include "webrtc/modules/rtp_rtcp/source/ssrc_database.h" namespace webrtc { @@ -87,8 +86,6 @@ class RTPSender { int8_t SendPayloadType() const; - void SetSendingStatus(bool enabled); - void SetSendingMediaStatus(bool enabled); bool SendingMedia() const; @@ -98,7 +95,6 @@ class RTPSender { uint32_t TimestampOffset() const; void SetTimestampOffset(uint32_t timestamp); - uint32_t GenerateNewSSRC(); void SetSSRC(uint32_t ssrc); uint16_t SequenceNumber() const; @@ -305,13 +301,13 @@ class RTPSender { // RTP variables uint32_t timestamp_offset_ GUARDED_BY(send_critsect_); - SSRCDatabase* const ssrc_db_; uint32_t remote_ssrc_ GUARDED_BY(send_critsect_); bool sequence_number_forced_ GUARDED_BY(send_critsect_); uint16_t sequence_number_ GUARDED_BY(send_critsect_); uint16_t sequence_number_rtx_ GUARDED_BY(send_critsect_); - bool ssrc_forced_ GUARDED_BY(send_critsect_); - uint32_t ssrc_ GUARDED_BY(send_critsect_); + // Must be explicitly set by the application, use of rtc::Optional + // only to keep track of correct use. + rtc::Optional ssrc_ GUARDED_BY(send_critsect_); uint32_t last_rtp_timestamp_ GUARDED_BY(send_critsect_); int64_t capture_time_ms_ GUARDED_BY(send_critsect_); int64_t last_timestamp_time_ms_ GUARDED_BY(send_critsect_); @@ -319,7 +315,7 @@ class RTPSender { bool last_packet_marker_bit_ GUARDED_BY(send_critsect_); std::vector csrcs_ GUARDED_BY(send_critsect_); int rtx_ GUARDED_BY(send_critsect_); - uint32_t ssrc_rtx_ GUARDED_BY(send_critsect_); + rtc::Optional ssrc_rtx_ GUARDED_BY(send_critsect_); // Mapping rtx_payload_type_map_[associated] = rtx. std::map rtx_payload_type_map_ GUARDED_BY(send_critsect_); size_t rtp_overhead_bytes_per_packet_ GUARDED_BY(send_critsect_); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 5c0ff1f1ae..22fe71e593 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -343,6 +343,7 @@ TEST_F(RtpSenderTestWithoutPacer, SendsPacketsWithTransportSequenceNumber) { false, &fake_clock_, &transport_, nullptr, nullptr, &seq_num_allocator_, &feedback_observer_, nullptr, nullptr, nullptr, &mock_rtc_event_log_, &send_packet_observer_, &retransmission_rate_limiter_, nullptr)); + rtp_sender_->SetSSRC(kSsrc); EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( kRtpExtensionTransportSequenceNumber, kTransportSequenceNumberExtensionId)); @@ -960,7 +961,7 @@ TEST_F(RtpSenderTest, FrameCountCallbacks) { new RTPSender(false, &fake_clock_, &transport_, &mock_paced_sender_, nullptr, nullptr, nullptr, nullptr, &callback, nullptr, nullptr, nullptr, &retransmission_rate_limiter_, nullptr)); - + rtp_sender_->SetSSRC(kSsrc); char payload_name[RTP_PAYLOAD_NAME_SIZE] = "GENERIC"; const uint8_t payload_type = 127; ASSERT_EQ(0, rtp_sender_->RegisterPayload(payload_name, payload_type, 90000, @@ -1022,6 +1023,7 @@ TEST_F(RtpSenderTest, BitrateCallbacks) { nullptr, nullptr, nullptr, &callback, nullptr, nullptr, nullptr, nullptr, &retransmission_rate_limiter_, nullptr)); + rtp_sender_->SetSSRC(kSsrc); // Simulate kNumPackets sent with kPacketInterval ms intervals, with the // number of packets selected so that we fill (but don't overflow) the one @@ -1080,6 +1082,7 @@ class RtpSenderAudioTest : public RtpSenderTest { nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, &retransmission_rate_limiter_, nullptr)); + rtp_sender_->SetSSRC(kSsrc); rtp_sender_->SetSequenceNumber(kSeqNum); } }; @@ -1440,6 +1443,7 @@ TEST_F(RtpSenderTest, OnOverheadChanged) { new RTPSender(false, &fake_clock_, &transport_, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, &retransmission_rate_limiter_, &mock_overhead_observer)); + rtp_sender_->SetSSRC(kSsrc); // RTP overhead is 12B. EXPECT_CALL(mock_overhead_observer, OnOverheadChanged(12)).Times(1); @@ -1460,6 +1464,7 @@ TEST_F(RtpSenderTest, DoesNotUpdateOverheadOnEqualSize) { new RTPSender(false, &fake_clock_, &transport_, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, &retransmission_rate_limiter_, &mock_overhead_observer)); + rtp_sender_->SetSSRC(kSsrc); EXPECT_CALL(mock_overhead_observer, OnOverheadChanged(_)).Times(1); SendGenericPayload(); @@ -1475,6 +1480,7 @@ TEST_F(RtpSenderTest, AddOverheadToTransportFeedbackObserver) { false, &fake_clock_, &transport_, nullptr, nullptr, &seq_num_allocator_, &feedback_observer_, nullptr, nullptr, nullptr, &mock_rtc_event_log_, nullptr, &retransmission_rate_limiter_, &mock_overhead_observer)); + rtp_sender_->SetSSRC(kSsrc); EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( kRtpExtensionTransportSequenceNumber, kTransportSequenceNumberExtensionId)); diff --git a/webrtc/modules/rtp_rtcp/source/ssrc_database.cc b/webrtc/modules/rtp_rtcp/source/ssrc_database.cc deleted file mode 100644 index a96d05db46..0000000000 --- a/webrtc/modules/rtp_rtcp/source/ssrc_database.cc +++ /dev/null @@ -1,51 +0,0 @@ -/* - * Copyright (c) 2011 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "webrtc/modules/rtp_rtcp/source/ssrc_database.h" -#include "webrtc/base/timeutils.h" -#include "webrtc/base/checks.h" - -namespace webrtc { - -SSRCDatabase* SSRCDatabase::GetSSRCDatabase() { - return GetStaticInstance(kAddRef); -} - -void SSRCDatabase::ReturnSSRCDatabase() { - GetStaticInstance(kRelease); -} - -uint32_t SSRCDatabase::CreateSSRC() { - rtc::CritScope lock(&crit_); - - while (true) { // Try until get a new ssrc. - // 0 and 0xffffffff are invalid values for SSRC. - uint32_t ssrc = random_.Rand(1u, 0xfffffffe); - if (ssrcs_.insert(ssrc).second) { - return ssrc; - } - } -} - -void SSRCDatabase::RegisterSSRC(uint32_t ssrc) { - rtc::CritScope lock(&crit_); - ssrcs_.insert(ssrc); -} - -void SSRCDatabase::ReturnSSRC(uint32_t ssrc) { - rtc::CritScope lock(&crit_); - ssrcs_.erase(ssrc); -} - -SSRCDatabase::SSRCDatabase() : random_(rtc::TimeMicros()) {} - -SSRCDatabase::~SSRCDatabase() {} - -} // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/ssrc_database.h b/webrtc/modules/rtp_rtcp/source/ssrc_database.h deleted file mode 100644 index 2f6357aec0..0000000000 --- a/webrtc/modules/rtp_rtcp/source/ssrc_database.h +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Copyright (c) 2011 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#ifndef WEBRTC_MODULES_RTP_RTCP_SOURCE_SSRC_DATABASE_H_ -#define WEBRTC_MODULES_RTP_RTCP_SOURCE_SSRC_DATABASE_H_ - -#include - -#include "webrtc/base/criticalsection.h" -#include "webrtc/base/random.h" -#include "webrtc/base/thread_annotations.h" -#include "webrtc/system_wrappers/include/static_instance.h" -#include "webrtc/typedefs.h" - -namespace webrtc { - -// TODO(tommi, holmer): Look into whether we can eliminate locking in this -// class or the class itself completely once voice engine doesn't rely on it. -// At the moment voe_auto_test requires locking, but it's not clear if that's -// an issue with the test code or if it reflects real world usage or if that's -// the best design performance wise. -// If we do decide to keep the class, we should at least get rid of using -// StaticInstance. -class SSRCDatabase { - public: - static SSRCDatabase* GetSSRCDatabase(); - static void ReturnSSRCDatabase(); - - uint32_t CreateSSRC(); - void RegisterSSRC(uint32_t ssrc); - void ReturnSSRC(uint32_t ssrc); - - protected: - SSRCDatabase(); - ~SSRCDatabase(); - - static SSRCDatabase* CreateInstance() { return new SSRCDatabase(); } - - // Friend function to allow the SSRC destructor to be accessed from the - // template class. - friend SSRCDatabase* GetStaticInstance( - CountOperation count_operation); - - private: - rtc::CriticalSection crit_; - Random random_ GUARDED_BY(crit_); - std::set ssrcs_ GUARDED_BY(crit_); - // TODO(tommi): Use a thread checker to ensure the object is created and - // deleted on the same thread. At the moment this isn't possible due to - // voe::ChannelOwner in voice engine. To reproduce, run: - // voe_auto_test --automated --gtest_filter=*MixManyChannelsForStressOpus -}; -} // namespace webrtc - -#endif // WEBRTC_MODULES_RTP_RTCP_SOURCE_SSRC_DATABASE_H_ diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc index b0aa654f27..2302acc087 100644 --- a/webrtc/voice_engine/channel.cc +++ b/webrtc/voice_engine/channel.cc @@ -583,7 +583,7 @@ MixerParticipant::AudioFrameInfo Channel::GetAudioFrameWithMuted( int32_t id, AudioFrame* audioFrame) { unsigned int ssrc; - RTC_CHECK_EQ(GetLocalSSRC(ssrc), 0); + RTC_CHECK_EQ(GetRemoteSSRC(ssrc), 0); event_log_proxy_->LogAudioPlayout(ssrc); // Get 10ms raw PCM data from the ACM (mixer limits output frequency) bool muted; diff --git a/webrtc/voice_engine/channel_manager.cc b/webrtc/voice_engine/channel_manager.cc index b8ffe6e796..b81c8a63cc 100644 --- a/webrtc/voice_engine/channel_manager.cc +++ b/webrtc/voice_engine/channel_manager.cc @@ -10,6 +10,7 @@ #include "webrtc/voice_engine/channel_manager.h" +#include "webrtc/base/timeutils.h" #include "webrtc/voice_engine/channel.h" namespace webrtc { @@ -45,12 +46,18 @@ ChannelOwner::ChannelRef::ChannelRef(class Channel* channel) : channel(channel), ref_count(1) {} ChannelManager::ChannelManager(uint32_t instance_id) - : instance_id_(instance_id), last_channel_id_(-1) {} + : instance_id_(instance_id), + last_channel_id_(-1), + random_(rtc::TimeNanos()) {} ChannelOwner ChannelManager::CreateChannel( const VoEBase::ChannelConfig& config) { Channel* channel; Channel::CreateChannel(channel, ++last_channel_id_, instance_id_, config); + // TODO(solenberg): Delete this, users should configure ssrc + // explicitly. + channel->SetLocalSSRC(random_.Rand()); + ChannelOwner channel_owner(channel); rtc::CritScope crit(&lock_); diff --git a/webrtc/voice_engine/channel_manager.h b/webrtc/voice_engine/channel_manager.h index ae16aca2b8..65cf9942d1 100644 --- a/webrtc/voice_engine/channel_manager.h +++ b/webrtc/voice_engine/channel_manager.h @@ -16,6 +16,7 @@ #include "webrtc/base/constructormagic.h" #include "webrtc/base/criticalsection.h" +#include "webrtc/base/random.h" #include "webrtc/base/scoped_ref_ptr.h" #include "webrtc/system_wrappers/include/atomic32.h" #include "webrtc/typedefs.h" @@ -116,6 +117,9 @@ class ChannelManager { rtc::CriticalSection lock_; std::vector channels_; + // For generation of random ssrc:s. + webrtc::Random random_; + RTC_DISALLOW_COPY_AND_ASSIGN(ChannelManager); }; } // namespace voe diff --git a/webrtc/voice_engine/test/auto_test/standard/rtp_rtcp_test.cc b/webrtc/voice_engine/test/auto_test/standard/rtp_rtcp_test.cc index b736aac420..0cb999029c 100644 --- a/webrtc/voice_engine/test/auto_test/standard/rtp_rtcp_test.cc +++ b/webrtc/voice_engine/test/auto_test/standard/rtp_rtcp_test.cc @@ -91,9 +91,10 @@ TEST_F(RtpRtcpTest, RemoteRtcpCnameHasPropagatedToRemoteSide) { return; } - // We need to sleep a bit here for the name to propagate. For instance, - // 200 milliseconds is not enough, so we'll go with one second here. - Sleep(1000); + // We need to sleep a bit here for the name to propagate. For + // instance, 200 milliseconds is not enough, 1 second still flaky, + // so we'll go with five seconds here. + Sleep(5000); char char_buffer[256]; voe_rtp_rtcp_->GetRemoteRTCP_CNAME(channel_, char_buffer);