From e8964903a948a3f67c29cb22ee4aaf9c931e4672 Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Fri, 29 Mar 2019 15:33:01 +0000 Subject: [PATCH] Revert "Fix target bitrate RTCP messages behavior for SVC streams" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit ab65d8aab5fe63619033371fca1ce2711c2c2137. Reason for revert: Fails video_engine_tests ExtendedReportsEndToEndTest.TestExtendedReportsCanSignalZeroTargetBitrate https://ci.chromium.org/p/webrtc/builders/ci/Linux%20MSan/18366 Original change's description: > Fix target bitrate RTCP messages behavior for SVC streams > > Before this CL for SVC streams (e.g VP9) still 3 separate RTP_RTCP senders > were created. The RTCP target bitrate messages were treated as simulcast > and were split and send for each separate spatial layer in a separate SSRC. > > To fix that an svc flag is now wired to VideoSendStream config > and filled based on the encoder config in WebrtcVideoEngine. This flag is > used to differentiate between simulcast and SVC mode in RtpVideoSender. > > Bug: webrtc:10485 > Change-Id: Ifa01d12a7d4f01fcbe448ad11e0cc39ab2d1df55 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/129929 > Commit-Queue: Ilya Nikolaevskiy > Reviewed-by: Niels Moller > Reviewed-by: Erik Språng > Cr-Commit-Position: refs/heads/master@{#27345} TBR=ilnik@webrtc.org,nisse@webrtc.org,sprang@webrtc.org Change-Id: I184f87289d9dccc67de165038d76a5690158a3b5 No-Tree-Checks: True No-Try: True Bug: webrtc:10485 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/130467 Commit-Queue: Oleh Prypin Reviewed-by: Oleh Prypin Reviewed-by: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/master@{#27355} --- call/rtp_transport_controller_send.cc | 3 +-- call/rtp_transport_controller_send.h | 1 - call/rtp_transport_controller_send_interface.h | 1 - call/rtp_video_sender.cc | 6 ++---- call/rtp_video_sender.h | 3 --- call/rtp_video_sender_unittest.cc | 2 +- call/test/mock_rtp_transport_controller_send.h | 3 +-- call/video_send_stream.h | 4 ---- media/engine/webrtc_video_engine.cc | 1 - video/video_quality_test.cc | 3 --- video/video_send_stream_impl.cc | 1 - video/video_send_stream_impl_unittest.cc | 2 +- 12 files changed, 6 insertions(+), 24 deletions(-) diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index 2816550086..ab95907ac8 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -104,14 +104,13 @@ RtpVideoSenderInterface* RtpTransportControllerSend::CreateRtpVideoSender( const RtpConfig& rtp_config, int rtcp_report_interval_ms, Transport* send_transport, - bool is_svc, const RtpSenderObservers& observers, RtcEventLog* event_log, std::unique_ptr fec_controller, const RtpSenderFrameEncryptionConfig& frame_encryption_config) { video_rtp_senders_.push_back(absl::make_unique( clock_, suspended_ssrcs, states, rtp_config, rtcp_report_interval_ms, - send_transport, is_svc, observers, + send_transport, observers, // TODO(holmer): Remove this circular dependency by injecting // the parts of RtpTransportControllerSendInterface that are really used. this, event_log, &retransmission_rate_limiter_, std::move(fec_controller), diff --git a/call/rtp_transport_controller_send.h b/call/rtp_transport_controller_send.h index 44135e4c71..c8a9f2cd4e 100644 --- a/call/rtp_transport_controller_send.h +++ b/call/rtp_transport_controller_send.h @@ -60,7 +60,6 @@ class RtpTransportControllerSend final const RtpConfig& rtp_config, int rtcp_report_interval_ms, Transport* send_transport, - bool is_svc, const RtpSenderObservers& observers, RtcEventLog* event_log, std::unique_ptr fec_controller, diff --git a/call/rtp_transport_controller_send_interface.h b/call/rtp_transport_controller_send_interface.h index 32862d8dd4..d56403542a 100644 --- a/call/rtp_transport_controller_send_interface.h +++ b/call/rtp_transport_controller_send_interface.h @@ -104,7 +104,6 @@ class RtpTransportControllerSendInterface { const RtpConfig& rtp_config, int rtcp_report_interval_ms, Transport* send_transport, - bool is_svc, const RtpSenderObservers& observers, RtcEventLog* event_log, std::unique_ptr fec_controller, diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index 748196d06d..87edf10a86 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -212,7 +212,6 @@ RtpVideoSender::RtpVideoSender( const RtpConfig& rtp_config, int rtcp_report_interval_ms, Transport* send_transport, - bool is_svc, const RtpSenderObservers& observers, RtpTransportControllerSendInterface* transport, RtcEventLog* event_log, @@ -254,8 +253,7 @@ RtpVideoSender::RtpVideoSender( overhead_bytes_per_packet_(0), encoder_target_rate_bps_(0), frame_counts_(rtp_config.ssrcs.size()), - frame_count_observer_(observers.frame_count_observer), - is_svc_(is_svc) { + frame_count_observer_(observers.frame_count_observer) { RTC_DCHECK_EQ(rtp_config.ssrcs.size(), rtp_streams_.size()); module_process_thread_checker_.DetachFromThread(); // SSRCs are assumed to be sorted in the same order as |rtp_modules|. @@ -450,7 +448,7 @@ void RtpVideoSender::OnBitrateAllocationUpdated( const VideoBitrateAllocation& bitrate) { rtc::CritScope lock(&crit_); if (IsActive()) { - if (rtp_streams_.size() == 1 || is_svc_) { + if (rtp_streams_.size() == 1) { // If spatial scalability is enabled, it is covered by a single stream. rtp_streams_[0].rtp_rtcp->SetVideoBitrateAllocation(bitrate); } else { diff --git a/call/rtp_video_sender.h b/call/rtp_video_sender.h index 5c0094f34e..d50cb7c2d7 100644 --- a/call/rtp_video_sender.h +++ b/call/rtp_video_sender.h @@ -76,7 +76,6 @@ class RtpVideoSender : public RtpVideoSenderInterface, const RtpConfig& rtp_config, int rtcp_report_interval_ms, Transport* send_transport, - bool is_svc, const RtpSenderObservers& observers, RtpTransportControllerSendInterface* transport, RtcEventLog* event_log, @@ -191,8 +190,6 @@ class RtpVideoSender : public RtpVideoSenderInterface, std::vector frame_counts_ RTC_GUARDED_BY(crit_); FrameCountObserver* const frame_count_observer_; - const bool is_svc_; - RTC_DISALLOW_COPY_AND_ASSIGN(RtpVideoSender); }; diff --git a/call/rtp_video_sender_unittest.cc b/call/rtp_video_sender_unittest.cc index e51e72af86..1a1dd9bdde 100644 --- a/call/rtp_video_sender_unittest.cc +++ b/call/rtp_video_sender_unittest.cc @@ -99,7 +99,7 @@ class RtpVideoSenderTestFixture { std::map suspended_ssrcs; router_ = absl::make_unique( &clock_, suspended_ssrcs, suspended_payload_states, config_.rtp, - config_.rtcp_report_interval_ms, &transport_, config_.is_svc, + config_.rtcp_report_interval_ms, &transport_, CreateObservers(&call_stats_, &encoder_feedback_, &stats_proxy_, &stats_proxy_, &stats_proxy_, frame_count_observer, &stats_proxy_, &stats_proxy_, &send_delay_stats_), diff --git a/call/test/mock_rtp_transport_controller_send.h b/call/test/mock_rtp_transport_controller_send.h index af44008352..b5a0a856a6 100644 --- a/call/test/mock_rtp_transport_controller_send.h +++ b/call/test/mock_rtp_transport_controller_send.h @@ -32,14 +32,13 @@ namespace webrtc { class MockRtpTransportControllerSend : public RtpTransportControllerSendInterface { public: - MOCK_METHOD10( + MOCK_METHOD9( CreateRtpVideoSender, RtpVideoSenderInterface*(std::map, const std::map&, const RtpConfig&, int rtcp_report_interval_ms, Transport*, - bool, const RtpSenderObservers&, RtcEventLog*, std::unique_ptr, diff --git a/call/video_send_stream.h b/call/video_send_stream.h index e4f96d7610..5daec19449 100644 --- a/call/video_send_stream.h +++ b/call/video_send_stream.h @@ -151,10 +151,6 @@ class VideoSendStream { // Per PeerConnection cryptography options. CryptoOptions crypto_options; - // Forces spatial scalability to be implemented via spatial layers - // instead of simulcast. - bool is_svc; - private: // Access to the copy constructor is private to force use of the Copy() // method for those exceptional cases where we do use it. diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 7b75840bb2..acbc36f701 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -2302,7 +2302,6 @@ void WebRtcVideoChannel::WebRtcVideoSendStream::RecreateWebRtcStream() { "payload type the set codec. Ignoring RTX."; config.rtp.rtx.ssrcs.clear(); } - config.is_svc = parameters_.encoder_config.number_of_streams == 1; stream_ = call_->CreateVideoSendStream(std::move(config), parameters_.encoder_config.Copy()); diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc index b8cf006c87..23c703e659 100644 --- a/video/video_quality_test.cc +++ b/video/video_quality_test.cc @@ -762,9 +762,6 @@ void VideoQualityTest::SetupVideo(Transport* send_transport, video_send_configs_[video_idx].suspend_below_min_bitrate = params_.video[video_idx].suspend_below_min_bitrate; - video_send_configs_[video_idx].is_svc = - params_.ss[video_idx].streams.size() == 1; - video_encoder_configs_[video_idx].number_of_streams = params_.ss[video_idx].streams.size(); video_encoder_configs_[video_idx].max_bitrate_bps = 0; diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc index 33ee854271..ae905df95a 100644 --- a/video/video_send_stream_impl.cc +++ b/video/video_send_stream_impl.cc @@ -236,7 +236,6 @@ VideoSendStreamImpl::VideoSendStreamImpl( config_->rtp, config_->rtcp_report_interval_ms, config_->send_transport, - config_->is_svc, CreateObservers(call_stats, &encoder_feedback_, stats_proxy_, diff --git a/video/video_send_stream_impl_unittest.cc b/video/video_send_stream_impl_unittest.cc index 47a1e6656a..e8eab842b6 100644 --- a/video/video_send_stream_impl_unittest.cc +++ b/video/video_send_stream_impl_unittest.cc @@ -96,7 +96,7 @@ class VideoSendStreamImplTest : public ::testing::Test { EXPECT_CALL(transport_controller_, packet_router()) .WillRepeatedly(Return(&packet_router_)); EXPECT_CALL(transport_controller_, - CreateRtpVideoSender(_, _, _, _, _, _, _, _, _, _)) + CreateRtpVideoSender(_, _, _, _, _, _, _, _, _)) .WillRepeatedly(Return(&rtp_video_sender_)); EXPECT_CALL(rtp_video_sender_, SetActive(_)) .WillRepeatedly(testing::Invoke(