From 479c05506ea85750c841c261b0032ce73efc1030 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Thu, 23 May 2019 16:57:01 +0200 Subject: [PATCH] Let RtpVideoStreamReceiver implement KeyFrameRequestSender MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: None Change-Id: I02c89aa169b63ddb6e9ec289c783f3e85d07841e Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/130101 Reviewed-by: Erik Språng Commit-Queue: Niels Moller Cr-Commit-Position: refs/heads/master@{#28053} --- video/rtp_video_stream_receiver.cc | 44 +++++++++++++++------ video/rtp_video_stream_receiver.h | 22 +++++++++-- video/rtp_video_stream_receiver_unittest.cc | 16 ++++++-- video/video_receive_stream.cc | 1 - video/video_receive_stream.h | 5 +-- 5 files changed, 63 insertions(+), 25 deletions(-) diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc index e12bd5c49e..0f38f19424 100644 --- a/video/rtp_video_stream_receiver.cc +++ b/video/rtp_video_stream_receiver.cc @@ -85,7 +85,32 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver( ReceiveStatisticsProxy* receive_stats_proxy, ProcessThread* process_thread, NackSender* nack_sender, - KeyFrameRequestSender* keyframe_request_sender, + video_coding::OnCompleteFrameCallback* complete_frame_callback, + rtc::scoped_refptr frame_decryptor) + : RtpVideoStreamReceiver(clock, + CreateRtpRtcpModule(clock, + rtp_receive_statistics, + transport, + rtt_stats, + receive_stats_proxy), + packet_router, + config, + rtp_receive_statistics, + receive_stats_proxy, + process_thread, + nack_sender, + complete_frame_callback, + frame_decryptor) {} + +RtpVideoStreamReceiver::RtpVideoStreamReceiver( + Clock* clock, + std::unique_ptr rtp_rtcp, + PacketRouter* packet_router, + const VideoReceiveStream::Config* config, + ReceiveStatistics* rtp_receive_statistics, + ReceiveStatisticsProxy* receive_stats_proxy, + ProcessThread* process_thread, + NackSender* nack_sender, video_coding::OnCompleteFrameCallback* complete_frame_callback, rtc::scoped_refptr frame_decryptor) : clock_(clock), @@ -98,13 +123,8 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver( ulpfec_receiver_(UlpfecReceiver::Create(config->rtp.remote_ssrc, this)), receiving_(false), last_packet_log_ms_(-1), - rtp_rtcp_(CreateRtpRtcpModule(clock, - rtp_receive_statistics_, - transport, - rtt_stats, - receive_stats_proxy)), + rtp_rtcp_(std::move(rtp_rtcp)), complete_frame_callback_(complete_frame_callback), - keyframe_request_sender_(keyframe_request_sender), has_received_frame_(false), frames_decryptable_(false) { constexpr bool remb_candidate = true; @@ -140,11 +160,9 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver( if (webrtc::field_trial::IsEnabled("WebRTC-RtcpLossNotification")) { loss_notification_controller_ = - absl::make_unique(keyframe_request_sender_, - this); + absl::make_unique(this, this); } else if (config_.rtp.nack.rtp_history_ms != 0) { - nack_module_ = absl::make_unique(clock_, nack_sender, - keyframe_request_sender); + nack_module_ = absl::make_unique(clock_, nack_sender, this); process_thread_->RegisterModule(nack_module_.get(), RTC_FROM_HERE); } @@ -273,7 +291,7 @@ int32_t RtpVideoStreamReceiver::OnReceivedPayloadData( switch (tracker_.CopyAndFixBitstream(&packet)) { case video_coding::H264SpsPpsTracker::kRequestKeyframe: - keyframe_request_sender_->RequestKeyFrame(); + RequestKeyFrame(); RTC_FALLTHROUGH(); case video_coding::H264SpsPpsTracker::kDrop: return 0; @@ -414,7 +432,7 @@ void RtpVideoStreamReceiver::OnAssembledFrame( } else if (!has_received_frame_) { // Request a key frame as soon as possible. if (frame->FrameType() != VideoFrameType::kVideoFrameKey) { - keyframe_request_sender_->RequestKeyFrame(); + RequestKeyFrame(); } } diff --git a/video/rtp_video_stream_receiver.h b/video/rtp_video_stream_receiver.h index 237a5144a0..e57a32d01d 100644 --- a/video/rtp_video_stream_receiver.h +++ b/video/rtp_video_stream_receiver.h @@ -58,6 +58,7 @@ class UlpfecReceiver; class RtpVideoStreamReceiver : public LossNotificationSender, public RecoveredPacketReceiver, public RtpPacketSinkInterface, + public KeyFrameRequestSender, public video_coding::OnAssembledFrameCallback, public video_coding::OnCompleteFrameCallback, public OnDecryptedFrameCallback, @@ -76,9 +77,25 @@ class RtpVideoStreamReceiver : public LossNotificationSender, ReceiveStatisticsProxy* receive_stats_proxy, ProcessThread* process_thread, NackSender* nack_sender, - KeyFrameRequestSender* keyframe_request_sender, video_coding::OnCompleteFrameCallback* complete_frame_callback, rtc::scoped_refptr frame_decryptor); + + // Constructor with injected RtpRtcp. Intended for tests only! + RtpVideoStreamReceiver( + Clock* clock, + std::unique_ptr rtp_rtcp, + // The packet router is optional; if provided, the RtpRtcp module for this + // stream is registered as a candidate for sending REMB and transport + // feedback. + PacketRouter* packet_router, + const VideoReceiveStream::Config* config, + ReceiveStatistics* rtp_receive_statistics, + ReceiveStatisticsProxy* receive_stats_proxy, + ProcessThread* process_thread, + NackSender* nack_sender, + video_coding::OnCompleteFrameCallback* complete_frame_callback, + rtc::scoped_refptr frame_decryptor); + ~RtpVideoStreamReceiver() override; void AddReceiveCodec(const VideoCodec& video_codec, @@ -119,7 +136,7 @@ class RtpVideoStreamReceiver : public LossNotificationSender, void OnRecoveredPacket(const uint8_t* packet, size_t packet_length) override; // Send an RTCP keyframe request. - void RequestKeyFrame(); + void RequestKeyFrame() override; // Implements LossNotificationSender. void SendLossNotification(uint16_t last_decoded_seq_num, @@ -205,7 +222,6 @@ class RtpVideoStreamReceiver : public LossNotificationSender, // Members for the new jitter buffer experiment. video_coding::OnCompleteFrameCallback* complete_frame_callback_; - KeyFrameRequestSender* const keyframe_request_sender_; std::unique_ptr nack_module_; std::unique_ptr loss_notification_controller_; rtc::scoped_refptr packet_buffer_; diff --git a/video/rtp_video_stream_receiver_unittest.cc b/video/rtp_video_stream_receiver_unittest.cc index d9c10713b9..981421c1eb 100644 --- a/video/rtp_video_stream_receiver_unittest.cc +++ b/video/rtp_video_stream_receiver_unittest.cc @@ -16,6 +16,7 @@ #include "api/video/video_frame_type.h" #include "common_video/h264/h264_common.h" #include "media/base/media_constants.h" +#include "modules/rtp_rtcp/mocks/mock_rtp_rtcp.h" #include "modules/rtp_rtcp/source/rtp_format.h" #include "modules/rtp_rtcp/source/rtp_generic_frame_descriptor.h" #include "modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension.h" @@ -139,8 +140,7 @@ class RtpVideoStreamReceiverTest : public ::testing::Test { rtp_video_stream_receiver_ = absl::make_unique( Clock::GetRealTimeClock(), &mock_transport_, nullptr, nullptr, &config_, rtp_receive_statistics_.get(), nullptr, process_thread_.get(), - &mock_nack_sender_, &mock_key_frame_request_sender_, - &mock_on_complete_frame_callback_, nullptr); + &mock_nack_sender_, &mock_on_complete_frame_callback_, nullptr); } RTPVideoHeader GetDefaultH264VideoHeader() { @@ -199,7 +199,6 @@ class RtpVideoStreamReceiverTest : public ::testing::Test { const webrtc::test::ScopedFieldTrials override_field_trials_; VideoReceiveStream::Config config_; MockNackSender mock_nack_sender_; - MockKeyFrameRequestSender mock_key_frame_request_sender_; MockTransport mock_transport_; MockOnCompleteFrameCallback mock_on_complete_frame_callback_; std::unique_ptr process_thread_; @@ -543,6 +542,15 @@ TEST_F(RtpVideoStreamReceiverTest, PaddingInMediaStream) { } TEST_F(RtpVideoStreamReceiverTest, RequestKeyframeIfFirstFrameIsDelta) { + // Keep raw pointer, but pass ownership to RtpVideoStreamReceiver. Object + // stays alive for the duration of this test. + auto* mock_rtp_rtcp = new ::testing::NiceMock; + + rtp_video_stream_receiver_ = absl::make_unique( + Clock::GetRealTimeClock(), absl::WrapUnique(mock_rtp_rtcp), nullptr, + &config_, rtp_receive_statistics_.get(), nullptr, process_thread_.get(), + &mock_nack_sender_, &mock_on_complete_frame_callback_, nullptr); + RTPHeader rtp_header; RTPVideoHeader video_header; const std::vector data({1, 2, 3, 4}); @@ -551,7 +559,7 @@ TEST_F(RtpVideoStreamReceiverTest, RequestKeyframeIfFirstFrameIsDelta) { video_header.is_last_packet_in_frame = true; video_header.codec = kVideoCodecGeneric; video_header.frame_type = VideoFrameType::kVideoFrameDelta; - EXPECT_CALL(mock_key_frame_request_sender_, RequestKeyFrame()); + EXPECT_CALL(*mock_rtp_rtcp, RequestKeyFrame()); rtp_video_stream_receiver_->OnReceivedPayloadData( data.data(), data.size(), rtp_header, video_header, absl::nullopt, false); } diff --git a/video/video_receive_stream.cc b/video/video_receive_stream.cc index 176b68258e..2d99be690c 100644 --- a/video/video_receive_stream.cc +++ b/video/video_receive_stream.cc @@ -205,7 +205,6 @@ VideoReceiveStream::VideoReceiveStream( &stats_proxy_, process_thread_, this, // NackSender - this, // KeyFrameRequestSender this, // OnCompleteFrameCallback config_.frame_decryptor), rtp_stream_sync_(this), diff --git a/video/video_receive_stream.h b/video/video_receive_stream.h index 6cff06d96a..89dc36f4b9 100644 --- a/video/video_receive_stream.h +++ b/video/video_receive_stream.h @@ -46,7 +46,6 @@ namespace internal { class VideoReceiveStream : public webrtc::VideoReceiveStream, public rtc::VideoSinkInterface, public NackSender, - public KeyFrameRequestSender, public video_coding::OnCompleteFrameCallback, public Syncable, public CallStatsObserver, @@ -103,9 +102,6 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream, // Implements NackSender. void SendNack(const std::vector& sequence_numbers) override; - // Implements KeyFrameRequestSender. - void RequestKeyFrame() override; - // Implements video_coding::OnCompleteFrameCallback. void OnCompleteFrame( std::unique_ptr frame) override; @@ -141,6 +137,7 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream, void UpdatePlayoutDelays() const RTC_EXCLUSIVE_LOCKS_REQUIRED(playout_delay_lock_); + void RequestKeyFrame(); SequenceChecker worker_sequence_checker_; SequenceChecker module_process_sequence_checker_;