From 2ddf98d8941098c5d7b7d2147ed3f3c91fc7f225 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Wed, 22 Nov 2017 14:00:41 +0100 Subject: [PATCH] Add RequestKeyFrame with Fir to RtcpTransceiver Bug: webrtc:8239 Change-Id: If094d434a7be20cdff5c80447322d68a4a7a4580 Reviewed-on: https://webrtc-review.googlesource.com/22961 Commit-Queue: Danil Chapovalov Reviewed-by: Niels Moller Cr-Commit-Position: refs/heads/master@{#20833} --- modules/rtp_rtcp/source/rtcp_transceiver.cc | 22 ++++- modules/rtp_rtcp/source/rtcp_transceiver.h | 6 +- .../rtp_rtcp/source/rtcp_transceiver_impl.cc | 82 +++++++++++++------ .../rtp_rtcp/source/rtcp_transceiver_impl.h | 18 ++-- .../source/rtcp_transceiver_impl_unittest.cc | 59 ++++++++++++- 5 files changed, 145 insertions(+), 42 deletions(-) diff --git a/modules/rtp_rtcp/source/rtcp_transceiver.cc b/modules/rtp_rtcp/source/rtcp_transceiver.cc index 7c668b91ae..7feca3fb90 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver.cc @@ -90,18 +90,32 @@ void RtcpTransceiver::UnsetRemb() { }); } -void RtcpTransceiver::RequestKeyFrame(std::vector ssrcs) { +void RtcpTransceiver::SendPictureLossIndication(std::vector ssrcs) { // TODO(danilchap): Replace with lambda with move capture when available. - struct RequestKeyFrameClosure { + struct Closure { void operator()() { if (ptr) - ptr->RequestKeyFrame(ssrcs); + ptr->SendPictureLossIndication(ssrcs); } rtc::WeakPtr ptr; std::vector ssrcs; }; - task_queue_->PostTask(RequestKeyFrameClosure{ptr_, std::move(ssrcs)}); + task_queue_->PostTask(Closure{ptr_, std::move(ssrcs)}); +} + +void RtcpTransceiver::SendFullIntraRequest(std::vector ssrcs) { + // TODO(danilchap): Replace with lambda with move capture when available. + struct Closure { + void operator()() { + if (ptr) + ptr->SendFullIntraRequest(ssrcs); + } + + rtc::WeakPtr ptr; + std::vector ssrcs; + }; + task_queue_->PostTask(Closure{ptr_, std::move(ssrcs)}); } } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtcp_transceiver.h b/modules/rtp_rtcp/source/rtcp_transceiver.h index 4e571ee85a..9d695451a1 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver.h +++ b/modules/rtp_rtcp/source/rtcp_transceiver.h @@ -44,7 +44,11 @@ class RtcpTransceiver { // Stops sending REMB in following compound packets. void UnsetRemb(); - void RequestKeyFrame(std::vector ssrcs); + // Request new key frame. + // using PLI, https://tools.ietf.org/html/rfc4585#section-6.3.1.1 + void SendPictureLossIndication(std::vector ssrcs); + // using FIR, https://tools.ietf.org/html/rfc5104#section-4.3.1.2 + void SendFullIntraRequest(std::vector ssrcs); private: rtc::TaskQueue* const task_queue_; diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc index 52bc4890c9..b870dd103f 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc @@ -17,6 +17,7 @@ #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/source/rtcp_packet.h" #include "modules/rtp_rtcp/source/rtcp_packet/common_header.h" +#include "modules/rtp_rtcp/source/rtcp_packet/fir.h" #include "modules/rtp_rtcp/source/rtcp_packet/pli.h" #include "modules/rtp_rtcp/source/rtcp_packet/receiver_report.h" #include "modules/rtp_rtcp/source/rtcp_packet/report_block.h" @@ -29,6 +30,19 @@ #include "rtc_base/timeutils.h" namespace webrtc { +namespace { + +struct SenderReportTimes { + int64_t local_received_time_us; + NtpTime remote_sent_time; +}; + +} // namespace + +struct RtcpTransceiverImpl::RemoteSenderState { + uint8_t fir_sequence_number = 0; + rtc::Optional last_received_sender_report; +}; // Helper to put several RTCP packets into lower layer datagram composing // Compound or Reduced-Size RTCP packet, as defined by RFC 5506 section 2. @@ -116,25 +130,30 @@ void RtcpTransceiverImpl::UnsetRemb() { remb_.reset(); } -void RtcpTransceiverImpl::RequestKeyFrame( +void RtcpTransceiverImpl::SendPictureLossIndication( rtc::ArrayView ssrcs) { RTC_DCHECK(!ssrcs.empty()); - const uint32_t sender_ssrc = config_.feedback_ssrc; - PacketSender sender(config_.outgoing_transport, config_.max_packet_size); - if (config_.rtcp_mode == RtcpMode::kCompound) - CreateCompoundPacket(&sender); + SendImmediateFeedback([this, ssrcs](PacketSender* sender) { + for (uint32_t media_ssrc : ssrcs) { + rtcp::Pli pli; + pli.SetSenderSsrc(config_.feedback_ssrc); + pli.SetMediaSsrc(media_ssrc); + sender->AppendPacket(pli); + } + }); +} - for (uint32_t media_ssrc : ssrcs) { - rtcp::Pli pli; - pli.SetSenderSsrc(sender_ssrc); - pli.SetMediaSsrc(media_ssrc); - sender.AppendPacket(pli); - } - - sender.Send(); - - if (config_.rtcp_mode == RtcpMode::kCompound) - ReschedulePeriodicCompoundPackets(); +void RtcpTransceiverImpl::SendFullIntraRequest( + rtc::ArrayView ssrcs) { + RTC_DCHECK(!ssrcs.empty()); + SendImmediateFeedback([this, ssrcs](PacketSender* sender) { + rtcp::Fir fir; + fir.SetSenderSsrc(config_.feedback_ssrc); + for (uint32_t media_ssrc : ssrcs) + fir.AddRequestTo(media_ssrc, + remote_senders_[media_ssrc].fir_sequence_number++); + sender->AppendPacket(fir); + }); } void RtcpTransceiverImpl::HandleReceivedPacket( @@ -145,10 +164,12 @@ void RtcpTransceiverImpl::HandleReceivedPacket( rtcp::SenderReport sender_report; if (!sender_report.Parse(rtcp_packet_header)) return; - SenderReportTimes& last = - last_received_sender_reports_[sender_report.sender_ssrc()]; - last.local_received_time_us = now_us; - last.remote_sent_time = sender_report.ntp(); + rtc::Optional& last = + remote_senders_[sender_report.sender_ssrc()] + .last_received_sender_report; + last.emplace(); + last->local_received_time_us = now_us; + last->remote_sent_time = sender_report.ntp(); break; } } @@ -220,6 +241,20 @@ void RtcpTransceiverImpl::SendPeriodicCompoundPacket() { sender.Send(); } +void RtcpTransceiverImpl::SendImmediateFeedback( + rtc::FunctionView append_feedback) { + PacketSender sender(config_.outgoing_transport, config_.max_packet_size); + if (config_.rtcp_mode == RtcpMode::kCompound) + CreateCompoundPacket(&sender); + + append_feedback(&sender); + + sender.Send(); + + if (config_.rtcp_mode == RtcpMode::kCompound) + ReschedulePeriodicCompoundPackets(); +} + std::vector RtcpTransceiverImpl::CreateReportBlocks() { if (!config_.receive_statistics) return {}; @@ -229,10 +264,11 @@ std::vector RtcpTransceiverImpl::CreateReportBlocks() { config_.receive_statistics->RtcpReportBlocks( rtcp::ReceiverReport::kMaxNumberOfReportBlocks); for (rtcp::ReportBlock& report_block : report_blocks) { - auto it = last_received_sender_reports_.find(report_block.source_ssrc()); - if (it == last_received_sender_reports_.end()) + auto it = remote_senders_.find(report_block.source_ssrc()); + if (it == remote_senders_.end() || !it->second.last_received_sender_report) continue; - const SenderReportTimes& last_sender_report = it->second; + const SenderReportTimes& last_sender_report = + *it->second.last_received_sender_report; report_block.SetLastSr(CompactNtp(last_sender_report.remote_sent_time)); report_block.SetDelayLastSr(SaturatedUsToCompactNtp( rtc::TimeMicros() - last_sender_report.local_received_time_us)); diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.h b/modules/rtp_rtcp/source/rtcp_transceiver_impl.h index aed850b07a..a89f3e106a 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.h +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.h @@ -23,6 +23,7 @@ #include "modules/rtp_rtcp/source/rtcp_packet/report_block.h" #include "modules/rtp_rtcp/source/rtcp_transceiver_config.h" #include "rtc_base/constructormagic.h" +#include "rtc_base/function_view.h" #include "rtc_base/weak_ptr.h" #include "system_wrappers/include/ntp_time.h" @@ -36,26 +37,19 @@ class RtcpTransceiverImpl { explicit RtcpTransceiverImpl(const RtcpTransceiverConfig& config); ~RtcpTransceiverImpl(); - // Handles incoming rtcp packets. void ReceivePacket(rtc::ArrayView packet, int64_t now_us); - // Sends RTCP packets starting with a sender or receiver report. void SendCompoundPacket(); - // (REMB) Receiver Estimated Max Bitrate. - // Includes REMB in following compound packets. void SetRemb(int bitrate_bps, std::vector ssrcs); - // Stops sending REMB in following compound packets. void UnsetRemb(); - void RequestKeyFrame(rtc::ArrayView ssrcs); + void SendPictureLossIndication(rtc::ArrayView ssrcs); + void SendFullIntraRequest(rtc::ArrayView ssrcs); private: class PacketSender; - struct SenderReportTimes { - int64_t local_received_time_us; - NtpTime remote_sent_time; - }; + struct RemoteSenderState; void HandleReceivedPacket(const rtcp::CommonHeader& rtcp_packet_header, int64_t now_us); @@ -67,13 +61,15 @@ class RtcpTransceiverImpl { void CreateCompoundPacket(PacketSender* sender); // Sends RTCP packets. void SendPeriodicCompoundPacket(); + void SendImmediateFeedback( + rtc::FunctionView append_feedback); // Generate Report Blocks to be send in Sender or Receiver Report. std::vector CreateReportBlocks(); const RtcpTransceiverConfig config_; rtc::Optional remb_; - std::map last_received_sender_reports_; + std::map remote_senders_; rtc::WeakPtrFactory ptr_factory_; RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(RtcpTransceiverImpl); diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc index e5f7a65c7c..c7f7482f8c 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc @@ -460,7 +460,7 @@ TEST(RtcpTransceiverImplTest, RequestKeyFrameWithPictureLossIndication) { EXPECT_CALL(outgoing_transport, SendRtcp(_, _)) .WillOnce(Invoke(&rtcp_parser, &RtcpPacketParser::Parse)); - rtcp_transceiver.RequestKeyFrame(kRemoteSsrcs); + rtcp_transceiver.SendPictureLossIndication(kRemoteSsrcs); // Expect a pli packet per ssrc in the sent single compound packet. EXPECT_EQ(rtcp_parser.pli()->num_packets(), 2); @@ -469,6 +469,59 @@ TEST(RtcpTransceiverImplTest, RequestKeyFrameWithPictureLossIndication) { EXPECT_EQ(rtcp_parser.pli()->media_ssrc(), kRemoteSsrcs[1]); } +TEST(RtcpTransceiverImplTest, RequestKeyFrameWithFullIntraRequest) { + const uint32_t kSenderSsrc = 1234; + const uint32_t kRemoteSsrcs[] = {4321, 5321}; + MockTransport outgoing_transport; + RtcpTransceiverConfig config; + config.feedback_ssrc = kSenderSsrc; + config.schedule_periodic_compound_packets = false; + config.outgoing_transport = &outgoing_transport; + RtcpTransceiverImpl rtcp_transceiver(config); + RtcpPacketParser rtcp_parser; + EXPECT_CALL(outgoing_transport, SendRtcp(_, _)) + .WillOnce(Invoke(&rtcp_parser, &RtcpPacketParser::Parse)); + + rtcp_transceiver.SendFullIntraRequest(kRemoteSsrcs); + + EXPECT_EQ(rtcp_parser.fir()->num_packets(), 1); + EXPECT_EQ(rtcp_parser.fir()->sender_ssrc(), kSenderSsrc); + EXPECT_EQ(rtcp_parser.fir()->requests()[0].ssrc, kRemoteSsrcs[0]); + EXPECT_EQ(rtcp_parser.fir()->requests()[1].ssrc, kRemoteSsrcs[1]); +} + +TEST(RtcpTransceiverImplTest, RequestKeyFrameWithFirIncreaseSeqNoPerSsrc) { + MockTransport outgoing_transport; + RtcpTransceiverConfig config; + config.schedule_periodic_compound_packets = false; + config.outgoing_transport = &outgoing_transport; + RtcpTransceiverImpl rtcp_transceiver(config); + RtcpPacketParser rtcp_parser; + EXPECT_CALL(outgoing_transport, SendRtcp(_, _)) + .WillRepeatedly(Invoke(&rtcp_parser, &RtcpPacketParser::Parse)); + + const uint32_t kBothRemoteSsrcs[] = {4321, 5321}; + const uint32_t kOneRemoteSsrc[] = {4321}; + + rtcp_transceiver.SendFullIntraRequest(kBothRemoteSsrcs); + ASSERT_EQ(rtcp_parser.fir()->requests()[0].ssrc, kBothRemoteSsrcs[0]); + uint8_t fir_sequence_number0 = rtcp_parser.fir()->requests()[0].seq_nr; + ASSERT_EQ(rtcp_parser.fir()->requests()[1].ssrc, kBothRemoteSsrcs[1]); + uint8_t fir_sequence_number1 = rtcp_parser.fir()->requests()[1].seq_nr; + + rtcp_transceiver.SendFullIntraRequest(kOneRemoteSsrc); + ASSERT_EQ(rtcp_parser.fir()->requests().size(), 1u); + ASSERT_EQ(rtcp_parser.fir()->requests()[0].ssrc, kBothRemoteSsrcs[0]); + EXPECT_EQ(rtcp_parser.fir()->requests()[0].seq_nr, fir_sequence_number0 + 1); + + rtcp_transceiver.SendFullIntraRequest(kBothRemoteSsrcs); + ASSERT_EQ(rtcp_parser.fir()->requests().size(), 2u); + ASSERT_EQ(rtcp_parser.fir()->requests()[0].ssrc, kBothRemoteSsrcs[0]); + EXPECT_EQ(rtcp_parser.fir()->requests()[0].seq_nr, fir_sequence_number0 + 2); + ASSERT_EQ(rtcp_parser.fir()->requests()[1].ssrc, kBothRemoteSsrcs[1]); + EXPECT_EQ(rtcp_parser.fir()->requests()[1].seq_nr, fir_sequence_number1 + 1); +} + TEST(RtcpTransceiverImplTest, KeyFrameRequestCreatesCompoundPacket) { const uint32_t kRemoteSsrcs[] = {4321}; MockTransport outgoing_transport; @@ -484,7 +537,7 @@ TEST(RtcpTransceiverImplTest, KeyFrameRequestCreatesCompoundPacket) { EXPECT_CALL(outgoing_transport, SendRtcp(_, _)) .WillOnce(Invoke(&rtcp_parser, &RtcpPacketParser::Parse)); - rtcp_transceiver.RequestKeyFrame(kRemoteSsrcs); + rtcp_transceiver.SendFullIntraRequest(kRemoteSsrcs); // Test sent packet is compound by expecting presense of receiver report. EXPECT_EQ(rtcp_parser.receiver_report()->num_packets(), 1); @@ -506,7 +559,7 @@ TEST(RtcpTransceiverImplTest, KeyFrameRequestCreatesReducedSizePacket) { EXPECT_CALL(outgoing_transport, SendRtcp(_, _)) .WillOnce(Invoke(&rtcp_parser, &RtcpPacketParser::Parse)); - rtcp_transceiver.RequestKeyFrame(kRemoteSsrcs); + rtcp_transceiver.SendFullIntraRequest(kRemoteSsrcs); // Test sent packet is reduced size by expecting absense of receiver report. EXPECT_EQ(rtcp_parser.receiver_report()->num_packets(), 0);