Remove NackSender argument from RtpVideoStreamReceiver2.

Bug: webrtc:14249
Change-Id: Ic6013c69da2d0f1345f688660521ea0c175ad896
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/267840
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37496}

video/rtp_video_stream_receiver2.cc

@@ -213,7 +213,6 @@ RtpVideoStreamReceiver2::RtpVideoStreamReceiver2(
     RtcpPacketTypeCounterObserver* rtcp_packet_type_counter_observer,
     RtcpCnameCallback* rtcp_cname_callback,
     NackPeriodicProcessor* nack_periodic_processor,
-    NackSender* nack_sender,
     OnCompleteFrameCallback* complete_frame_callback,
     rtc::scoped_refptr<FrameDecryptorInterface> frame_decryptor,
     rtc::scoped_refptr<FrameTransformerInterface> frame_transformer,
@@ -245,7 +244,7 @@ RtpVideoStreamReceiver2::RtpVideoStreamReceiver2(
       keyframe_request_method_(config_.rtp.keyframe_method),
       // TODO(bugs.webrtc.org/10336): Let `rtcp_feedback_buffer_` communicate
       // directly with `rtp_rtcp_`.
-      rtcp_feedback_buffer_(this, nack_sender, this),
+      rtcp_feedback_buffer_(this, this, this),
       nack_module_(MaybeConstructNackModule(current_queue,
                                             nack_periodic_processor,
                                             config_,
@@ -689,6 +688,12 @@ void RtpVideoStreamReceiver2::RequestKeyFrame() {
   }
 }
 
+void RtpVideoStreamReceiver2::SendNack(
+    const std::vector<uint16_t>& sequence_numbers,
+    bool /*buffering_allowed*/) {
+  rtp_rtcp_->SendNack(sequence_numbers);
+}
+
 void RtpVideoStreamReceiver2::SendLossNotification(
     uint16_t last_decoded_seq_num,
     uint16_t last_received_seq_num,
@@ -707,12 +712,6 @@ bool RtpVideoStreamReceiver2::IsRetransmissionsEnabled() const {
   return config_.rtp.nack.rtp_history_ms > 0;
 }
 
-void RtpVideoStreamReceiver2::RequestPacketRetransmit(
-    const std::vector<uint16_t>& sequence_numbers) {
-  RTC_DCHECK_RUN_ON(&worker_task_checker_);
-  rtp_rtcp_->SendNack(sequence_numbers);
-}
-
 bool RtpVideoStreamReceiver2::IsDecryptable() const {
   RTC_DCHECK_RUN_ON(&worker_task_checker_);
   return frames_decryptable_;

video/rtp_video_stream_receiver2.h

@@ -64,6 +64,7 @@ class RtpVideoStreamReceiver2 : public LossNotificationSender,
                                 public RecoveredPacketReceiver,
                                 public RtpPacketSinkInterface,
                                 public KeyFrameRequestSender,
+                                public NackSender,
                                 public OnDecryptedFrameCallback,
                                 public OnDecryptionStatusChangeCallback,
                                 public RtpVideoFrameReceiver {
@@ -90,7 +91,6 @@ class RtpVideoStreamReceiver2 : public LossNotificationSender,
       RtcpPacketTypeCounterObserver* rtcp_packet_type_counter_observer,
       RtcpCnameCallback* rtcp_cname_callback,
       NackPeriodicProcessor* nack_periodic_processor,
-      NackSender* nack_sender,
       // The KeyFrameRequestSender is optional; if not provided, key frame
       // requests are sent via the internal RtpRtcp module.
       OnCompleteFrameCallback* complete_frame_callback,
@@ -138,6 +138,10 @@ class RtpVideoStreamReceiver2 : public LossNotificationSender,
   // Send an RTCP keyframe request.
   void RequestKeyFrame() override;
 
+  // Implements NackSender.
+  void SendNack(const std::vector<uint16_t>& sequence_numbers,
+                bool buffering_allowed) override;
+
   // Implements LossNotificationSender.
   void SendLossNotification(uint16_t last_decoded_seq_num,
                             uint16_t last_received_seq_num,
@@ -152,12 +156,6 @@ class RtpVideoStreamReceiver2 : public LossNotificationSender,
   // Decryption not SRTP.
   bool IsDecryptable() const;
 
-  // Request packet retransmits via NACK. Called via
-  // VideoReceiveStream2::SendNack, which gets called when
-  // RtpVideoStreamReceiver2::RtcpFeedbackBuffer's SendNack and
-  // SendBufferedRtcpFeedback methods (see `rtcp_feedback_buffer_` below).
-  void RequestPacketRetransmit(const std::vector<uint16_t>& sequence_numbers);
-
   // Implements OnDecryptedFrameCallback.
   void OnDecryptedFrame(std::unique_ptr<RtpFrameObject> frame) override;
 

video/rtp_video_stream_receiver2_unittest.cc

@@ -160,8 +160,8 @@ class RtpVideoStreamReceiver2Test : public ::testing::Test,
     rtp_video_stream_receiver_ = std::make_unique<RtpVideoStreamReceiver2>(
         TaskQueueBase::Current(), Clock::GetRealTimeClock(), &mock_transport_,
         nullptr, nullptr, &config_, rtp_receive_statistics_.get(), nullptr,
-        nullptr, &nack_periodic_processor_, &mock_nack_sender_,
+        nullptr, &nack_periodic_processor_, &mock_on_complete_frame_callback_,
-        &mock_on_complete_frame_callback_, nullptr, nullptr, field_trials_);
+        nullptr, nullptr, field_trials_);
     rtp_video_stream_receiver_->AddReceiveCodec(kPayloadType,
                                                 kVideoCodecGeneric, {},
                                                 /*raw_payload=*/false);
@@ -235,7 +235,6 @@ class RtpVideoStreamReceiver2Test : public ::testing::Test,
   webrtc::test::ScopedKeyValueConfig field_trials_;
   VideoReceiveStreamInterface::Config config_;
   NackPeriodicProcessor nack_periodic_processor_;
-  MockNackSender mock_nack_sender_;
   test::RtcpPacketParser rtcp_packet_parser_;
   MockTransport mock_transport_;
   MockOnCompleteFrameCallback mock_on_complete_frame_callback_;
@@ -1143,9 +1142,8 @@ TEST_F(RtpVideoStreamReceiver2Test, TransformFrame) {
   auto receiver = std::make_unique<RtpVideoStreamReceiver2>(
       TaskQueueBase::Current(), Clock::GetRealTimeClock(), &mock_transport_,
       nullptr, nullptr, &config_, rtp_receive_statistics_.get(), nullptr,
-      nullptr, &nack_periodic_processor_, &mock_nack_sender_,
+      nullptr, &nack_periodic_processor_, &mock_on_complete_frame_callback_,
-      &mock_on_complete_frame_callback_, nullptr, mock_frame_transformer,
+      nullptr, mock_frame_transformer, field_trials_);
-      field_trials_);
   receiver->AddReceiveCodec(kPayloadType, kVideoCodecGeneric, {},
                             /*raw_payload=*/false);
 

video/video_receive_stream2.cc

@@ -234,7 +234,6 @@ VideoReceiveStream2::VideoReceiveStream2(
                                  &stats_proxy_,
                                  &stats_proxy_,
                                  nack_periodic_processor,
-                                 this,  // NackSender
                                  this,  // OnCompleteFrameCallback
                                  std::move(config_.frame_decryptor),
                                  std::move(config_.frame_transformer),
@@ -653,14 +652,6 @@ void VideoReceiveStream2::SetDepacketizerToDecoderFrameTransformer(
       std::move(frame_transformer));
 }
 
-void VideoReceiveStream2::SendNack(
-    const std::vector<uint16_t>& sequence_numbers,
-    bool buffering_allowed) {
-  RTC_DCHECK_RUN_ON(&worker_sequence_checker_);
-  RTC_DCHECK(buffering_allowed);
-  rtp_video_stream_receiver_.RequestPacketRetransmit(sequence_numbers);
-}
-
 void VideoReceiveStream2::RequestKeyFrame(Timestamp now) {
   // Running on worker_sequence_checker_.
   // Called from RtpVideoStreamReceiver (rtp_video_stream_receiver_ is

video/video_receive_stream2.h

@@ -84,7 +84,6 @@ struct VideoFrameMetaData {
 class VideoReceiveStream2
     : public webrtc::VideoReceiveStreamInterface,
       public rtc::VideoSinkInterface<VideoFrame>,
-      public NackSender,
       public RtpVideoStreamReceiver2::OnCompleteFrameCallback,
       public Syncable,
       public CallStatsObserver,
@@ -163,12 +162,6 @@ class VideoReceiveStream2
   // Implements rtc::VideoSinkInterface<VideoFrame>.
   void OnFrame(const VideoFrame& video_frame) override;
 
-  // Implements NackSender.
-  // For this particular override of the interface,
-  // only (buffering_allowed == true) is acceptable.
-  void SendNack(const std::vector<uint16_t>& sequence_numbers,
-                bool buffering_allowed) override;
-
   // Implements RtpVideoStreamReceiver2::OnCompleteFrameCallback.
   void OnCompleteFrame(std::unique_ptr<EncodedFrame> frame) override;