From b981394841714ac98d775c8eb90b92a9a8bf554c Mon Sep 17 00:00:00 2001 From: philipel Date: Tue, 5 Jul 2022 11:31:36 +0200 Subject: [PATCH] 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 Commit-Queue: Philip Eliasson Cr-Commit-Position: refs/heads/main@{#37496} --- video/rtp_video_stream_receiver2.cc | 15 +++++++-------- video/rtp_video_stream_receiver2.h | 12 +++++------- video/rtp_video_stream_receiver2_unittest.cc | 10 ++++------ video/video_receive_stream2.cc | 9 --------- video/video_receive_stream2.h | 7 ------- 5 files changed, 16 insertions(+), 37 deletions(-) diff --git a/video/rtp_video_stream_receiver2.cc b/video/rtp_video_stream_receiver2.cc index afc51fbea2..6cd2083b5b 100644 --- a/video/rtp_video_stream_receiver2.cc +++ b/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 frame_decryptor, rtc::scoped_refptr 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& 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& 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_; diff --git a/video/rtp_video_stream_receiver2.h b/video/rtp_video_stream_receiver2.h index d0db53c635..b47e23978a 100644 --- a/video/rtp_video_stream_receiver2.h +++ b/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& 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& sequence_numbers); - // Implements OnDecryptedFrameCallback. void OnDecryptedFrame(std::unique_ptr frame) override; diff --git a/video/rtp_video_stream_receiver2_unittest.cc b/video/rtp_video_stream_receiver2_unittest.cc index 21803baaf9..4eb0406db6 100644 --- a/video/rtp_video_stream_receiver2_unittest.cc +++ b/video/rtp_video_stream_receiver2_unittest.cc @@ -160,8 +160,8 @@ class RtpVideoStreamReceiver2Test : public ::testing::Test, rtp_video_stream_receiver_ = std::make_unique( TaskQueueBase::Current(), Clock::GetRealTimeClock(), &mock_transport_, nullptr, nullptr, &config_, rtp_receive_statistics_.get(), nullptr, - nullptr, &nack_periodic_processor_, &mock_nack_sender_, - &mock_on_complete_frame_callback_, nullptr, nullptr, field_trials_); + nullptr, &nack_periodic_processor_, &mock_on_complete_frame_callback_, + 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( TaskQueueBase::Current(), Clock::GetRealTimeClock(), &mock_transport_, nullptr, nullptr, &config_, rtp_receive_statistics_.get(), nullptr, - nullptr, &nack_periodic_processor_, &mock_nack_sender_, - &mock_on_complete_frame_callback_, nullptr, mock_frame_transformer, - field_trials_); + nullptr, &nack_periodic_processor_, &mock_on_complete_frame_callback_, + nullptr, mock_frame_transformer, field_trials_); receiver->AddReceiveCodec(kPayloadType, kVideoCodecGeneric, {}, /*raw_payload=*/false); diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc index d3e882184d..f0dc245c94 100644 --- a/video/video_receive_stream2.cc +++ b/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& 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 diff --git a/video/video_receive_stream2.h b/video/video_receive_stream2.h index 06978b45b6..e327302d88 100644 --- a/video/video_receive_stream2.h +++ b/video/video_receive_stream2.h @@ -84,7 +84,6 @@ struct VideoFrameMetaData { class VideoReceiveStream2 : public webrtc::VideoReceiveStreamInterface, public rtc::VideoSinkInterface, - public NackSender, public RtpVideoStreamReceiver2::OnCompleteFrameCallback, public Syncable, public CallStatsObserver, @@ -163,12 +162,6 @@ class VideoReceiveStream2 // Implements rtc::VideoSinkInterface. 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& sequence_numbers, - bool buffering_allowed) override; - // Implements RtpVideoStreamReceiver2::OnCompleteFrameCallback. void OnCompleteFrame(std::unique_ptr frame) override;