From a0e9943ca69a826a2b0997361773cb96655e2bd7 Mon Sep 17 00:00:00 2001 From: Elad Alon Date: Fri, 24 May 2019 13:50:56 +0200 Subject: [PATCH] Negotiation of LNTF controls instantiation of RTPSenderVideo::rtp_sequence_number_map_ Bug: webrtc:10662 Change-Id: I9e6b8636d915646c2a822599f5b1735494429ab9 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/138217 Commit-Queue: Elad Alon Reviewed-by: Danil Chapovalov Reviewed-by: Niels Moller Cr-Commit-Position: refs/heads/master@{#28059} --- call/rtp_config.h | 2 +- call/rtp_video_sender.cc | 2 +- modules/rtp_rtcp/source/nack_rtx_unittest.cc | 2 +- .../rtp_rtcp/source/rtp_rtcp_impl_unittest.cc | 2 +- .../rtp_rtcp/source/rtp_sender_unittest.cc | 34 +++++++++---------- modules/rtp_rtcp/source/rtp_sender_video.cc | 14 +++----- modules/rtp_rtcp/source/rtp_sender_video.h | 1 + .../source/rtp_sender_video_unittest.cc | 1 + 8 files changed, 28 insertions(+), 30 deletions(-) diff --git a/call/rtp_config.h b/call/rtp_config.h index 105d28425f..0af48a5d57 100644 --- a/call/rtp_config.h +++ b/call/rtp_config.h @@ -31,7 +31,7 @@ struct RtpPayloadState { struct LntfConfig { std::string ToString() const; - bool enabled{false}; // TODO(bugs.webrtc.org/10662): Consume this flag. + bool enabled{false}; }; // Settings for NACK, see RFC 4585 for details. diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index bd56c33668..59817fb190 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -125,7 +125,7 @@ std::vector CreateRtpStreamSenders( configuration.clock, rtp_rtcp->RtpSender(), configuration.flexfec_sender, playout_delay_oracle.get(), frame_encryptor, crypto_options.sframe.require_frame_encryption, - FieldTrialBasedConfig()); + rtp_config.lntf.enabled, FieldTrialBasedConfig()); rtp_streams.emplace_back(std::move(playout_delay_oracle), std::move(rtp_rtcp), std::move(sender_video)); } diff --git a/modules/rtp_rtcp/source/nack_rtx_unittest.cc b/modules/rtp_rtcp/source/nack_rtx_unittest.cc index f8fd39a55f..7ace5a38e6 100644 --- a/modules/rtp_rtcp/source/nack_rtx_unittest.cc +++ b/modules/rtp_rtcp/source/nack_rtx_unittest.cc @@ -138,7 +138,7 @@ class RtpRtcpRtxNackTest : public ::testing::Test { rtp_rtcp_module_ = RtpRtcp::Create(configuration); rtp_sender_video_ = absl::make_unique( &fake_clock, rtp_rtcp_module_->RtpSender(), nullptr, - &playout_delay_oracle_, nullptr, false, FieldTrialBasedConfig()); + &playout_delay_oracle_, nullptr, false, false, FieldTrialBasedConfig()); rtp_rtcp_module_->SetSSRC(kTestSsrc); rtp_rtcp_module_->SetRTCPStatus(RtcpMode::kCompound); rtp_rtcp_module_->SetStorePacketsStatus(true, 600); diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc index 09f8dbd61d..2f059c45fe 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc @@ -187,7 +187,7 @@ class RtpRtcpImplTest : public ::testing::Test { sender_video_ = absl::make_unique( &clock_, sender_.impl_->RtpSender(), nullptr, &playout_delay_oracle_, - nullptr, false, FieldTrialBasedConfig()); + nullptr, false, false, FieldTrialBasedConfig()); memset(&codec_, 0, sizeof(VideoCodec)); codec_.plType = 100; diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 3d1bed45d1..944ec07937 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -515,7 +515,7 @@ TEST_P(RtpSenderTestWithoutPacer, OnSendSideDelayUpdated) { rtp_sender_->SetSSRC(kSsrc); PlayoutDelayOracle playout_delay_oracle; RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), nullptr, - &playout_delay_oracle, nullptr, false, + &playout_delay_oracle, nullptr, false, false, FieldTrialBasedConfig()); const uint8_t kPayloadType = 127; @@ -1098,7 +1098,7 @@ TEST_P(RtpSenderTestWithoutPacer, SendGenericVideo) { const uint8_t payload_type = 127; PlayoutDelayOracle playout_delay_oracle; RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), nullptr, - &playout_delay_oracle, nullptr, false, + &playout_delay_oracle, nullptr, false, false, FieldTrialBasedConfig()); rtp_sender_video.RegisterPayloadType(payload_type, payload_name, /*raw_payload=*/false); @@ -1141,7 +1141,7 @@ TEST_P(RtpSenderTestWithoutPacer, SendRawVideo) { PlayoutDelayOracle playout_delay_oracle; RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), nullptr, - &playout_delay_oracle, nullptr, false, + &playout_delay_oracle, nullptr, false, false, FieldTrialBasedConfig()); rtp_sender_video.RegisterPayloadType(payload_type, payload_name, /*raw_payload=*/true); @@ -1180,9 +1180,9 @@ TEST_P(RtpSenderTest, SendFlexfecPackets) { rtp_sender_->SetStorePacketsStatus(true, 10); PlayoutDelayOracle playout_delay_oracle; - RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), - &flexfec_sender, &playout_delay_oracle, - nullptr, false, FieldTrialBasedConfig()); + RTPSenderVideo rtp_sender_video( + &fake_clock_, rtp_sender_.get(), &flexfec_sender, &playout_delay_oracle, + nullptr, false, false, FieldTrialBasedConfig()); rtp_sender_video.RegisterPayloadType(kMediaPayloadType, "GENERIC", /*raw_payload=*/false); @@ -1257,9 +1257,9 @@ TEST_P(RtpSenderTest, NoFlexfecForTimingFrames) { rtp_sender_->SetStorePacketsStatus(true, 10); PlayoutDelayOracle playout_delay_oracle; - RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), - &flexfec_sender, &playout_delay_oracle, - nullptr, false, FieldTrialBasedConfig()); + RTPSenderVideo rtp_sender_video( + &fake_clock_, rtp_sender_.get(), &flexfec_sender, &playout_delay_oracle, + nullptr, false, false, FieldTrialBasedConfig()); rtp_sender_video.RegisterPayloadType(kMediaPayloadType, "GENERIC", /*raw_payload=*/false); @@ -1359,9 +1359,9 @@ TEST_P(RtpSenderTestWithoutPacer, SendFlexfecPackets) { rtp_sender_->SetSequenceNumber(kSeqNum); PlayoutDelayOracle playout_delay_oracle; - RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), - &flexfec_sender, &playout_delay_oracle, - nullptr, false, FieldTrialBasedConfig()); + RTPSenderVideo rtp_sender_video( + &fake_clock_, rtp_sender_.get(), &flexfec_sender, &playout_delay_oracle, + nullptr, false, false, FieldTrialBasedConfig()); rtp_sender_video.RegisterPayloadType(kMediaPayloadType, "GENERIC", /*raw_payload=*/false); @@ -1490,9 +1490,9 @@ TEST_P(RtpSenderTest, FecOverheadRate) { rtp_sender_->SetSequenceNumber(kSeqNum); PlayoutDelayOracle playout_delay_oracle; - RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), - &flexfec_sender, &playout_delay_oracle, - nullptr, false, FieldTrialBasedConfig()); + RTPSenderVideo rtp_sender_video( + &fake_clock_, rtp_sender_.get(), &flexfec_sender, &playout_delay_oracle, + nullptr, false, false, FieldTrialBasedConfig()); rtp_sender_video.RegisterPayloadType(kMediaPayloadType, "GENERIC", /*raw_payload=*/false); // Parameters selected to generate a single FEC packet per media packet. @@ -1562,7 +1562,7 @@ TEST_P(RtpSenderTest, BitrateCallbacks) { PlayoutDelayOracle playout_delay_oracle; RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), nullptr, - &playout_delay_oracle, nullptr, false, + &playout_delay_oracle, nullptr, false, false, FieldTrialBasedConfig()); const char payload_name[] = "GENERIC"; const uint8_t payload_type = 127; @@ -1651,7 +1651,7 @@ TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacks) { const uint8_t payload_type = 127; PlayoutDelayOracle playout_delay_oracle; RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), nullptr, - &playout_delay_oracle, nullptr, false, + &playout_delay_oracle, nullptr, false, false, FieldTrialBasedConfig()); rtp_sender_video.RegisterPayloadType(payload_type, payload_name, /*raw_payload=*/false); diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index cd388b49c0..b69b0d5542 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -187,6 +187,7 @@ RTPSenderVideo::RTPSenderVideo(Clock* clock, PlayoutDelayOracle* playout_delay_oracle, FrameEncryptorInterface* frame_encryptor, bool require_frame_encryption, + bool need_rtp_packet_infos, const WebRtcKeyValueConfig& field_trials) : rtp_sender_(rtp_sender), clock_(clock), @@ -195,15 +196,10 @@ RTPSenderVideo::RTPSenderVideo(Clock* clock, last_rotation_(kVideoRotation_0), transmit_color_space_next_frame_(false), playout_delay_oracle_(playout_delay_oracle), - // TODO(bugs.webrtc.org/10662): Choose whether to instantiate - // |rtp_sequence_number_map_| according to the negotiation of the - // LNTF (loss notification) rtcp-fb message. - rtp_sequence_number_map_( - field_trials.Lookup("WebRTC-RtcpLossNotification").find("Enabled") != - std::string::npos - ? absl::make_unique( - kRtpSequenceNumberMapMaxEntries) - : nullptr), + rtp_sequence_number_map_(need_rtp_packet_infos + ? absl::make_unique( + kRtpSequenceNumberMapMaxEntries) + : nullptr), red_payload_type_(-1), ulpfec_payload_type_(-1), flexfec_sender_(flexfec_sender), diff --git a/modules/rtp_rtcp/source/rtp_sender_video.h b/modules/rtp_rtcp/source/rtp_sender_video.h index bdbe90a2f5..ecb3be5e58 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.h +++ b/modules/rtp_rtcp/source/rtp_sender_video.h @@ -58,6 +58,7 @@ class RTPSenderVideo { PlayoutDelayOracle* playout_delay_oracle, FrameEncryptorInterface* frame_encryptor, bool require_frame_encryption, + bool need_rtp_packet_infos, const WebRtcKeyValueConfig& field_trials); virtual ~RTPSenderVideo(); diff --git a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc index 9864ecb0f6..e6468aaeb1 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc @@ -104,6 +104,7 @@ class TestRtpSenderVideo : public RTPSenderVideo { &playout_delay_oracle_, nullptr, false, + false, field_trials) {} ~TestRtpSenderVideo() override {}