From cd6ae6652f075d9699c9f22795d15742ce1a7205 Mon Sep 17 00:00:00 2001 From: Fredrik Solenberg Date: Wed, 11 May 2016 13:05:05 +0200 Subject: [PATCH] Removing some old code which looked like it had to do with NACK handling but in reality did nothing. BUG=webrtc:5762, webrtc:4690 R=stefan@webrtc.org TBR=mflodman Review URL: https://codereview.webrtc.org/1946183002 . Cr-Commit-Position: refs/heads/master@{#12682} --- webrtc/modules/rtp_rtcp/include/rtp_receiver.h | 6 ------ webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h | 2 -- webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h | 4 ---- .../modules/rtp_rtcp/source/nack_rtx_unittest.cc | 1 - .../modules/rtp_rtcp/source/rtp_receiver_impl.cc | 14 +------------- webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.h | 7 ------- webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc | 4 ---- .../rtp_rtcp/test/testAPI/test_api_video.cc | 1 - webrtc/modules/video_coding/test/rtp_player.cc | 1 - webrtc/video/rtp_stream_receiver.cc | 5 +---- webrtc/voice_engine/channel.cc | 1 - 11 files changed, 2 insertions(+), 44 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/include/rtp_receiver.h b/webrtc/modules/rtp_rtcp/include/rtp_receiver.h index f393e41eab..9db1c63da7 100644 --- a/webrtc/modules/rtp_rtcp/include/rtp_receiver.h +++ b/webrtc/modules/rtp_rtcp/include/rtp_receiver.h @@ -75,12 +75,6 @@ class RtpReceiver { PayloadUnion payload_specific, bool in_order) = 0; - // Returns the currently configured NACK method. - virtual NACKMethod NACK() const = 0; - - // Turn negative acknowledgement (NACK) requests on/off. - virtual void SetNACKStatus(const NACKMethod method) = 0; - // Gets the last received timestamp. Returns true if a packet has been // received, false otherwise. virtual bool Timestamp(uint32_t* timestamp) const = 0; diff --git a/webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h index 9acef79f38..282483b095 100644 --- a/webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h +++ b/webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h @@ -99,8 +99,6 @@ enum KeyFrameRequestMethod { kKeyFrameReqPliRtcp, kKeyFrameReqFirRtcp }; enum RtpRtcpPacketType { kPacketRtp = 0, kPacketKeepAlive = 1 }; -enum NACKMethod { kNackOff = 0, kNackRtcp = 2 }; - enum RetransmissionMode : uint8_t { kRetransmitOff = 0x0, kRetransmitFECPackets = 0x1, diff --git a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index 4cbb042428..bf5e936921 100644 --- a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -202,10 +202,6 @@ class MockRtpRtcp : public RtpRtcp { MOCK_METHOD1(SetTMMBRStatus, void(const bool enable)); MOCK_METHOD1(OnBandwidthEstimateUpdate, void(uint16_t bandWidthKbit)); - MOCK_CONST_METHOD0(NACK, - NACKMethod()); - MOCK_METHOD2(SetNACKStatus, - int32_t(const NACKMethod method, int oldestSequenceNumberToNack)); MOCK_CONST_METHOD0(SelectiveRetransmissions, int()); MOCK_METHOD1(SetSelectiveRetransmissions, diff --git a/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc b/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc index a73d4edaf3..9a7b9d3f7e 100644 --- a/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc @@ -191,7 +191,6 @@ class RtpRtcpRtxNackTest : public ::testing::Test { rtp_rtcp_module_->SetSSRC(kTestSsrc); rtp_rtcp_module_->SetRTCPStatus(RtcpMode::kCompound); - rtp_receiver_->SetNACKStatus(kNackRtcp); rtp_rtcp_module_->SetStorePacketsStatus(true, 600); EXPECT_EQ(0, rtp_rtcp_module_->SetSendingStatus(true)); rtp_rtcp_module_->SetSequenceNumber(kTestSequenceNumber); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc index 04725ae7fe..190449b3dd 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc @@ -68,8 +68,7 @@ RtpReceiverImpl::RtpReceiverImpl( current_remote_csrc_(), last_received_timestamp_(0), last_received_frame_time_ms_(-1), - last_received_sequence_number_(0), - nack_method_(kNackOff) { + last_received_sequence_number_(0) { assert(incoming_messages_callback); memset(current_remote_csrc_, 0, sizeof(current_remote_csrc_)); @@ -113,17 +112,6 @@ int32_t RtpReceiverImpl::DeRegisterReceivePayload( return rtp_payload_registry_->DeRegisterReceivePayload(payload_type); } -NACKMethod RtpReceiverImpl::NACK() const { - rtc::CritScope lock(&critical_section_rtp_receiver_); - return nack_method_; -} - -// Turn negative acknowledgment requests on/off. -void RtpReceiverImpl::SetNACKStatus(const NACKMethod method) { - rtc::CritScope lock(&critical_section_rtp_receiver_); - nack_method_ = method; -} - uint32_t RtpReceiverImpl::SSRC() const { rtc::CritScope lock(&critical_section_rtp_receiver_); return ssrc_; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.h b/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.h index dca1978159..be659fe874 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.h @@ -48,11 +48,6 @@ class RtpReceiverImpl : public RtpReceiver { PayloadUnion payload_specific, bool in_order) override; - NACKMethod NACK() const override; - - // Turn negative acknowledgement requests on/off. - void SetNACKStatus(const NACKMethod method) override; - // Returns the last received timestamp. bool Timestamp(uint32_t* timestamp) const override; bool LastReceivedTimeMs(int64_t* receive_time_ms) const override; @@ -93,8 +88,6 @@ class RtpReceiverImpl : public RtpReceiver { uint32_t last_received_timestamp_; int64_t last_received_frame_time_ms_; uint16_t last_received_sequence_number_; - - NACKMethod nack_method_; }; } // namespace webrtc #endif // WEBRTC_MODULES_RTP_RTCP_SOURCE_RTP_RECEIVER_IMPL_H_ diff --git a/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc b/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc index ea2d98bc3a..89c9cbebeb 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc @@ -152,10 +152,6 @@ TEST_F(RtpRtcpAPITest, RTCP) { EXPECT_TRUE(module_->TMMBR()); module_->SetTMMBRStatus(false); EXPECT_FALSE(module_->TMMBR()); - - EXPECT_EQ(kNackOff, rtp_receiver_->NACK()); - rtp_receiver_->SetNACKStatus(kNackRtcp); - EXPECT_EQ(kNackRtcp, rtp_receiver_->NACK()); } TEST_F(RtpRtcpAPITest, RtxSender) { diff --git a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_video.cc b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_video.cc index 74daba94c9..d84ff37be7 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_video.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_video.cc @@ -56,7 +56,6 @@ class RtpRtcpVideoTest : public ::testing::Test { video_module_->SetRTCPStatus(RtcpMode::kCompound); video_module_->SetSSRC(test_ssrc_); - rtp_receiver_->SetNACKStatus(kNackRtcp); video_module_->SetStorePacketsStatus(true, 600); EXPECT_EQ(0, video_module_->SetSendingStatus(true)); diff --git a/webrtc/modules/video_coding/test/rtp_player.cc b/webrtc/modules/video_coding/test/rtp_player.cc index 816960cbe0..41cd360d13 100644 --- a/webrtc/modules/video_coding/test/rtp_player.cc +++ b/webrtc/modules/video_coding/test/rtp_player.cc @@ -228,7 +228,6 @@ class SsrcHandlers { return -1; } - handler->rtp_module_->SetNACKStatus(kNackOff); handler->rtp_header_parser_->RegisterRtpHeaderExtension( kRtpExtensionTransmissionTimeOffset, kDefaultTransmissionTimeOffsetExtensionId); diff --git a/webrtc/video/rtp_stream_receiver.cc b/webrtc/video/rtp_stream_receiver.cc index 29bd78152e..98b46831c5 100644 --- a/webrtc/video/rtp_stream_receiver.cc +++ b/webrtc/video/rtp_stream_receiver.cc @@ -132,11 +132,8 @@ RtpStreamReceiver::RtpStreamReceiver( } static const int kMaxPacketAgeToNack = 450; - NACKMethod nack_method = - config.rtp.nack.rtp_history_ms > 0 ? kNackRtcp : kNackOff; - const int max_reordering_threshold = (nack_method == kNackRtcp) + const int max_reordering_threshold = (config.rtp.nack.rtp_history_ms > 0) ? kMaxPacketAgeToNack : kDefaultMaxReorderingThreshold; - rtp_receiver_->SetNACKStatus(nack_method); rtp_receive_statistics_->SetMaxReorderingThreshold(max_reordering_threshold); // TODO(pbos): Support multiple RTX, per video payload. diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc index a6c0c8d050..37b02702dc 100644 --- a/webrtc/voice_engine/channel.cc +++ b/webrtc/voice_engine/channel.cc @@ -2914,7 +2914,6 @@ void Channel::SetNACKStatus(bool enable, int maxNumberOfPackets) { if (!pacing_enabled_) _rtpRtcpModule->SetStorePacketsStatus(enable, maxNumberOfPackets); rtp_receive_statistics_->SetMaxReorderingThreshold(maxNumberOfPackets); - rtp_receiver_->SetNACKStatus(enable ? kNackRtcp : kNackOff); if (enable) audio_coding_->EnableNack(maxNumberOfPackets); else