Add SetTransportCc to ReceiveStreamInterface.

Setting the transport cc flag was only possible post creation for
audio receive streams, while video receive streams need to be recreated.

This CL moves the setter for transport_cc() to where the getter is and
adds boiler plate implementations for the video streams. For audio
streams this splits "SetUseTransportCcAndNackHistory" into two methods,
SetTransportCc and SetNackHistory.

Bug: none
Change-Id: Idbec8217aef10ee77907cebaecdc27b4b0fb18e4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264443
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37038}

audio/audio_receive_stream.cc

@@ -214,6 +214,11 @@ bool AudioReceiveStreamImpl::transport_cc() const {
   return config_.rtp.transport_cc;
 }
 
+void AudioReceiveStreamImpl::SetTransportCc(bool transport_cc) {
+  RTC_DCHECK_RUN_ON(&worker_thread_checker_);
+  config_.rtp.transport_cc = transport_cc;
+}
+
 bool AudioReceiveStreamImpl::IsRunning() const {
   RTC_DCHECK_RUN_ON(&worker_thread_checker_);
   return playing_;
@@ -233,18 +238,17 @@ void AudioReceiveStreamImpl::SetDecoderMap(
   channel_receive_->SetReceiveCodecs(config_.decoder_map);
 }
 
-void AudioReceiveStreamImpl::SetUseTransportCcAndNackHistory(
+void AudioReceiveStreamImpl::SetNackHistory(int history_ms) {
-    bool use_transport_cc,
-    int history_ms) {
   RTC_DCHECK_RUN_ON(&worker_thread_checker_);
   RTC_DCHECK_GE(history_ms, 0);
-  config_.rtp.transport_cc = use_transport_cc;
+
-  if (config_.rtp.nack.rtp_history_ms != history_ms) {
+  if (config_.rtp.nack.rtp_history_ms == history_ms)
+    return;
+
   config_.rtp.nack.rtp_history_ms = history_ms;
   // TODO(solenberg): Config NACK history window (which is a packet count),
   // using the actual packet size for the configured codec.
   channel_receive_->SetNACKStatus(history_ms != 0, history_ms / 20);
-  }
 }
 
 void AudioReceiveStreamImpl::SetNonSenderRttMeasurement(bool enabled) {

audio/audio_receive_stream.h

@@ -86,13 +86,13 @@ class AudioReceiveStreamImpl final : public webrtc::AudioReceiveStreamInterface,
   void Start() override;
   void Stop() override;
   bool transport_cc() const override;
+  void SetTransportCc(bool transport_cc) override;
   bool IsRunning() const override;
   void SetDepacketizerToDecoderFrameTransformer(
       rtc::scoped_refptr<webrtc::FrameTransformerInterface> frame_transformer)
       override;
   void SetDecoderMap(std::map<int, SdpAudioFormat> decoder_map) override;
-  void SetUseTransportCcAndNackHistory(bool use_transport_cc,
+  void SetNackHistory(int history_ms) override;
-                                       int history_ms) override;
   void SetNonSenderRttMeasurement(bool enabled) override;
   void SetFrameDecryptor(rtc::scoped_refptr<webrtc::FrameDecryptorInterface>
                              frame_decryptor) override;

audio/audio_receive_stream_unittest.cc

@@ -399,8 +399,8 @@ TEST(AudioReceiveStreamTest, ReconfigureWithUpdatedConfig) {
     recv_stream->SetDecoderMap(new_config.decoder_map);
 
     EXPECT_CALL(channel_receive, SetNACKStatus(true, 15 + 1)).Times(1);
-    recv_stream->SetUseTransportCcAndNackHistory(new_config.rtp.transport_cc,
+    recv_stream->SetTransportCc(new_config.rtp.transport_cc);
-                                                 300 + 20);
+    recv_stream->SetNackHistory(300 + 20);
 
     recv_stream->UnregisterFromTransport();
   }

call/audio_receive_stream.h

@@ -161,8 +161,7 @@ class AudioReceiveStreamInterface : public MediaReceiveStreamInterface {
 
   // Methods that support reconfiguring the stream post initialization.
   virtual void SetDecoderMap(std::map<int, SdpAudioFormat> decoder_map) = 0;
-  virtual void SetUseTransportCcAndNackHistory(bool use_transport_cc,
+  virtual void SetNackHistory(int history_ms) = 0;
-                                               int history_ms) = 0;
   virtual void SetNonSenderRttMeasurement(bool enabled) = 0;
 
   // Returns true if the stream has been started.

call/flexfec_receive_stream_impl.h

@@ -64,11 +64,17 @@ class FlexfecReceiveStreamImpl : public FlexfecReceiveStream {
   void SetLocalSsrc(uint32_t local_ssrc);
 
   uint32_t remote_ssrc() const { return remote_ssrc_; }
+
   bool transport_cc() const override {
     RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
     return transport_cc_;
   }
 
+  void SetTransportCc(bool transport_cc) override {
+    RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
+    transport_cc_ = transport_cc;
+  }
+
  private:
   RTC_NO_UNIQUE_ADDRESS SequenceChecker packet_sequence_checker_;
 

call/receive_stream.h

@@ -67,6 +67,8 @@ class ReceiveStreamInterface {
   // that the value is read on (i.e. packet delivery).
   virtual bool transport_cc() const = 0;
 
+  virtual void SetTransportCc(bool transport_cc) = 0;
+
  protected:
   virtual ~ReceiveStreamInterface() {}
 };

media/engine/fake_webrtc_call.cc

@@ -110,10 +110,7 @@ void FakeAudioReceiveStream::SetDecoderMap(
   config_.decoder_map = std::move(decoder_map);
 }
 
-void FakeAudioReceiveStream::SetUseTransportCcAndNackHistory(
+void FakeAudioReceiveStream::SetNackHistory(int history_ms) {
-    bool use_transport_cc,
-    int history_ms) {
-  config_.rtp.transport_cc = use_transport_cc;
   config_.rtp.nack.rtp_history_ms = history_ms;
 }
 

media/engine/fake_webrtc_call.h

@@ -111,8 +111,10 @@ class FakeAudioReceiveStream final
     config_.sync_group = std::string(sync_group);
   }
 
- private:
   bool transport_cc() const override { return config_.rtp.transport_cc; }
+  void SetTransportCc(bool transport_cc) override {
+    config_.rtp.transport_cc = transport_cc;
+  }
   uint32_t remote_ssrc() const override { return config_.rtp.remote_ssrc; }
   void Start() override { started_ = true; }
   void Stop() override { started_ = false; }
@@ -122,8 +124,7 @@ class FakeAudioReceiveStream final
       override;
   void SetDecoderMap(
       std::map<int, webrtc::SdpAudioFormat> decoder_map) override;
-  void SetUseTransportCcAndNackHistory(bool use_transport_cc,
+  void SetNackHistory(int history_ms) override;
-                                       int history_ms) override;
   void SetNonSenderRttMeasurement(bool enabled) override;
   void SetFrameDecryptor(rtc::scoped_refptr<webrtc::FrameDecryptorInterface>
                              frame_decryptor) override;
@@ -146,6 +147,7 @@ class FakeAudioReceiveStream final
     return std::vector<webrtc::RtpSource>();
   }
 
+ private:
   int id_ = -1;
   webrtc::AudioReceiveStreamInterface::Config config_;
   webrtc::AudioReceiveStreamInterface::Stats stats_;
@@ -271,11 +273,13 @@ class FakeVideoReceiveStream final
   }
   void GenerateKeyFrame() override {}
 
- private:
   // webrtc::VideoReceiveStreamInterface implementation.
   void SetRtpExtensions(std::vector<webrtc::RtpExtension> extensions) override;
   webrtc::RtpHeaderExtensionMap GetRtpExtensionMap() const override;
   bool transport_cc() const override { return config_.rtp.transport_cc; }
+  void SetTransportCc(bool transport_cc) override {
+    config_.rtp.transport_cc = transport_cc;
+  }
 
   void Start() override;
   void Stop() override;
@@ -291,6 +295,7 @@ class FakeVideoReceiveStream final
     return base_mininum_playout_delay_ms_;
   }
 
+ private:
   webrtc::VideoReceiveStreamInterface::Config config_;
   bool receiving_;
   webrtc::VideoReceiveStreamInterface::Stats stats_;
@@ -310,6 +315,9 @@ class FakeFlexfecReceiveStream final : public webrtc::FlexfecReceiveStream {
   void SetRtpExtensions(std::vector<webrtc::RtpExtension> extensions) override;
   webrtc::RtpHeaderExtensionMap GetRtpExtensionMap() const override;
   bool transport_cc() const override { return config_.rtp.transport_cc; }
+  void SetTransportCc(bool transport_cc) override {
+    config_.rtp.transport_cc = transport_cc;
+  }
 
   const webrtc::FlexfecReceiveStream::Config& GetConfig() const;
 

media/engine/webrtc_video_engine_unittest.cc

@@ -3140,12 +3140,12 @@ TEST_F(WebRtcVideoChannelTest, RtcpIsCompoundByDefault) {
 
 TEST_F(WebRtcVideoChannelTest, TransportCcIsEnabledByDefault) {
   FakeVideoReceiveStream* stream = AddRecvStream();
-  EXPECT_TRUE(stream->GetConfig().rtp.transport_cc);
+  EXPECT_TRUE(stream->transport_cc());
 }
 
 TEST_F(WebRtcVideoChannelTest, TransportCcCanBeEnabledAndDisabled) {
   FakeVideoReceiveStream* stream = AddRecvStream();
-  EXPECT_TRUE(stream->GetConfig().rtp.transport_cc);
+  EXPECT_TRUE(stream->transport_cc());
 
   // Verify that transport cc feedback is turned off when send(!) codecs without
   // transport cc feedback are set.
@@ -3154,14 +3154,14 @@ TEST_F(WebRtcVideoChannelTest, TransportCcCanBeEnabledAndDisabled) {
   EXPECT_TRUE(parameters.codecs[0].feedback_params.params().empty());
   EXPECT_TRUE(channel_->SetSendParameters(parameters));
   stream = fake_call_->GetVideoReceiveStreams()[0];
-  EXPECT_FALSE(stream->GetConfig().rtp.transport_cc);
+  EXPECT_FALSE(stream->transport_cc());
 
   // Verify that transport cc feedback is turned on when setting default codecs
   // since the default codecs have transport cc feedback enabled.
   parameters.codecs = engine_.send_codecs();
   EXPECT_TRUE(channel_->SetSendParameters(parameters));
   stream = fake_call_->GetVideoReceiveStreams()[0];
-  EXPECT_TRUE(stream->GetConfig().rtp.transport_cc);
+  EXPECT_TRUE(stream->transport_cc());
 }
 
 TEST_F(WebRtcVideoChannelTest, LossNotificationIsDisabledByDefault) {

media/engine/webrtc_voice_engine.cc

@@ -1180,8 +1180,8 @@ class WebRtcVoiceMediaChannel::WebRtcAudioReceiveStream {
 
   void SetUseTransportCc(bool use_transport_cc, bool use_nack) {
     RTC_DCHECK_RUN_ON(&worker_thread_checker_);
-    stream_->SetUseTransportCcAndNackHistory(use_transport_cc,
+    stream_->SetTransportCc(use_transport_cc);
-                                             use_nack ? kNackRtpHistoryMs : 0);
+    stream_->SetNackHistory(use_nack ? kNackRtpHistoryMs : 0);
   }
 
   void SetNonSenderRttMeasurement(bool enabled) {

media/engine/webrtc_voice_engine_unittest.cc

@@ -1865,14 +1865,12 @@ TEST_P(WebRtcVoiceEngineTestFake, TransportCcCanBeEnabledAndDisabled) {
   EXPECT_TRUE(channel_->SetRecvParameters(recv_parameters));
   EXPECT_TRUE(AddRecvStream(kSsrcX));
   ASSERT_TRUE(call_.GetAudioReceiveStream(kSsrcX) != nullptr);
-  EXPECT_FALSE(
+  EXPECT_FALSE(call_.GetAudioReceiveStream(kSsrcX)->transport_cc());
-      call_.GetAudioReceiveStream(kSsrcX)->GetConfig().rtp.transport_cc);
 
   send_parameters.codecs = engine_->send_codecs();
   SetSendParameters(send_parameters);
   ASSERT_TRUE(call_.GetAudioReceiveStream(kSsrcX) != nullptr);
-  EXPECT_TRUE(
+  EXPECT_TRUE(call_.GetAudioReceiveStream(kSsrcX)->transport_cc());
-      call_.GetAudioReceiveStream(kSsrcX)->GetConfig().rtp.transport_cc);
 }
 
 // Test that we can switch back and forth between Opus and ISAC with CN.

video/video_receive_stream2.cc

@@ -498,6 +498,17 @@ RtpHeaderExtensionMap VideoReceiveStream2::GetRtpExtensionMap() const {
   return rtp_video_stream_receiver_.GetRtpExtensions();
 }
 
+bool VideoReceiveStream2::transport_cc() const {
+  RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
+  return config_.rtp.transport_cc;
+}
+
+void VideoReceiveStream2::SetTransportCc(bool transport_cc) {
+  RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
+  // TODO(tommi): Stop using the config struct for the internal state.
+  const_cast<bool&>(config_.rtp.transport_cc) = transport_cc;
+}
+
 void VideoReceiveStream2::CreateAndRegisterExternalDecoder(
     const Decoder& decoder) {
   TRACE_EVENT0("webrtc",

video/video_receive_stream2.h

@@ -141,7 +141,8 @@ class VideoReceiveStream2
 
   void SetRtpExtensions(std::vector<RtpExtension> extensions) override;
   RtpHeaderExtensionMap GetRtpExtensionMap() const override;
-  bool transport_cc() const override { return config_.rtp.transport_cc; }
+  bool transport_cc() const override;
+  void SetTransportCc(bool transport_cc) override;
 
   webrtc::VideoReceiveStreamInterface::Stats GetStats() const override;