From c69385de8b614b7edba4f640bca6aa0963e1505c Mon Sep 17 00:00:00 2001 From: nisse Date: Thu, 9 Mar 2017 06:13:20 -0800 Subject: [PATCH] Add |protected_by_flexfec| flag to VideoReceiveStream::Config. Use of FlexFEC is known when streams are created in WebRtcVideoChannel2, so this replaces the code in Call to infer FlexFEC config of video streams from the configuration of the FlexFEC stream(s). This also allows us to switch to a more logical creation order, where media streams are created before the FlexFEC stream. This is done in preparation for a larger refactoring of the RTP demuxing done in Call. BUG=None Review-Url: https://codereview.webrtc.org/2712683002 Cr-Commit-Position: refs/heads/master@{#17143} --- webrtc/call/call.cc | 12 ++---------- webrtc/media/engine/webrtcvideoengine2.cc | 12 +++++++++--- webrtc/video/video_receive_stream.cc | 6 ++---- webrtc/video/video_receive_stream.h | 2 -- webrtc/video/video_receive_stream_unittest.cc | 1 - webrtc/video_receive_stream.h | 3 +++ 6 files changed, 16 insertions(+), 20 deletions(-) diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc index 730233590f..bb83b72016 100644 --- a/webrtc/call/call.cc +++ b/webrtc/call/call.cc @@ -682,17 +682,9 @@ webrtc::VideoReceiveStream* Call::CreateVideoReceiveStream( TRACE_EVENT0("webrtc", "Call::CreateVideoReceiveStream"); RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread()); - bool protected_by_flexfec = false; - { - ReadLockScoped read_lock(*receive_crit_); - protected_by_flexfec = - flexfec_receive_ssrcs_media_.find(configuration.rtp.remote_ssrc) != - flexfec_receive_ssrcs_media_.end(); - } VideoReceiveStream* receive_stream = new VideoReceiveStream( - num_cpu_cores_, protected_by_flexfec, - &packet_router_, std::move(configuration), module_process_thread_.get(), - call_stats_.get(), &remb_); + num_cpu_cores_, &packet_router_, std::move(configuration), + module_process_thread_.get(), call_stats_.get(), &remb_); const webrtc::VideoReceiveStream::Config& config = receive_stream->config(); ReceiveRtpConfig receive_config(config.rtp.extensions, diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc index 8b4c5df181..6723b1b89f 100644 --- a/webrtc/media/engine/webrtcvideoengine2.cc +++ b/webrtc/media/engine/webrtcvideoengine2.cc @@ -2310,12 +2310,18 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::RecreateWebRtcStream() { call_->DestroyFlexfecReceiveStream(flexfec_stream_); flexfec_stream_ = nullptr; } - if (flexfec_config_.IsCompleteAndEnabled()) { + const bool use_flexfec = flexfec_config_.IsCompleteAndEnabled(); + // TODO(nisse): There are way too many copies here. And why isn't + // the argument to CreateVideoReceiveStream a const ref? + webrtc::VideoReceiveStream::Config config = config_.Copy(); + config.rtp.protected_by_flexfec = use_flexfec; + stream_ = call_->CreateVideoReceiveStream(config.Copy()); + stream_->Start(); + + if (use_flexfec) { flexfec_stream_ = call_->CreateFlexfecReceiveStream(flexfec_config_); flexfec_stream_->Start(); } - stream_ = call_->CreateVideoReceiveStream(config_.Copy()); - stream_->Start(); } void WebRtcVideoChannel2::WebRtcVideoReceiveStream::ClearDecoders( diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc index 32bf0f097b..edee9094a8 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -176,7 +176,6 @@ namespace internal { VideoReceiveStream::VideoReceiveStream( int num_cpu_cores, - bool protected_by_flexfec, PacketRouter* packet_router, VideoReceiveStream::Config config, ProcessThread* process_thread, @@ -185,7 +184,6 @@ VideoReceiveStream::VideoReceiveStream( : transport_adapter_(config.rtcp_send_transport), config_(std::move(config)), num_cpu_cores_(num_cpu_cores), - protected_by_flexfec_(protected_by_flexfec), process_thread_(process_thread), clock_(Clock::GetRealTimeClock()), decode_thread_(DecodeThreadFunction, this, "DecodingThread"), @@ -271,8 +269,8 @@ void VideoReceiveStream::Start() { if (decode_thread_.IsRunning()) return; - bool protected_by_fec = - protected_by_flexfec_ || rtp_stream_receiver_.IsUlpfecEnabled(); + bool protected_by_fec = config_.rtp.protected_by_flexfec || + rtp_stream_receiver_.IsUlpfecEnabled(); frame_buffer_->Start(); call_stats_->RegisterStatsObserver(&rtp_stream_receiver_); diff --git a/webrtc/video/video_receive_stream.h b/webrtc/video/video_receive_stream.h index 4223081054..cb2c73fe28 100644 --- a/webrtc/video/video_receive_stream.h +++ b/webrtc/video/video_receive_stream.h @@ -50,7 +50,6 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream, public Syncable { public: VideoReceiveStream(int num_cpu_cores, - bool protected_by_flexfec, PacketRouter* packet_router, VideoReceiveStream::Config config, ProcessThread* process_thread, @@ -119,7 +118,6 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream, TransportAdapter transport_adapter_; const VideoReceiveStream::Config config_; const int num_cpu_cores_; - const bool protected_by_flexfec_; ProcessThread* const process_thread_; Clock* const clock_; diff --git a/webrtc/video/video_receive_stream_unittest.cc b/webrtc/video/video_receive_stream_unittest.cc index 6160e284f4..9824890be8 100644 --- a/webrtc/video/video_receive_stream_unittest.cc +++ b/webrtc/video/video_receive_stream_unittest.cc @@ -93,7 +93,6 @@ class VideoReceiveStreamTest : public testing::Test { video_receive_stream_.reset(new webrtc::internal::VideoReceiveStream( kDefaultNumCpuCores, - false, // flex_fec &packet_router_, config_.Copy(), process_thread_.get(), &call_stats_, nullptr)); // remb } diff --git a/webrtc/video_receive_stream.h b/webrtc/video_receive_stream.h index 12629d0637..33d779d9b3 100644 --- a/webrtc/video_receive_stream.h +++ b/webrtc/video_receive_stream.h @@ -147,6 +147,9 @@ class VideoReceiveStream { // SSRC for retransmissions. uint32_t rtx_ssrc = 0; + // Set if the stream is protected using FlexFEC. + bool protected_by_flexfec = false; + // Map from video payload type (apt) -> RTX payload type (pt). // For RTX to be enabled, both an SSRC and this mapping are needed. std::map rtx_payload_types;