Video and flexfec receive stream config changes without recreate.

SetFeedbackParameters no longer recreates the embedded streams for:
- transport cc flag
- rtcp status

Bug: none
Change-Id: If6117a1ae760ca9a02f06bbfa2b46c6e0f448cfc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/268281
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37526}

audio/audio_receive_stream.cc

@@ -215,7 +215,7 @@ bool AudioReceiveStreamImpl::transport_cc() const {
 }
 
 void AudioReceiveStreamImpl::SetTransportCc(bool transport_cc) {
-  RTC_DCHECK_RUN_ON(&worker_thread_checker_);
+  RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
   config_.rtp.transport_cc = transport_cc;
 }
 

call/flexfec_receive_stream.h

@@ -59,6 +59,12 @@ class FlexfecReceiveStream : public RtpPacketSinkInterface,
     // Transport for outgoing RTCP packets.
     Transport* rtcp_send_transport = nullptr;
   };
+
+  // TODO(tommi): FlexfecReceiveStream inherits from ReceiveStreamInterface,
+  // not VideoReceiveStreamInterface where there's also a SetRtcpMode method.
+  // Perhaps this should be in ReceiveStreamInterface and apply to audio streams
+  // as well (although there's no logic that would use it at present).
+  virtual void SetRtcpMode(RtcpMode mode) = 0;
 };
 
 }  // namespace webrtc

call/flexfec_receive_stream_impl.h

@@ -75,6 +75,11 @@ class FlexfecReceiveStreamImpl : public FlexfecReceiveStream {
     transport_cc_ = transport_cc;
   }
 
+  void SetRtcpMode(RtcpMode mode) override {
+    RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
+    rtp_rtcp_->SetRTCPStatus(mode);
+  }
+
  private:
   RTC_NO_UNIQUE_ADDRESS SequenceChecker packet_sequence_checker_;
 

call/video_receive_stream.h

@@ -285,6 +285,8 @@ class VideoReceiveStreamInterface : public MediaReceiveStreamInterface {
   // Cause eventual generation of a key frame from the sender.
   virtual void GenerateKeyFrame() = 0;
 
+  virtual void SetRtcpMode(RtcpMode mode) = 0;
+
  protected:
   virtual ~VideoReceiveStreamInterface() {}
 };

media/engine/fake_webrtc_call.h

@@ -280,6 +280,9 @@ class FakeVideoReceiveStream final
   void SetTransportCc(bool transport_cc) override {
     config_.rtp.transport_cc = transport_cc;
   }
+  void SetRtcpMode(webrtc::RtcpMode mode) override {
+    config_.rtp.rtcp_mode = mode;
+  }
 
   void Start() override;
   void Stop() override;
@@ -318,6 +321,7 @@ class FakeFlexfecReceiveStream final : public webrtc::FlexfecReceiveStream {
   void SetTransportCc(bool transport_cc) override {
     config_.rtp.transport_cc = transport_cc;
   }
+  void SetRtcpMode(webrtc::RtcpMode mode) override { config_.rtcp_mode = mode; }
 
   const webrtc::FlexfecReceiveStream::Config& GetConfig() const;
 

media/engine/webrtc_video_engine.cc

@@ -2997,31 +2997,47 @@ void WebRtcVideoChannel::WebRtcVideoReceiveStream::SetFeedbackParameters(
     bool transport_cc_enabled,
     webrtc::RtcpMode rtcp_mode,
     int rtx_time) {
+  RTC_DCHECK(stream_);
+
+  if (config_.rtp.rtcp_mode != rtcp_mode) {
+    config_.rtp.rtcp_mode = rtcp_mode;
+    stream_->SetRtcpMode(rtcp_mode);
+
+    flexfec_config_.rtcp_mode = rtcp_mode;
+    if (flexfec_stream_) {
+      flexfec_stream_->SetRtcpMode(rtcp_mode);
+    }
+  }
+
+  if (config_.rtp.transport_cc != transport_cc_enabled) {
+    config_.rtp.transport_cc = transport_cc_enabled;
+    stream_->SetTransportCc(transport_cc_enabled);
+    // TODO(brandtr): We should be spec-compliant and set `transport_cc` here
+    // based on the rtcp-fb for the FlexFEC codec, not the media codec.
+    flexfec_config_.rtp.transport_cc = transport_cc_enabled;
+    if (flexfec_stream_) {
+      flexfec_stream_->SetTransportCc(transport_cc_enabled);
+    }
+  }
+
   int nack_history_ms =
       nack_enabled ? rtx_time != -1 ? rtx_time : kNackHistoryMs : 0;
   if (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) {
+      config_.rtp.nack.rtp_history_ms == nack_history_ms) {
     RTC_LOG(LS_INFO)
         << "Ignoring call to SetFeedbackParameters because parameters are "
            "unchanged; lntf="
         << lntf_enabled << ", nack=" << nack_enabled
-        << ", transport_cc=" << transport_cc_enabled
         << ", rtx_time=" << rtx_time;
     return;
   }
+
   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;
-  // TODO(brandtr): We should be spec-compliant and set `transport_cc` here
-  // based on the rtcp-fb for the FlexFEC codec, not the media codec.
-  flexfec_config_.rtp.transport_cc = config_.rtp.transport_cc;
-  flexfec_config_.rtcp_mode = config_.rtp.rtcp_mode;
+
   RTC_LOG(LS_INFO) << "RecreateReceiveStream (recv) because of "
                       "SetFeedbackParameters; nack="
-                   << nack_enabled << ", transport_cc=" << transport_cc_enabled;
+                   << nack_enabled;
   RecreateReceiveStream();
 }
 

video/rtp_video_stream_receiver2.cc

@@ -933,6 +933,11 @@ void RtpVideoStreamReceiver2::OnLocalSsrcChange(uint32_t local_ssrc) {
   rtp_rtcp_->SetLocalSsrc(local_ssrc);
 }
 
+void RtpVideoStreamReceiver2::SetRtcpMode(RtcpMode mode) {
+  RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
+  rtp_rtcp_->SetRTCPStatus(mode);
+}
+
 absl::optional<int64_t> RtpVideoStreamReceiver2::LastReceivedPacketMs() const {
   RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
   if (last_received_rtp_system_time_) {

video/rtp_video_stream_receiver2.h

@@ -184,6 +184,9 @@ class RtpVideoStreamReceiver2 : public LossNotificationSender,
   // Called when the local_ssrc is changed to match with a sender.
   void OnLocalSsrcChange(uint32_t local_ssrc);
 
+  // Forwards the call to set rtcp_sender_ to the RTCP mode of the rtcp sender.
+  void SetRtcpMode(RtcpMode mode);
+
   absl::optional<int64_t> LastReceivedPacketMs() const;
   absl::optional<int64_t> LastReceivedKeyframePacketMs() const;
 

video/video_receive_stream2.cc

@@ -501,6 +501,13 @@ void VideoReceiveStream2::SetTransportCc(bool transport_cc) {
   const_cast<bool&>(config_.rtp.transport_cc) = transport_cc;
 }
 
+void VideoReceiveStream2::SetRtcpMode(RtcpMode mode) {
+  RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
+  // TODO(tommi): Stop using the config struct for the internal state.
+  const_cast<RtcpMode&>(config_.rtp.rtcp_mode) = mode;
+  rtp_video_stream_receiver_.SetRtcpMode(mode);
+}
+
 void VideoReceiveStream2::CreateAndRegisterExternalDecoder(
     const Decoder& decoder) {
   TRACE_EVENT0("webrtc",

video/video_receive_stream2.h

@@ -145,6 +145,7 @@ class VideoReceiveStream2
   RtpHeaderExtensionMap GetRtpExtensionMap() const override;
   bool transport_cc() const override;
   void SetTransportCc(bool transport_cc) override;
+  void SetRtcpMode(RtcpMode mode) override;
 
   webrtc::VideoReceiveStreamInterface::Stats GetStats() const override;