Let RtpVideoStreamReceiver implement KeyFrameRequestSender
Bug: None
Change-Id: I02c89aa169b63ddb6e9ec289c783f3e85d07841e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/130101
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28053}
diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc
index e12bd5c..0f38f19 100644
--- a/video/rtp_video_stream_receiver.cc
+++ b/video/rtp_video_stream_receiver.cc
@@ -85,7 +85,32 @@
ReceiveStatisticsProxy* receive_stats_proxy,
ProcessThread* process_thread,
NackSender* nack_sender,
- KeyFrameRequestSender* keyframe_request_sender,
+ video_coding::OnCompleteFrameCallback* complete_frame_callback,
+ rtc::scoped_refptr<FrameDecryptorInterface> 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<RtpRtcp> 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<FrameDecryptorInterface> frame_decryptor)
: clock_(clock),
@@ -98,13 +123,8 @@
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 @@
if (webrtc::field_trial::IsEnabled("WebRTC-RtcpLossNotification")) {
loss_notification_controller_ =
- absl::make_unique<LossNotificationController>(keyframe_request_sender_,
- this);
+ absl::make_unique<LossNotificationController>(this, this);
} else if (config_.rtp.nack.rtp_history_ms != 0) {
- nack_module_ = absl::make_unique<NackModule>(clock_, nack_sender,
- keyframe_request_sender);
+ nack_module_ = absl::make_unique<NackModule>(clock_, nack_sender, this);
process_thread_->RegisterModule(nack_module_.get(), RTC_FROM_HERE);
}
@@ -273,7 +291,7 @@
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 @@
} 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 237a514..e57a32d 100644
--- a/video/rtp_video_stream_receiver.h
+++ b/video/rtp_video_stream_receiver.h
@@ -58,6 +58,7 @@
class RtpVideoStreamReceiver : public LossNotificationSender,
public RecoveredPacketReceiver,
public RtpPacketSinkInterface,
+ public KeyFrameRequestSender,
public video_coding::OnAssembledFrameCallback,
public video_coding::OnCompleteFrameCallback,
public OnDecryptedFrameCallback,
@@ -76,9 +77,25 @@
ReceiveStatisticsProxy* receive_stats_proxy,
ProcessThread* process_thread,
NackSender* nack_sender,
- KeyFrameRequestSender* keyframe_request_sender,
video_coding::OnCompleteFrameCallback* complete_frame_callback,
rtc::scoped_refptr<FrameDecryptorInterface> frame_decryptor);
+
+ // Constructor with injected RtpRtcp. Intended for tests only!
+ RtpVideoStreamReceiver(
+ Clock* clock,
+ std::unique_ptr<RtpRtcp> 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<FrameDecryptorInterface> frame_decryptor);
+
~RtpVideoStreamReceiver() override;
void AddReceiveCodec(const VideoCodec& video_codec,
@@ -119,7 +136,7 @@
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 @@
// Members for the new jitter buffer experiment.
video_coding::OnCompleteFrameCallback* complete_frame_callback_;
- KeyFrameRequestSender* const keyframe_request_sender_;
std::unique_ptr<NackModule> nack_module_;
std::unique_ptr<LossNotificationController> loss_notification_controller_;
rtc::scoped_refptr<video_coding::PacketBuffer> packet_buffer_;
diff --git a/video/rtp_video_stream_receiver_unittest.cc b/video/rtp_video_stream_receiver_unittest.cc
index d9c1071..981421c 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 @@
rtp_video_stream_receiver_ = absl::make_unique<RtpVideoStreamReceiver>(
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 @@
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<ProcessThread> process_thread_;
@@ -543,6 +542,15 @@
}
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<MockRtpRtcp>;
+
+ rtp_video_stream_receiver_ = absl::make_unique<RtpVideoStreamReceiver>(
+ 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<uint8_t> data({1, 2, 3, 4});
@@ -551,7 +559,7 @@
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 176b682..2d99be6 100644
--- a/video/video_receive_stream.cc
+++ b/video/video_receive_stream.cc
@@ -205,7 +205,6 @@
&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 6cff06d..89dc36f 100644
--- a/video/video_receive_stream.h
+++ b/video/video_receive_stream.h
@@ -46,7 +46,6 @@
class VideoReceiveStream : public webrtc::VideoReceiveStream,
public rtc::VideoSinkInterface<VideoFrame>,
public NackSender,
- public KeyFrameRequestSender,
public video_coding::OnCompleteFrameCallback,
public Syncable,
public CallStatsObserver,
@@ -103,9 +102,6 @@
// Implements NackSender.
void SendNack(const std::vector<uint16_t>& sequence_numbers) override;
- // Implements KeyFrameRequestSender.
- void RequestKeyFrame() override;
-
// Implements video_coding::OnCompleteFrameCallback.
void OnCompleteFrame(
std::unique_ptr<video_coding::EncodedFrame> frame) override;
@@ -141,6 +137,7 @@
void UpdatePlayoutDelays() const
RTC_EXCLUSIVE_LOCKS_REQUIRED(playout_delay_lock_);
+ void RequestKeyFrame();
SequenceChecker worker_sequence_checker_;
SequenceChecker module_process_sequence_checker_;