From 0013dcc0c1b541acc51394b91c559a26db3bf4bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Bostr=C3=B6m?= Date: Fri, 19 Feb 2016 20:42:19 +0100 Subject: [PATCH] Simplify SSRC usage inside ViEEncoder. Since SSRCs can no longer change on the fly, SSRC code can be made a lot simpler (and faster). Resulting code has less and shorter locking. BUG=webrtc:5494 R=danilchap@webrtc.org Review URL: https://codereview.webrtc.org/1713683003 . Cr-Commit-Position: refs/heads/master@{#11691} --- .../sanitizers/tsan_suppressions_webrtc.cc | 1 - .../video/encoder_state_feedback_unittest.cc | 3 +- webrtc/video/video_send_stream.cc | 9 +-- webrtc/video/vie_encoder.cc | 72 +++++-------------- webrtc/video/vie_encoder.h | 22 +++--- 5 files changed, 27 insertions(+), 80 deletions(-) diff --git a/webrtc/build/sanitizers/tsan_suppressions_webrtc.cc b/webrtc/build/sanitizers/tsan_suppressions_webrtc.cc index a42925620c..716378ee6b 100644 --- a/webrtc/build/sanitizers/tsan_suppressions_webrtc.cc +++ b/webrtc/build/sanitizers/tsan_suppressions_webrtc.cc @@ -79,7 +79,6 @@ char kTSanDefaultSuppressions[] = "deadlock:webrtc::RTCPReceiver::SetSsrcs\n" "deadlock:webrtc::test::UdpSocketManagerPosixImpl::RemoveSocket\n" "deadlock:webrtc::vcm::VideoReceiver::RegisterPacketRequestCallback\n" -"deadlock:webrtc::ViEEncoder::OnLocalSsrcChanged\n" // TODO(pbos): Trace events are racy due to lack of proper POD atomics. // https://code.google.com/p/webrtc/issues/detail?id=2497 diff --git a/webrtc/video/encoder_state_feedback_unittest.cc b/webrtc/video/encoder_state_feedback_unittest.cc index be5f0420e5..be81bdade2 100644 --- a/webrtc/video/encoder_state_feedback_unittest.cc +++ b/webrtc/video/encoder_state_feedback_unittest.cc @@ -29,6 +29,7 @@ class MockVieEncoder : public ViEEncoder { public: explicit MockVieEncoder(ProcessThread* process_thread, PacedSender* pacer) : ViEEncoder(1, + std::vector(), process_thread, nullptr, nullptr, @@ -44,8 +45,6 @@ class MockVieEncoder : public ViEEncoder { void(uint32_t ssrc, uint8_t picture_id)); MOCK_METHOD2(OnReceivedRPSI, void(uint32_t ssrc, uint64_t picture_id)); - MOCK_METHOD2(OnLocalSsrcChanged, - void(uint32_t old_ssrc, uint32_t new_ssrc)); }; TEST(VieKeyRequestTest, CreateAndTriggerRequests) { diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index e6674c8531..db00adf24e 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -188,6 +188,7 @@ VideoSendStream::VideoSendStream( true), vie_receiver_(vie_channel_.vie_receiver()), vie_encoder_(num_cpu_cores, + config_.rtp.ssrcs, module_process_thread_, &stats_proxy_, config.pre_encode_callback, @@ -216,8 +217,6 @@ VideoSendStream::VideoSendStream( call_stats_->RegisterStatsObserver(vie_channel_.GetStatsObserver()); - vie_encoder_.SetSsrcs(std::vector(1, config_.rtp.ssrcs[0])); - for (size_t i = 0; i < config_.rtp.extensions.size(); ++i) { const std::string& extension = config_.rtp.extensions[i].name; int id = config_.rtp.extensions[i].id; @@ -602,12 +601,6 @@ bool VideoSendStream::SetSendCodec(VideoCodec video_codec) { return false; } - // Not all configured SSRCs have to be utilized (simulcast senders don't have - // to send on all SSRCs at once etc.) - std::vector used_ssrcs = config_.rtp.ssrcs; - used_ssrcs.resize(static_cast(video_codec.numberOfSimulcastStreams)); - vie_encoder_.SetSsrcs(used_ssrcs); - // Restart the media flow vie_encoder_.Restart(); diff --git a/webrtc/video/vie_encoder.cc b/webrtc/video/vie_encoder.cc index 13d52b43bc..c9f5942979 100644 --- a/webrtc/video/vie_encoder.cc +++ b/webrtc/video/vie_encoder.cc @@ -105,6 +105,7 @@ class ViEBitrateObserver : public BitrateObserver { }; ViEEncoder::ViEEncoder(uint32_t number_of_cores, + const std::vector& ssrcs, ProcessThread* module_process_thread, SendStatisticsProxy* stats_proxy, I420FrameCallback* pre_encode_callback, @@ -113,6 +114,7 @@ ViEEncoder::ViEEncoder(uint32_t number_of_cores, PayloadRouter* payload_router, BitrateAllocator* bitrate_allocator) : number_of_cores_(number_of_cores), + ssrcs_(ssrcs), vp_(VideoProcessing::Create()), qm_callback_(new QMVideoSettingsCallback(vp_.get())), vcm_(VideoCodingModule::Create(Clock::GetRealTimeClock(), @@ -132,6 +134,7 @@ ViEEncoder::ViEEncoder(uint32_t number_of_cores, network_is_transmitting_(true), encoder_paused_(false), encoder_paused_and_dropped_frame_(false), + time_last_intra_request_ms_(ssrcs.size(), -1), module_process_thread_(module_process_thread), has_received_sli_(false), picture_id_sli_(0), @@ -479,61 +482,22 @@ void ViEEncoder::OnReceivedIntraFrameRequest(uint32_t ssrc) { // Key frame request from remote side, signal to VCM. TRACE_EVENT0("webrtc", "OnKeyFrameRequest"); - int idx = 0; - { - rtc::CritScope lock(&data_cs_); - auto stream_it = ssrc_streams_.find(ssrc); - if (stream_it == ssrc_streams_.end()) { - LOG_F(LS_WARNING) << "ssrc not found: " << ssrc << ", map size " - << ssrc_streams_.size(); - return; + for (size_t i = 0; i < ssrcs_.size(); ++i) { + if (ssrcs_[i] != ssrc) + continue; + int64_t now_ms = TickTime::MillisecondTimestamp(); + { + rtc::CritScope lock(&data_cs_); + if (time_last_intra_request_ms_[i] + kMinKeyFrameRequestIntervalMs > + now_ms) { + return; + } + time_last_intra_request_ms_[i] = now_ms; } - std::map::iterator time_it = - time_last_intra_request_ms_.find(ssrc); - if (time_it == time_last_intra_request_ms_.end()) { - time_last_intra_request_ms_[ssrc] = 0; - } - - int64_t now = TickTime::MillisecondTimestamp(); - if (time_last_intra_request_ms_[ssrc] + kMinKeyFrameRequestIntervalMs - > now) { - return; - } - time_last_intra_request_ms_[ssrc] = now; - idx = stream_it->second; - } - // Release the critsect before triggering key frame. - vcm_->IntraFrameRequest(idx); -} - -void ViEEncoder::OnLocalSsrcChanged(uint32_t old_ssrc, uint32_t new_ssrc) { - rtc::CritScope lock(&data_cs_); - std::map::iterator it = ssrc_streams_.find(old_ssrc); - if (it == ssrc_streams_.end()) { + vcm_->IntraFrameRequest(static_cast(i)); return; } - - ssrc_streams_[new_ssrc] = it->second; - ssrc_streams_.erase(it); - - std::map::iterator time_it = - time_last_intra_request_ms_.find(old_ssrc); - int64_t last_intra_request_ms = 0; - if (time_it != time_last_intra_request_ms_.end()) { - last_intra_request_ms = time_it->second; - time_last_intra_request_ms_.erase(time_it); - } - time_last_intra_request_ms_[new_ssrc] = last_intra_request_ms; -} - -void ViEEncoder::SetSsrcs(const std::vector& ssrcs) { - rtc::CritScope lock(&data_cs_); - ssrc_streams_.clear(); - time_last_intra_request_ms_.clear(); - int idx = 0; - for (uint32_t ssrc : ssrcs) { - ssrc_streams_[ssrc] = idx++; - } + RTC_NOTREACHED() << "Should not receive keyframe requests on unknown SSRCs."; } void ViEEncoder::SetMinTransmitBitrate(int min_transmit_bitrate_kbps) { @@ -554,14 +518,12 @@ void ViEEncoder::OnNetworkChanged(uint32_t bitrate_bps, bool video_is_suspended = vcm_->VideoSuspended(); bool video_suspension_changed; VideoCodec send_codec; - uint32_t first_ssrc; { rtc::CritScope lock(&data_cs_); last_observed_bitrate_bps_ = bitrate_bps; video_suspension_changed = video_suspended_ != video_is_suspended; video_suspended_ = video_is_suspended; send_codec = encoder_config_; - first_ssrc = ssrc_streams_.begin()->first; } SimulcastStream* stream_configs = send_codec.simulcastStream; @@ -574,7 +536,7 @@ void ViEEncoder::OnNetworkChanged(uint32_t bitrate_bps, return; // Video suspend-state changed, inform codec observer. LOG(LS_INFO) << "Video suspend state changed " << video_is_suspended - << " for ssrc " << first_ssrc; + << " for ssrc " << ssrcs_[0]; if (stats_proxy_) stats_proxy_->OnSuspendChange(video_is_suspended); } diff --git a/webrtc/video/vie_encoder.h b/webrtc/video/vie_encoder.h index 0456d2a3e4..d32d6c7d7e 100644 --- a/webrtc/video/vie_encoder.h +++ b/webrtc/video/vie_encoder.h @@ -11,7 +11,6 @@ #ifndef WEBRTC_VIDEO_VIE_ENCODER_H_ #define WEBRTC_VIDEO_VIE_ENCODER_H_ -#include #include #include "webrtc/base/criticalsection.h" @@ -43,8 +42,7 @@ class ViEBitrateObserver; class ViEEffectFilter; class VideoCodingModule; -class ViEEncoder : public RtcpIntraFrameObserver, - public VideoEncoderRateObserver, +class ViEEncoder : public VideoEncoderRateObserver, public VCMPacketizationCallback, public VCMSendStatisticsCallback, public VideoCaptureCallback { @@ -52,6 +50,7 @@ class ViEEncoder : public RtcpIntraFrameObserver, friend class ViEBitrateObserver; ViEEncoder(uint32_t number_of_cores, + const std::vector& ssrcs, ProcessThread* module_process_thread, SendStatisticsProxy* stats_proxy, I420FrameCallback* pre_encode_callback, @@ -108,14 +107,10 @@ class ViEEncoder : public RtcpIntraFrameObserver, int32_t SendStatistics(const uint32_t bit_rate, const uint32_t frame_rate) override; - // Implements RtcpIntraFrameObserver. - void OnReceivedIntraFrameRequest(uint32_t ssrc) override; - void OnReceivedSLI(uint32_t ssrc, uint8_t picture_id) override; - void OnReceivedRPSI(uint32_t ssrc, uint64_t picture_id) override; - void OnLocalSsrcChanged(uint32_t old_ssrc, uint32_t new_ssrc) override; - - // Sets SSRCs for all streams. - void SetSsrcs(const std::vector& ssrcs); + // virtual to test EncoderStateFeedback with mocks. + virtual void OnReceivedIntraFrameRequest(uint32_t ssrc); + virtual void OnReceivedSLI(uint32_t ssrc, uint8_t picture_id); + virtual void OnReceivedRPSI(uint32_t ssrc, uint64_t picture_id); void SetMinTransmitBitrate(int min_transmit_bitrate_kbps); @@ -142,6 +137,7 @@ class ViEEncoder : public RtcpIntraFrameObserver, void TraceFrameDropEnd() EXCLUSIVE_LOCKS_REQUIRED(data_cs_); const uint32_t number_of_cores_; + const std::vector ssrcs_; const rtc::scoped_ptr vp_; const rtc::scoped_ptr qm_callback_; @@ -168,8 +164,7 @@ class ViEEncoder : public RtcpIntraFrameObserver, bool network_is_transmitting_ GUARDED_BY(data_cs_); bool encoder_paused_ GUARDED_BY(data_cs_); bool encoder_paused_and_dropped_frame_ GUARDED_BY(data_cs_); - std::map time_last_intra_request_ms_ - GUARDED_BY(data_cs_); + std::vector time_last_intra_request_ms_ GUARDED_BY(data_cs_); ProcessThread* module_process_thread_; @@ -177,7 +172,6 @@ class ViEEncoder : public RtcpIntraFrameObserver, uint8_t picture_id_sli_ GUARDED_BY(data_cs_); bool has_received_rpsi_ GUARDED_BY(data_cs_); uint64_t picture_id_rpsi_ GUARDED_BY(data_cs_); - std::map ssrc_streams_ GUARDED_BY(data_cs_); bool video_suspended_ GUARDED_BY(data_cs_); };