From 498ee8e816a9f900f0904319d899cecde8d62b14 Mon Sep 17 00:00:00 2001 From: danilchap Date: Wed, 8 Feb 2017 05:24:31 -0800 Subject: [PATCH] Remove repeat flag from SendRTCP It is always false BUG=webrtc:5565 Review-Url: https://codereview.webrtc.org/2684023002 Cr-Commit-Position: refs/heads/master@{#16491} --- webrtc/modules/rtp_rtcp/source/rtcp_sender.cc | 12 +++-------- webrtc/modules/rtp_rtcp/source/rtcp_sender.h | 2 -- .../rtp_rtcp/source/rtcp_sender_unittest.cc | 20 +++---------------- .../modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 9 +++------ 4 files changed, 9 insertions(+), 34 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc index ff535ea4c6..423694f869 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc @@ -128,20 +128,17 @@ class RTCPSender::RtcpContext { RtcpContext(const FeedbackState& feedback_state, int32_t nack_size, const uint16_t* nack_list, - bool repeat, uint64_t picture_id, NtpTime now) : feedback_state_(feedback_state), nack_size_(nack_size), nack_list_(nack_list), - repeat_(repeat), picture_id_(picture_id), now_(now) {} const FeedbackState& feedback_state_; const int32_t nack_size_; const uint16_t* nack_list_; - const bool repeat_; const uint64_t picture_id_; const NtpTime now_; }; @@ -502,8 +499,7 @@ std::unique_ptr RTCPSender::BuildPLI(const RtcpContext& ctx) { } std::unique_ptr RTCPSender::BuildFIR(const RtcpContext& ctx) { - if (!ctx.repeat_) - ++sequence_number_fir_; // Do not increase if repetition. + ++sequence_number_fir_; rtcp::Fir* fir = new rtcp::Fir(); fir->SetSenderSsrc(ssrc_); @@ -739,11 +735,10 @@ int32_t RTCPSender::SendRTCP(const FeedbackState& feedback_state, RTCPPacketType packetType, int32_t nack_size, const uint16_t* nack_list, - bool repeat, uint64_t pictureID) { return SendCompoundRTCP( feedback_state, std::set(&packetType, &packetType + 1), - nack_size, nack_list, repeat, pictureID); + nack_size, nack_list, pictureID); } int32_t RTCPSender::SendCompoundRTCP( @@ -751,7 +746,6 @@ int32_t RTCPSender::SendCompoundRTCP( const std::set& packet_types, int32_t nack_size, const uint16_t* nack_list, - bool repeat, uint64_t pictureID) { PacketContainer container(transport_, event_log_); { @@ -784,7 +778,7 @@ int32_t RTCPSender::SendCompoundRTCP( packet_type_counter_.first_packet_time_ms = clock_->TimeInMilliseconds(); // We need to send our NTP even if we haven't received any reports. - RtcpContext context(feedback_state, nack_size, nack_list, repeat, pictureID, + RtcpContext context(feedback_state, nack_size, nack_list, pictureID, NtpTime(*clock_)); PrepareReport(feedback_state); diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.h b/webrtc/modules/rtp_rtcp/source/rtcp_sender.h index 96a49a72e0..82b6adbd68 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.h @@ -113,14 +113,12 @@ class RTCPSender { RTCPPacketType packetType, int32_t nackSize = 0, const uint16_t* nackList = 0, - bool repeat = false, uint64_t pictureID = 0); int32_t SendCompoundRTCP(const FeedbackState& feedback_state, const std::set& packetTypes, int32_t nackSize = 0, const uint16_t* nackList = 0, - bool repeat = false, uint64_t pictureID = 0); bool REMB() const; diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index 8346ad4592..0aa6361e26 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc @@ -454,7 +454,7 @@ TEST_F(RtcpSenderTest, SetInvalidApplicationSpecificData) { 0, 0, kData, kInvalidDataLength)); // Should by multiple of 4. } -TEST_F(RtcpSenderTest, SendFirNonRepeat) { +TEST_F(RtcpSenderTest, SendFir) { rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize); EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpFir)); EXPECT_EQ(1, parser()->fir()->num_packets()); @@ -462,25 +462,11 @@ TEST_F(RtcpSenderTest, SendFirNonRepeat) { EXPECT_EQ(1U, parser()->fir()->requests().size()); EXPECT_EQ(kRemoteSsrc, parser()->fir()->requests()[0].ssrc); uint8_t seq = parser()->fir()->requests()[0].seq_nr; - // Sends non-repeat FIR as default. EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpFir)); EXPECT_EQ(2, parser()->fir()->num_packets()); EXPECT_EQ(seq + 1, parser()->fir()->requests()[0].seq_nr); } -TEST_F(RtcpSenderTest, SendFirRepeat) { - rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize); - EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpFir)); - EXPECT_EQ(1, parser()->fir()->num_packets()); - EXPECT_EQ(1U, parser()->fir()->requests().size()); - uint8_t seq = parser()->fir()->requests()[0].seq_nr; - const bool kRepeat = true; - EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpFir, 0, nullptr, - kRepeat)); - EXPECT_EQ(2, parser()->fir()->num_packets()); - EXPECT_EQ(seq, parser()->fir()->requests()[0].seq_nr); -} - TEST_F(RtcpSenderTest, SendPli) { rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize); EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpPli)); @@ -496,7 +482,7 @@ TEST_F(RtcpSenderTest, SendRpsi) { RTCPSender::FeedbackState feedback_state = rtp_rtcp_impl_->GetFeedbackState(); feedback_state.send_payload_type = kPayloadType; EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state, kRtcpRpsi, 0, nullptr, - false, kPictureId)); + kPictureId)); EXPECT_EQ(1, parser()->rpsi()->num_packets()); EXPECT_EQ(kPayloadType, parser()->rpsi()->payload_type()); EXPECT_EQ(kPictureId, parser()->rpsi()->picture_id()); @@ -508,7 +494,7 @@ TEST_F(RtcpSenderTest, SendSli) { const uint8_t kPictureId = 60; rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize); EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpSli, 0, nullptr, - false, kPictureId)); + kPictureId)); EXPECT_EQ(1, parser()->sli()->num_packets()); EXPECT_EQ(kSenderSsrc, parser()->sli()->sender_ssrc()); EXPECT_EQ(kRemoteSsrc, parser()->sli()->media_ssrc()); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index d1f70ee6f0..50885f457f 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -773,10 +773,8 @@ int32_t ModuleRtpRtcpImpl::RequestKeyFrame() { return -1; } -int32_t ModuleRtpRtcpImpl::SendRTCPSliceLossIndication( - const uint8_t picture_id) { - return rtcp_sender_.SendRTCP( - GetFeedbackState(), kRtcpSli, 0, 0, false, picture_id); +int32_t ModuleRtpRtcpImpl::SendRTCPSliceLossIndication(uint8_t picture_id) { + return rtcp_sender_.SendRTCP(GetFeedbackState(), kRtcpSli, 0, 0, picture_id); } void ModuleRtpRtcpImpl::SetUlpfecConfig(int red_payload_type, @@ -830,8 +828,7 @@ void ModuleRtpRtcpImpl::OnRequestSendReport() { int32_t ModuleRtpRtcpImpl::SendRTCPReferencePictureSelection( const uint64_t picture_id) { - return rtcp_sender_.SendRTCP( - GetFeedbackState(), kRtcpRpsi, 0, 0, false, picture_id); + return rtcp_sender_.SendRTCP(GetFeedbackState(), kRtcpRpsi, 0, 0, picture_id); } void ModuleRtpRtcpImpl::OnReceivedNack(