Negotiate use of RTCP loss notification feedback (LNTF)

When the LossNotifications field trial is in effect, LNTF should
be offered/accepted in the SDP message, not assumed to be configured
on both sides equally.

Bug: webrtc:10662
Change-Id: Ibd827779bd301821cbb4196857f6baebfc9e7dc2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/138079
Commit-Queue: Elad Alon <eladalon@webrtc.org>
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28056}

api/rtp_parameters.h

@@ -50,6 +50,7 @@ enum class FecMechanism {
 // Used in RtcpFeedback struct.
 enum class RtcpFeedbackType {
   CCM,
+  LNTF,  // "goog-lntf"
   NACK,
   REMB,  // "goog-remb"
   TRANSPORT_CC,

call/rtp_config.cc

@@ -17,6 +17,10 @@
 
 namespace webrtc {
 
+std::string LntfConfig::ToString() const {
+  return enabled ? "{enabled: true}" : "{enabled: false}";
+}
+
 std::string NackConfig::ToString() const {
   char buf[1024];
   rtc::SimpleStringBuilder ss(buf);
@@ -72,6 +76,7 @@ std::string RtpConfig::ToString() const {
   }
   ss << ']';
 
+  ss << ", lntf: " << lntf.ToString();
   ss << ", nack: {rtp_history_ms: " << nack.rtp_history_ms << '}';
   ss << ", ulpfec: " << ulpfec.ToString();
   ss << ", payload_name: " << payload_name;

call/rtp_config.h

@@ -26,6 +26,14 @@ struct RtpPayloadState {
   uint8_t tl0_pic_idx = 0;
   int64_t shared_frame_id = 0;
 };
+
+// Settings for LNTF (LossNotification). Still highly experimental.
+struct LntfConfig {
+  std::string ToString() const;
+
+  bool enabled{false};  // TODO(bugs.webrtc.org/10662): Consume this flag.
+};
+
 // Settings for NACK, see RFC 4585 for details.
 struct NackConfig {
   NackConfig() : rtp_history_ms(0) {}
@@ -104,6 +112,9 @@ struct RtpConfig {
   // frame descriptor RTP header extension).
   bool raw_payload = false;
 
+  // See LntfConfig for description.
+  LntfConfig lntf;
+
   // See NackConfig for description.
   NackConfig nack;
 

call/video_receive_stream.cc

@@ -116,6 +116,7 @@ std::string VideoReceiveStream::Config::Rtp::ToString() const {
   ss << '}';
   ss << ", remb: " << (remb ? "on" : "off");
   ss << ", transport_cc: " << (transport_cc ? "on" : "off");
+  ss << ", lntf: {enabled: " << (lntf.enabled ? "true" : "false") << '}';
   ss << ", nack: {rtp_history_ms: " << nack.rtp_history_ms << '}';
   ss << ", ulpfec_payload_type: " << ulpfec_payload_type;
   ss << ", red_type: " << red_payload_type;

call/video_receive_stream.h

@@ -179,6 +179,9 @@ class VideoReceiveStream {
       // See draft-holmer-rmcat-transport-wide-cc-extensions for details.
       bool transport_cc = false;
 
+      // See LntfConfig for description.
+      LntfConfig lntf;
+
       // See NackConfig for description.
       NackConfig nack;
 

media/base/codec.cc

@@ -353,6 +353,11 @@ std::string RtpDataCodec::ToString() const {
   return sb.str();
 }
 
+bool HasLntf(const Codec& codec) {
+  return codec.HasFeedbackParam(
+      FeedbackParam(kRtcpFbParamLntf, kParamValueEmpty));
+}
+
 bool HasNack(const Codec& codec) {
   return codec.HasFeedbackParam(
       FeedbackParam(kRtcpFbParamNack, kParamValueEmpty));

media/base/codec.h

@@ -220,6 +220,7 @@ const Codec* FindCodecById(const std::vector<Codec>& codecs, int payload_type) {
   return nullptr;
 }
 
+bool HasLntf(const Codec& codec);
 bool HasNack(const Codec& codec);
 bool HasRemb(const Codec& codec);
 bool HasRrtr(const Codec& codec);

media/base/media_constants.cc

@@ -77,6 +77,7 @@ const int kPreferredSPropStereo = 0;
 const int kPreferredStereo = 0;
 const int kPreferredUseInbandFec = 0;
 
+const char kRtcpFbParamLntf[] = "goog-lntf";
 const char kRtcpFbParamNack[] = "nack";
 const char kRtcpFbNackParamPli[] = "pli";
 const char kRtcpFbParamRemb[] = "goog-remb";

media/base/media_constants.h

@@ -91,6 +91,8 @@ extern const int kPreferredSPropStereo;
 extern const int kPreferredStereo;
 extern const int kPreferredUseInbandFec;
 
+// rtcp-fb message in its first experimental stages. Documentation pending.
+extern const char kRtcpFbParamLntf[];
 // rtcp-fb messages according to RFC 4585
 extern const char kRtcpFbParamNack[];
 extern const char kRtcpFbNackParamPli[];

media/engine/webrtc_video_engine.cc

@@ -72,6 +72,10 @@ void AddDefaultFeedbackParams(VideoCodec* codec) {
   codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamCcm, kRtcpFbCcmParamFir));
   codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamNack, kParamValueEmpty));
   codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamNack, kRtcpFbNackParamPli));
+  if (codec->name == kVp8CodecName &&
+      webrtc::field_trial::IsEnabled("WebRTC-RtcpLossNotification")) {
+    codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamLntf, kParamValueEmpty));
+  }
 }
 
 // This function will assign dynamic payload types (in the range [96, 127]) to
@@ -766,8 +770,8 @@ bool WebRtcVideoChannel::SetSendParameters(const VideoSendParameters& params) {
       for (auto& kv : receive_streams_) {
         RTC_DCHECK(kv.second != nullptr);
         kv.second->SetFeedbackParameters(
-            HasNack(send_codec_->codec), HasRemb(send_codec_->codec),
-            HasTransportCc(send_codec_->codec),
+            HasLntf(send_codec_->codec), HasNack(send_codec_->codec),
+            HasRemb(send_codec_->codec), HasTransportCc(send_codec_->codec),
             params.rtcp.reduced_size ? webrtc::RtcpMode::kReducedSize
                                      : webrtc::RtcpMode::kCompound);
       }
@@ -1889,6 +1893,8 @@ void WebRtcVideoChannel::WebRtcVideoSendStream::SetCodec(
     }
   }
 
+  parameters_.config.rtp.lntf.enabled = HasLntf(codec_settings.codec);
+
   parameters_.config.rtp.nack.rtp_history_ms =
       HasNack(codec_settings.codec) ? kNackHistoryMs : 0;
 
@@ -2470,6 +2476,7 @@ void WebRtcVideoChannel::WebRtcVideoReceiveStream::ConfigureCodecs(
   config_.rtp.ulpfec_payload_type = codec.ulpfec.ulpfec_payload_type;
   config_.rtp.red_payload_type = codec.ulpfec.red_payload_type;
 
+  config_.rtp.lntf.enabled = HasLntf(codec.codec);
   config_.rtp.nack.rtp_history_ms = HasNack(codec.codec) ? kNackHistoryMs : 0;
   config_.rtp.rtcp_xr.receiver_reference_time_report = HasRrtr(codec.codec);
   if (codec.ulpfec.red_rtx_payload_type != -1) {
@@ -2507,23 +2514,27 @@ void WebRtcVideoChannel::WebRtcVideoReceiveStream::SetLocalSsrc(
 }
 
 void WebRtcVideoChannel::WebRtcVideoReceiveStream::SetFeedbackParameters(
+    bool lntf_enabled,
     bool nack_enabled,
     bool remb_enabled,
     bool transport_cc_enabled,
     webrtc::RtcpMode rtcp_mode) {
   int nack_history_ms = nack_enabled ? kNackHistoryMs : 0;
-  if (config_.rtp.nack.rtp_history_ms == nack_history_ms &&
+  if (config_.rtp.lntf.enabled == lntf_enabled &&
+      config_.rtp.nack.rtp_history_ms == nack_history_ms &&
       config_.rtp.remb == remb_enabled &&
       config_.rtp.transport_cc == transport_cc_enabled &&
       config_.rtp.rtcp_mode == rtcp_mode) {
     RTC_LOG(LS_INFO)
         << "Ignoring call to SetFeedbackParameters because parameters are "
-           "unchanged; nack="
-        << nack_enabled << ", remb=" << remb_enabled
+           "unchanged; lntf="
+        << lntf_enabled << ", nack=" << nack_enabled
+        << ", remb=" << remb_enabled
         << ", transport_cc=" << transport_cc_enabled;
     return;
   }
   config_.rtp.remb = remb_enabled;
+  config_.rtp.lntf.enabled = lntf_enabled;
   config_.rtp.nack.rtp_history_ms = nack_history_ms;
   config_.rtp.transport_cc = transport_cc_enabled;
   config_.rtp.rtcp_mode = rtcp_mode;
@@ -2743,6 +2754,7 @@ WebRtcVideoChannel::WebRtcVideoReceiveStream::GetVideoReceiverInfo(
   info.firs_sent = stats.rtcp_packet_type_counts.fir_packets;
   info.plis_sent = stats.rtcp_packet_type_counts.pli_packets;
   info.nacks_sent = stats.rtcp_packet_type_counts.nack_packets;
+  // TODO(bugs.webrtc.org/10662): Add stats for LNTF.
 
   info.timing_frame_info = stats.timing_frame_info;
 

media/engine/webrtc_video_engine.h

@@ -400,7 +400,8 @@ class WebRtcVideoChannel : public VideoMediaChannel, public webrtc::Transport {
 
     void SetLocalSsrc(uint32_t local_ssrc);
     // TODO(deadbeef): Move these feedback parameters into the recv parameters.
-    void SetFeedbackParameters(bool nack_enabled,
+    void SetFeedbackParameters(bool lntf_enabled,
+                               bool nack_enabled,
                                bool remb_enabled,
                                bool transport_cc_enabled,
                                webrtc::RtcpMode rtcp_mode);

media/engine/webrtc_video_engine_unittest.cc

@@ -91,7 +91,11 @@ cricket::VideoCodec RemoveFeedbackParams(cricket::VideoCodec&& codec) {
   return std::move(codec);
 }
 
-void VerifyCodecHasDefaultFeedbackParams(const cricket::VideoCodec& codec) {
+void VerifyCodecHasDefaultFeedbackParams(const cricket::VideoCodec& codec,
+                                         bool lntf_expected) {
+  EXPECT_EQ(lntf_expected,
+            codec.HasFeedbackParam(cricket::FeedbackParam(
+                cricket::kRtcpFbParamLntf, cricket::kParamValueEmpty)));
   EXPECT_TRUE(codec.HasFeedbackParam(cricket::FeedbackParam(
       cricket::kRtcpFbParamNack, cricket::kParamValueEmpty)));
   EXPECT_TRUE(codec.HasFeedbackParam(cricket::FeedbackParam(
@@ -1104,7 +1108,8 @@ TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, Vp8) {
       cricket::kCodecParamAssociatedPayloadType, &associated_payload_type));
   EXPECT_EQ(engine_codecs.at(0).id, associated_payload_type);
   // Verify default parameters has been added to the VP8 codec.
-  VerifyCodecHasDefaultFeedbackParams(engine_codecs.at(0));
+  VerifyCodecHasDefaultFeedbackParams(engine_codecs.at(0),
+                                      /*lntf_expected=*/false);
 
   // Mock encoder creation. |engine| take ownership of the encoder.
   webrtc::VideoEncoderFactory::CodecInfo codec_info;
@@ -2262,6 +2267,26 @@ class WebRtcVideoChannelTest : public WebRtcVideoEngineTest {
     EXPECT_EQ(ext_uri, recv_stream->GetConfig().rtp.extensions[0].uri);
   }
 
+  void TestLossNotificationState(bool expect_lntf_enabled) {
+    AssignDefaultCodec();
+    VerifyCodecHasDefaultFeedbackParams(default_codec_, expect_lntf_enabled);
+
+    cricket::VideoSendParameters parameters;
+    parameters.codecs = engine_.codecs();
+    EXPECT_TRUE(channel_->SetSendParameters(parameters));
+    EXPECT_TRUE(channel_->SetSend(true));
+
+    // Send side.
+    FakeVideoSendStream* send_stream =
+        AddSendStream(cricket::StreamParams::CreateLegacy(1));
+    EXPECT_EQ(send_stream->GetConfig().rtp.lntf.enabled, expect_lntf_enabled);
+
+    // Receiver side.
+    FakeVideoReceiveStream* recv_stream =
+        AddRecvStream(cricket::StreamParams::CreateLegacy(1));
+    EXPECT_EQ(recv_stream->GetConfig().rtp.lntf.enabled, expect_lntf_enabled);
+  }
+
   void TestExtensionFilter(const std::vector<std::string>& extensions,
                            const std::string& expected_extension) {
     cricket::VideoSendParameters parameters = send_parameters_;
@@ -2734,9 +2759,64 @@ TEST_F(WebRtcVideoChannelTest, TransportCcCanBeEnabledAndDisabled) {
   EXPECT_TRUE(stream->GetConfig().rtp.transport_cc);
 }
 
+TEST_F(WebRtcVideoChannelTest, LossNotificationIsDisabledByDefault) {
+  TestLossNotificationState(false);
+}
+
+TEST_F(WebRtcVideoChannelTest, LossNotificationIsEnabledByFieldTrial) {
+  RTC_DCHECK(!override_field_trials_);
+  override_field_trials_ = absl::make_unique<webrtc::test::ScopedFieldTrials>(
+      "WebRTC-RtcpLossNotification/Enabled/");
+  SetUp();
+  TestLossNotificationState(true);
+}
+
+TEST_F(WebRtcVideoChannelTest, LossNotificationCanBeEnabledAndDisabled) {
+  RTC_DCHECK(!override_field_trials_);
+  override_field_trials_ = absl::make_unique<webrtc::test::ScopedFieldTrials>(
+      "WebRTC-RtcpLossNotification/Enabled/");
+  SetUp();
+
+  AssignDefaultCodec();
+  VerifyCodecHasDefaultFeedbackParams(default_codec_, true);
+
+  {
+    cricket::VideoSendParameters parameters;
+    parameters.codecs = engine_.codecs();
+    EXPECT_TRUE(channel_->SetSendParameters(parameters));
+    EXPECT_TRUE(channel_->SetSend(true));
+  }
+
+  // Start with LNTF enabled.
+  FakeVideoSendStream* send_stream =
+      AddSendStream(cricket::StreamParams::CreateLegacy(1));
+  ASSERT_TRUE(send_stream->GetConfig().rtp.lntf.enabled);
+  FakeVideoReceiveStream* recv_stream =
+      AddRecvStream(cricket::StreamParams::CreateLegacy(1));
+  ASSERT_TRUE(recv_stream->GetConfig().rtp.lntf.enabled);
+
+  // Verify that LNTF is turned off when send(!) codecs without LNTF are set.
+  cricket::VideoSendParameters parameters;
+  parameters.codecs.push_back(RemoveFeedbackParams(GetEngineCodec("VP8")));
+  EXPECT_TRUE(parameters.codecs[0].feedback_params.params().empty());
+  EXPECT_TRUE(channel_->SetSendParameters(parameters));
+  recv_stream = fake_call_->GetVideoReceiveStreams()[0];
+  EXPECT_FALSE(recv_stream->GetConfig().rtp.lntf.enabled);
+  send_stream = fake_call_->GetVideoSendStreams()[0];
+  EXPECT_FALSE(send_stream->GetConfig().rtp.lntf.enabled);
+
+  // Setting the default codecs again, including VP8, turns LNTF back on.
+  parameters.codecs = engine_.codecs();
+  EXPECT_TRUE(channel_->SetSendParameters(parameters));
+  recv_stream = fake_call_->GetVideoReceiveStreams()[0];
+  EXPECT_TRUE(recv_stream->GetConfig().rtp.lntf.enabled);
+  send_stream = fake_call_->GetVideoSendStreams()[0];
+  EXPECT_TRUE(send_stream->GetConfig().rtp.lntf.enabled);
+}
+
 TEST_F(WebRtcVideoChannelTest, NackIsEnabledByDefault) {
   AssignDefaultCodec();
-  VerifyCodecHasDefaultFeedbackParams(default_codec_);
+  VerifyCodecHasDefaultFeedbackParams(default_codec_, false);
 
   cricket::VideoSendParameters parameters;
   parameters.codecs = engine_.codecs();

modules/rtp_rtcp/source/rtp_sender_video.cc

@@ -195,8 +195,9 @@ RTPSenderVideo::RTPSenderVideo(Clock* clock,
       last_rotation_(kVideoRotation_0),
       transmit_color_space_next_frame_(false),
       playout_delay_oracle_(playout_delay_oracle),
-      // TODO(eladalon): Choose whether to instantiate rtp_sequence_number_map_
-      // according to the negotiation of the RTCP message.
+      // 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

pc/rtp_parameters_conversion.cc

@@ -39,6 +39,13 @@ RTCErrorOr<cricket::FeedbackParam> ToCricketFeedbackParam(
       }
       return cricket::FeedbackParam(cricket::kRtcpFbParamCcm,
                                     cricket::kRtcpFbCcmParamFir);
+    case RtcpFeedbackType::LNTF:
+      if (feedback.message_type) {
+        LOG_AND_RETURN_ERROR(
+            RTCErrorType::INVALID_PARAMETER,
+            "Didn't expect message type in LNTF RtcpFeedback.");
+      }
+      return cricket::FeedbackParam(cricket::kRtcpFbParamLntf);
     case RtcpFeedbackType::NACK:
       if (!feedback.message_type) {
         LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER,
@@ -253,6 +260,14 @@ absl::optional<RtcpFeedback> ToRtcpFeedback(
                           << cricket_feedback.param();
       return absl::nullopt;
     }
+  } else if (cricket_feedback.id() == cricket::kRtcpFbParamLntf) {
+    if (cricket_feedback.param().empty()) {
+      return RtcpFeedback(RtcpFeedbackType::LNTF);
+    } else {
+      RTC_LOG(LS_WARNING) << "Unsupported parameter for LNTF RTCP feedback: "
+                          << cricket_feedback.param();
+      return absl::nullopt;
+    }
   } else if (cricket_feedback.id() == cricket::kRtcpFbParamNack) {
     if (cricket_feedback.param().empty()) {
       return RtcpFeedback(RtcpFeedbackType::NACK,

pc/rtp_parameters_conversion_unittest.cc

@@ -22,14 +22,21 @@ TEST(RtpParametersConversionTest, ToCricketFeedbackParam) {
   auto result = ToCricketFeedbackParam(
       {RtcpFeedbackType::CCM, RtcpFeedbackMessageType::FIR});
   EXPECT_EQ(cricket::FeedbackParam("ccm", "fir"), result.value());
+
+  result = ToCricketFeedbackParam(RtcpFeedback(RtcpFeedbackType::LNTF));
+  EXPECT_EQ(cricket::FeedbackParam("goog-lntf"), result.value());
+
   result = ToCricketFeedbackParam(
       {RtcpFeedbackType::NACK, RtcpFeedbackMessageType::GENERIC_NACK});
   EXPECT_EQ(cricket::FeedbackParam("nack"), result.value());
+
   result = ToCricketFeedbackParam(
       {RtcpFeedbackType::NACK, RtcpFeedbackMessageType::PLI});
   EXPECT_EQ(cricket::FeedbackParam("nack", "pli"), result.value());
+
   result = ToCricketFeedbackParam(RtcpFeedback(RtcpFeedbackType::REMB));
   EXPECT_EQ(cricket::FeedbackParam("goog-remb"), result.value());
+
   result = ToCricketFeedbackParam(RtcpFeedback(RtcpFeedbackType::TRANSPORT_CC));
   EXPECT_EQ(cricket::FeedbackParam("transport-cc"), result.value());
 }
@@ -38,19 +45,29 @@ TEST(RtpParametersConversionTest, ToCricketFeedbackParamErrors) {
   // CCM with missing or invalid message type.
   auto result = ToCricketFeedbackParam(RtcpFeedback(RtcpFeedbackType::CCM));
   EXPECT_EQ(RTCErrorType::INVALID_PARAMETER, result.error().type());
+
   result = ToCricketFeedbackParam(
       {RtcpFeedbackType::CCM, RtcpFeedbackMessageType::PLI});
   EXPECT_EQ(RTCErrorType::INVALID_PARAMETER, result.error().type());
+
+  // LNTF with message type (should be left empty).
+  result = ToCricketFeedbackParam(
+      {RtcpFeedbackType::LNTF, RtcpFeedbackMessageType::GENERIC_NACK});
+  EXPECT_EQ(RTCErrorType::INVALID_PARAMETER, result.error().type());
+
   // NACK with missing or invalid message type.
   result = ToCricketFeedbackParam(RtcpFeedback(RtcpFeedbackType::NACK));
   EXPECT_EQ(RTCErrorType::INVALID_PARAMETER, result.error().type());
+
   result = ToCricketFeedbackParam(
       {RtcpFeedbackType::NACK, RtcpFeedbackMessageType::FIR});
   EXPECT_EQ(RTCErrorType::INVALID_PARAMETER, result.error().type());
+
   // REMB with message type (should be left empty).
   result = ToCricketFeedbackParam(
       {RtcpFeedbackType::REMB, RtcpFeedbackMessageType::GENERIC_NACK});
   EXPECT_EQ(RTCErrorType::INVALID_PARAMETER, result.error().type());
+
   // TRANSPORT_CC with message type (should be left empty).
   result = ToCricketFeedbackParam(
       {RtcpFeedbackType::TRANSPORT_CC, RtcpFeedbackMessageType::FIR});
@@ -88,6 +105,7 @@ TEST(RtpParametersConversionTest, ToVideoCodec) {
   codec.clock_rate.emplace(90000);
   codec.parameters["foo"] = "bar";
   codec.parameters["PING"] = "PONG";
+  codec.rtcp_feedback.emplace_back(RtcpFeedbackType::LNTF);
   codec.rtcp_feedback.emplace_back(RtcpFeedbackType::TRANSPORT_CC);
   codec.rtcp_feedback.emplace_back(RtcpFeedbackType::NACK,
                                    RtcpFeedbackMessageType::PLI);
@@ -100,7 +118,9 @@ TEST(RtpParametersConversionTest, ToVideoCodec) {
   ASSERT_EQ(2u, result.value().params.size());
   EXPECT_EQ("bar", result.value().params["foo"]);
   EXPECT_EQ("PONG", result.value().params["PING"]);
-  EXPECT_EQ(2u, result.value().feedback_params.params().size());
+  EXPECT_EQ(3u, result.value().feedback_params.params().size());
+  EXPECT_TRUE(
+      result.value().feedback_params.Has(cricket::FeedbackParam("goog-lntf")));
   EXPECT_TRUE(result.value().feedback_params.Has(
       cricket::FeedbackParam("transport-cc")));
   EXPECT_TRUE(result.value().feedback_params.Has(
@@ -382,15 +402,22 @@ TEST(RtpParametersConversionTest, ToRtcpFeedback) {
   absl::optional<RtcpFeedback> result = ToRtcpFeedback({"ccm", "fir"});
   EXPECT_EQ(RtcpFeedback(RtcpFeedbackType::CCM, RtcpFeedbackMessageType::FIR),
             *result);
+
+  result = ToRtcpFeedback(cricket::FeedbackParam("goog-lntf"));
+  EXPECT_EQ(RtcpFeedback(RtcpFeedbackType::LNTF), *result);
+
   result = ToRtcpFeedback(cricket::FeedbackParam("nack"));
   EXPECT_EQ(RtcpFeedback(RtcpFeedbackType::NACK,
                          RtcpFeedbackMessageType::GENERIC_NACK),
             *result);
+
   result = ToRtcpFeedback({"nack", "pli"});
   EXPECT_EQ(RtcpFeedback(RtcpFeedbackType::NACK, RtcpFeedbackMessageType::PLI),
             *result);
+
   result = ToRtcpFeedback(cricket::FeedbackParam("goog-remb"));
   EXPECT_EQ(RtcpFeedback(RtcpFeedbackType::REMB), *result);
+
   result = ToRtcpFeedback(cricket::FeedbackParam("transport-cc"));
   EXPECT_EQ(RtcpFeedback(RtcpFeedbackType::TRANSPORT_CC), *result);
 }
@@ -399,17 +426,26 @@ TEST(RtpParametersConversionTest, ToRtcpFeedbackErrors) {
   // CCM with missing or invalid message type.
   absl::optional<RtcpFeedback> result = ToRtcpFeedback({"ccm", "pli"});
   EXPECT_FALSE(result);
+
   result = ToRtcpFeedback(cricket::FeedbackParam("ccm"));
   EXPECT_FALSE(result);
+
+  // LNTF with message type (should be left empty).
+  result = ToRtcpFeedback({"goog-lntf", "pli"});
+  EXPECT_FALSE(result);
+
   // NACK with missing or invalid message type.
   result = ToRtcpFeedback({"nack", "fir"});
   EXPECT_FALSE(result);
+
   // REMB with message type (should be left empty).
   result = ToRtcpFeedback({"goog-remb", "pli"});
   EXPECT_FALSE(result);
+
   // TRANSPORT_CC with message type (should be left empty).
   result = ToRtcpFeedback({"transport-cc", "fir"});
   EXPECT_FALSE(result);
+
   // Unknown message type.
   result = ToRtcpFeedback(cricket::FeedbackParam("foo"));
   EXPECT_FALSE(result);
@@ -445,6 +481,7 @@ TEST(RtpParametersConversionTest, ToVideoRtpCodecCapability) {
   cricket_codec.params["foo"] = "bar";
   cricket_codec.params["ANOTHER"] = "param";
   cricket_codec.feedback_params.Add(cricket::FeedbackParam("transport-cc"));
+  cricket_codec.feedback_params.Add(cricket::FeedbackParam("goog-lntf"));
   cricket_codec.feedback_params.Add({"nack", "pli"});
   RtpCodecCapability codec = ToRtpCodecCapability(cricket_codec);
 
@@ -455,11 +492,12 @@ TEST(RtpParametersConversionTest, ToVideoRtpCodecCapability) {
   ASSERT_EQ(2u, codec.parameters.size());
   EXPECT_EQ("bar", codec.parameters["foo"]);
   EXPECT_EQ("param", codec.parameters["ANOTHER"]);
-  EXPECT_EQ(2u, codec.rtcp_feedback.size());
+  EXPECT_EQ(3u, codec.rtcp_feedback.size());
   EXPECT_EQ(RtcpFeedback(RtcpFeedbackType::TRANSPORT_CC),
             codec.rtcp_feedback[0]);
+  EXPECT_EQ(RtcpFeedback(RtcpFeedbackType::LNTF), codec.rtcp_feedback[1]);
   EXPECT_EQ(RtcpFeedback(RtcpFeedbackType::NACK, RtcpFeedbackMessageType::PLI),
-            codec.rtcp_feedback[1]);
+            codec.rtcp_feedback[2]);
 }
 
 TEST(RtpParametersConversionTest, ToRtpEncodingsWithEmptyStreamParamsVec) {
@@ -519,6 +557,7 @@ TEST(RtpParametersConversionTest, ToVideoRtpCodecParameters) {
   cricket_codec.params["foo"] = "bar";
   cricket_codec.params["ANOTHER"] = "param";
   cricket_codec.feedback_params.Add(cricket::FeedbackParam("transport-cc"));
+  cricket_codec.feedback_params.Add(cricket::FeedbackParam("goog-lntf"));
   cricket_codec.feedback_params.Add({"nack", "pli"});
   RtpCodecParameters codec = ToRtpCodecParameters(cricket_codec);
 
@@ -529,11 +568,12 @@ TEST(RtpParametersConversionTest, ToVideoRtpCodecParameters) {
   ASSERT_EQ(2u, codec.parameters.size());
   EXPECT_EQ("bar", codec.parameters["foo"]);
   EXPECT_EQ("param", codec.parameters["ANOTHER"]);
-  EXPECT_EQ(2u, codec.rtcp_feedback.size());
+  EXPECT_EQ(3u, codec.rtcp_feedback.size());
   EXPECT_EQ(RtcpFeedback(RtcpFeedbackType::TRANSPORT_CC),
             codec.rtcp_feedback[0]);
+  EXPECT_EQ(RtcpFeedback(RtcpFeedbackType::LNTF), codec.rtcp_feedback[1]);
   EXPECT_EQ(RtcpFeedback(RtcpFeedbackType::NACK, RtcpFeedbackMessageType::PLI),
-            codec.rtcp_feedback[1]);
+            codec.rtcp_feedback[2]);
 }
 
 // An unknown feedback param should just be ignored.

pc/webrtc_sdp_unittest.cc

@@ -1993,6 +1993,7 @@ class WebRtcSdpTest : public ::testing::Test {
     std::string sdp_video =
         "m=video 3457 RTP/SAVPF 101\r\n"
         "a=rtpmap:101 VP8/90000\r\n"
+        "a=rtcp-fb:101 goog-lntf\r\n"
         "a=rtcp-fb:101 nack\r\n"
         "a=rtcp-fb:101 nack pli\r\n"
         "a=rtcp-fb:101 goog-remb\r\n";
@@ -2022,6 +2023,8 @@ class WebRtcSdpTest : public ::testing::Test {
     EXPECT_STREQ(webrtc::JsepSessionDescription::kDefaultVideoCodecName,
                  vp8.name.c_str());
     EXPECT_EQ(101, vp8.id);
+    EXPECT_TRUE(vp8.HasFeedbackParam(cricket::FeedbackParam(
+        cricket::kRtcpFbParamLntf, cricket::kParamValueEmpty)));
     EXPECT_TRUE(vp8.HasFeedbackParam(cricket::FeedbackParam(
         cricket::kRtcpFbParamNack, cricket::kParamValueEmpty)));
     EXPECT_TRUE(vp8.HasFeedbackParam(cricket::FeedbackParam(

sdk/objc/unittests/RTCSessionDescriptionTest.mm

@@ -109,6 +109,7 @@
           "a=rtcp-mux\r\n"
           "a=rtpmap:100 VP8/90000\r\n"
           "a=rtcp-fb:100 ccm fir\r\n"
+          "a=rtcp-fb:100 goog-lntf\r\n"
           "a=rtcp-fb:100 nack\r\n"
           "a=rtcp-fb:100 nack pli\r\n"
           "a=rtcp-fb:100 goog-remb\r\n"

test/call_config_utils.cc

@@ -45,6 +45,7 @@ VideoReceiveStream::Config ParseVideoReceiveStreamJsonConfig(
           : RtcpMode::kReducedSize;
   receive_config.rtp.remb = json["rtp"]["remb"].asBool();
   receive_config.rtp.transport_cc = json["rtp"]["transport_cc"].asBool();
+  receive_config.rtp.lntf.enabled = json["rtp"]["lntf"]["enabled"].asInt64();
   receive_config.rtp.nack.rtp_history_ms =
       json["rtp"]["nack"]["rtp_history_ms"].asInt64();
   receive_config.rtp.ulpfec_payload_type =
@@ -94,6 +95,7 @@ Json::Value GenerateVideoReceiveStreamJsonConfig(
                               : "RtcpMode::kReducedSize";
   rtp_json["remb"] = config.rtp.remb;
   rtp_json["transport_cc"] = config.rtp.transport_cc;
+  rtp_json["lntf"]["enabled"] = config.rtp.lntf.enabled;
   rtp_json["nack"]["rtp_history_ms"] = config.rtp.nack.rtp_history_ms;
   rtp_json["ulpfec_payload_type"] = config.rtp.ulpfec_payload_type;
   rtp_json["red_payload_type"] = config.rtp.red_payload_type;

test/call_config_utils_unittest.cc

@@ -31,6 +31,7 @@ TEST(CallConfigUtils, MarshalUnmarshalProcessSameObject) {
   recv_config.rtp.rtcp_mode = RtcpMode::kCompound;
   recv_config.rtp.remb = false;
   recv_config.rtp.transport_cc = false;
+  recv_config.rtp.lntf.enabled = false;
   recv_config.rtp.nack.rtp_history_ms = 150;
   recv_config.rtp.red_payload_type = 50;
   recv_config.rtp.rtx_ssrc = 1000;
@@ -54,6 +55,7 @@ TEST(CallConfigUtils, MarshalUnmarshalProcessSameObject) {
   EXPECT_EQ(recv_config.rtp.rtcp_mode, unmarshaled_config.rtp.rtcp_mode);
   EXPECT_EQ(recv_config.rtp.remb, unmarshaled_config.rtp.remb);
   EXPECT_EQ(recv_config.rtp.transport_cc, unmarshaled_config.rtp.transport_cc);
+  EXPECT_EQ(recv_config.rtp.lntf.enabled, unmarshaled_config.rtp.lntf.enabled);
   EXPECT_EQ(recv_config.rtp.nack.rtp_history_ms,
             unmarshaled_config.rtp.nack.rtp_history_ms);
   EXPECT_EQ(recv_config.rtp.red_payload_type,

test/fuzzers/configs/replay_packet_fuzzer/h264_fec_config.json

@@ -112,6 +112,9 @@
             }
          ],
          "local_ssrc" : 1,
+         "lntf" : {
+            "enabled": false,
+         },
          "nack" : {
             "rtp_history_ms" : 1000
          },

test/fuzzers/configs/replay_packet_fuzzer/h264_non_interleaved_config.json

@@ -36,6 +36,9 @@
       "rtp" : {
          "extensions" : [],
          "local_ssrc" : 1,
+         "lntf" : {
+            "enabled": false,
+         },
          "nack" : {
             "rtp_history_ms" : 1000
          },

test/fuzzers/configs/replay_packet_fuzzer/h264_single_nal_config.json

@@ -36,6 +36,9 @@
       "rtp" : {
          "extensions" : [],
          "local_ssrc" : 1,
+         "lntf" : {
+            "enabled": false,
+         },
          "nack" : {
             "rtp_history_ms" : 1000
          },

test/fuzzers/configs/replay_packet_fuzzer/vp8_config.json

@@ -10,6 +10,9 @@
       "rtp" : {
          "extensions" : [],
          "local_ssrc" : 7331,
+         "lntf" : {
+            "enabled": false,
+         },
          "nack" : {
             "rtp_history_ms" : 1000
          },

test/fuzzers/configs/replay_packet_fuzzer/vp8_fec_config.json

@@ -47,6 +47,9 @@
             }
          ],
          "local_ssrc" : 1,
+         "lntf" : {
+            "enabled": false,
+         },
          "nack" : {
             "rtp_history_ms" : 1000
          },

test/fuzzers/configs/replay_packet_fuzzer/vp9_config.json

@@ -10,6 +10,9 @@
       "rtp" : {
          "extensions" : [],
          "local_ssrc" : 7331,
+         "lntf" : {
+            "enabled": false,
+         },
          "nack" : {
             "rtp_history_ms" : 1000
          },

test/fuzzers/configs/replay_packet_fuzzer/vp9_fec_config.json

@@ -52,6 +52,9 @@
             }
          ],
          "local_ssrc" : 1,
+         "lntf" : {
+            "enabled": false,
+         },
          "nack" : {
             "rtp_history_ms" : 1000
          },

test/fuzzers/vp8_replay_fuzzer.cc

@@ -31,6 +31,7 @@ void FuzzOneInput(const uint8_t* data, size_t size) {
   vp8_config.rtp.transport_cc = true;
   vp8_config.rtp.remb = true;
   vp8_config.rtp.nack.rtp_history_ms = 1000;
+  vp8_config.rtp.lntf.enabled = true;
 
   std::vector<VideoReceiveStream::Config> replay_configs;
   replay_configs.push_back(std::move(vp8_config));

video/end_to_end_tests/config_tests.cc

@@ -52,6 +52,8 @@ void VerifyEmptyFlexfecConfig(const RtpConfig::Flexfec& config) {
 
 TEST_F(ConfigEndToEndTest, VerifyDefaultSendConfigParameters) {
   VideoSendStream::Config default_send_config(nullptr);
+  EXPECT_FALSE(default_send_config.rtp.lntf.enabled)
+      << "Enabling LNTF require rtcp-fb: goog-lntf negotiation.";
   EXPECT_EQ(0, default_send_config.rtp.nack.rtp_history_ms)
       << "Enabling NACK require rtcp-fb: nack negotiation.";
   EXPECT_TRUE(default_send_config.rtp.rtx.ssrcs.empty())
@@ -74,6 +76,8 @@ TEST_F(ConfigEndToEndTest, VerifyDefaultVideoReceiveConfigParameters) {
   VideoReceiveStream::Config default_receive_config(nullptr);
   EXPECT_EQ(RtcpMode::kCompound, default_receive_config.rtp.rtcp_mode)
       << "Reduced-size RTCP require rtcp-rsize to be negotiated.";
+  EXPECT_FALSE(default_receive_config.rtp.lntf.enabled)
+      << "Enabling LNTF require rtcp-fb: goog-lntf negotiation.";
   EXPECT_FALSE(default_receive_config.rtp.remb)
       << "REMB require rtcp-fb: goog-remb to be negotiated.";
   EXPECT_FALSE(

video/rtp_video_stream_receiver.cc

@@ -158,7 +158,8 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver(
 
   process_thread_->RegisterModule(rtp_rtcp_.get(), RTC_FROM_HERE);
 
-  if (webrtc::field_trial::IsEnabled("WebRTC-RtcpLossNotification")) {
+  // TODO(bugs.webrtc.org/10662): NACK and LNTF shouldn't be mutually exclusive.
+  if (config_.rtp.lntf.enabled) {
     loss_notification_controller_ =
         absl::make_unique<LossNotificationController>(this, this);
   } else if (config_.rtp.nack.rtp_history_ms != 0) {
@@ -395,6 +396,7 @@ void RtpVideoStreamReceiver::SendLossNotification(
     uint16_t last_decoded_seq_num,
     uint16_t last_received_seq_num,
     bool decodability_flag) {
+  RTC_DCHECK(config_.rtp.lntf.enabled);
   rtp_rtcp_->SendLossNotification(last_decoded_seq_num, last_received_seq_num,
                                   decodability_flag);
 }