From 601504c5cc82a132b5eea8ee0cc7068b08614625 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Wed, 12 Sep 2018 11:27:43 +0200 Subject: [PATCH] in RtcpTransceiver remove workaround for old bug in RtcpReceiver the bug in RtcpReceiver was fixed Jan 30, i.e. 10.5 month ago Bug: webrtc:8805 Change-Id: I5f5f00fba5e984ede906c5dbbe841ee5f4992e09 Reviewed-on: https://webrtc-review.googlesource.com/c/99822 Commit-Queue: Danil Chapovalov Reviewed-by: Niels Moller Cr-Commit-Position: refs/heads/master@{#25683} --- .../rtp_rtcp/source/rtcp_transceiver_config.h | 5 -- .../rtp_rtcp/source/rtcp_transceiver_impl.cc | 6 --- .../source/rtcp_transceiver_impl_unittest.cc | 47 ------------------- 3 files changed, 58 deletions(-) diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_config.h b/modules/rtp_rtcp/source/rtcp_transceiver_config.h index 0da18e2cc1..01330d0bc7 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_config.h +++ b/modules/rtp_rtcp/source/rtcp_transceiver_config.h @@ -97,11 +97,6 @@ struct RtcpTransceiverConfig { // Estimate RTT as non-sender as described in // https://tools.ietf.org/html/rfc3611#section-4.4 and #section-4.5 bool non_sender_rtt_measurement = false; - // Copies LastSR/DelaySinceLastSR for previous report block to avoid - // triggering bug in older version of RtcpReceiver. - // TODO(bugs.webrtc.org/8805): Change to false by default then remove when - // all major webrtc clients updated with the fix in RtcpReceiver. - bool avoid_zero_last_sr_in_last_report_block = true; }; } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc index 2c6c3acc39..97c2ac018b 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc @@ -421,12 +421,6 @@ std::vector RtcpTransceiverImpl::CreateReportBlocks( auto it = remote_senders_.find(report_block.source_ssrc()); if (it == remote_senders_.end() || !it->second.last_received_sender_report) { - if (config_.avoid_zero_last_sr_in_last_report_block && last_sr != 0) { - // Simulate behaviour of the RtcpSender to avoid hitting bug in - // RtcpReceiver. - report_block.SetLastSr(last_sr); - report_block.SetDelayLastSr(last_delay); - } continue; } const SenderReportTimes& last_sender_report = diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc index 1e684a3839..e86d67f579 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc @@ -670,7 +670,6 @@ TEST(RtcpTransceiverImplTest, RtcpTransceiverConfig config; config.schedule_periodic_compound_packets = false; - config.avoid_zero_last_sr_in_last_report_block = false; RtcpPacketParser rtcp_parser; RtcpParserTransport transport(&rtcp_parser); config.outgoing_transport = &transport; @@ -702,52 +701,6 @@ TEST(RtcpTransceiverImplTest, EXPECT_EQ(report_blocks[1].last_sr(), 0u); } -TEST(RtcpTransceiverImplTest, AvoidLastReportBlockToHaveZeroLastSrField) { - const uint32_t kRemoteSsrc1 = 54321; - const uint32_t kRemoteSsrc2 = 54323; - MockReceiveStatisticsProvider receive_statistics; - std::vector statistics_report_blocks(2); - statistics_report_blocks[0].SetMediaSsrc(kRemoteSsrc1); - statistics_report_blocks[1].SetMediaSsrc(kRemoteSsrc2); - ON_CALL(receive_statistics, RtcpReportBlocks(_)) - .WillByDefault(Return(statistics_report_blocks)); - - RtcpTransceiverConfig config; - config.schedule_periodic_compound_packets = false; - config.avoid_zero_last_sr_in_last_report_block = true; - RtcpPacketParser rtcp_parser; - RtcpParserTransport transport(&rtcp_parser); - config.outgoing_transport = &transport; - config.receive_statistics = &receive_statistics; - RtcpTransceiverImpl rtcp_transceiver(config); - - const NtpTime kRemoteNtp(0x9876543211); - // Receive SenderReport for RemoteSsrc1, but no report for RemoteSsrc2. - SenderReport sr; - sr.SetSenderSsrc(kRemoteSsrc1); - sr.SetNtp(kRemoteNtp); - auto raw_packet = sr.Build(); - rtcp_transceiver.ReceivePacket(raw_packet, /*now_us=*/0); - - // Trigger sending ReceiverReport. - rtcp_transceiver.SendCompoundPacket(); - - EXPECT_GT(rtcp_parser.receiver_report()->num_packets(), 0); - const auto& report_blocks = rtcp_parser.receiver_report()->report_blocks(); - ASSERT_EQ(report_blocks.size(), 2u); - // RtcpTransceiverImpl doesn't guarantee order of the report blocks - // match result of ReceiveStatisticsProvider::RtcpReportBlocks callback, - // but for simplicity of the test asume it is the same. - ASSERT_EQ(report_blocks[0].source_ssrc(), kRemoteSsrc1); - EXPECT_NE(report_blocks[0].last_sr(), 0u); - - ASSERT_EQ(report_blocks[1].source_ssrc(), kRemoteSsrc2); - // No Sender Report for kRemoteSsrc2, use same LastSR as for kRemoteSsrc1 - EXPECT_EQ(report_blocks[1].last_sr(), report_blocks[0].last_sr()); - EXPECT_EQ(report_blocks[1].delay_since_last_sr(), - report_blocks[0].delay_since_last_sr()); -} - TEST(RtcpTransceiverImplTest, WhenSendsReceiverReportCalculatesDelaySinceLastSenderReport) { const uint32_t kRemoteSsrc1 = 4321;