Make KeyFrameRequestSender injectable in RtpVideoStreamReceiver
This is a partial revert of
https://webrtc-review.googlesource.com/c/src/+/130101.
The KeyFrameRequestSender argument is added back to the constructor of
RtpVideoStreamReceiver. It is optional; if a null pointer is passed,
key frame requests are sent via the internal RtpRtcp module, and this is
how the class is used by VideoReceiveStream. An injectable
KeyFrameRequestSender is useful for tests, for downstream applications
that want to route key frame requests elsewhere, and should also aid
later migration to RtcpTransciever.
Bug: None
Change-Id: Idf9baeed21570625ad74e9afbe38f7ea5bf79feb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/139107
Commit-Queue: Niels Moller <nisse@webrtc.org>
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28102}
diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc
index adca218..4e51385 100644
--- a/video/rtp_video_stream_receiver.cc
+++ b/video/rtp_video_stream_receiver.cc
@@ -85,32 +85,7 @@
ReceiveStatisticsProxy* receive_stats_proxy,
ProcessThread* process_thread,
NackSender* nack_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,
+ KeyFrameRequestSender* keyframe_request_sender,
video_coding::OnCompleteFrameCallback* complete_frame_callback,
rtc::scoped_refptr<FrameDecryptorInterface> frame_decryptor)
: clock_(clock),
@@ -123,8 +98,13 @@
ulpfec_receiver_(UlpfecReceiver::Create(config->rtp.remote_ssrc, this)),
receiving_(false),
last_packet_log_ms_(-1),
- rtp_rtcp_(std::move(rtp_rtcp)),
+ rtp_rtcp_(CreateRtpRtcpModule(clock,
+ rtp_receive_statistics_,
+ transport,
+ rtt_stats,
+ receive_stats_proxy)),
complete_frame_callback_(complete_frame_callback),
+ keyframe_request_sender_(keyframe_request_sender),
has_received_frame_(false),
frames_decryptable_(false) {
constexpr bool remb_candidate = true;
@@ -400,7 +380,11 @@
}
void RtpVideoStreamReceiver::RequestKeyFrame() {
- rtp_rtcp_->RequestKeyFrame();
+ if (keyframe_request_sender_) {
+ keyframe_request_sender_->RequestKeyFrame();
+ } else {
+ rtp_rtcp_->RequestKeyFrame();
+ }
}
void RtpVideoStreamReceiver::SendLossNotification(
diff --git a/video/rtp_video_stream_receiver.h b/video/rtp_video_stream_receiver.h
index e57a32d..6a63ad5 100644
--- a/video/rtp_video_stream_receiver.h
+++ b/video/rtp_video_stream_receiver.h
@@ -77,25 +77,11 @@
ReceiveStatisticsProxy* receive_stats_proxy,
ProcessThread* process_thread,
NackSender* nack_sender,
+ // The KeyFrameRequestSender is optional; if not provided, key frame
+ // requests are sent via the internal RtpRtcp module.
+ 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,
@@ -222,6 +208,7 @@
// 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 981421c..d9c1071 100644
--- a/video/rtp_video_stream_receiver_unittest.cc
+++ b/video/rtp_video_stream_receiver_unittest.cc
@@ -16,7 +16,6 @@
#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"
@@ -140,7 +139,8 @@
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_on_complete_frame_callback_, nullptr);
+ &mock_nack_sender_, &mock_key_frame_request_sender_,
+ &mock_on_complete_frame_callback_, nullptr);
}
RTPVideoHeader GetDefaultH264VideoHeader() {
@@ -199,6 +199,7 @@
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_;
@@ -542,15 +543,6 @@
}
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});
@@ -559,7 +551,7 @@
video_header.is_last_packet_in_frame = true;
video_header.codec = kVideoCodecGeneric;
video_header.frame_type = VideoFrameType::kVideoFrameDelta;
- EXPECT_CALL(*mock_rtp_rtcp, RequestKeyFrame());
+ EXPECT_CALL(mock_key_frame_request_sender_, 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 c495f4e..99b83c9 100644
--- a/video/video_receive_stream.cc
+++ b/video/video_receive_stream.cc
@@ -204,8 +204,9 @@
rtp_receive_statistics_.get(),
&stats_proxy_,
process_thread_,
- this, // NackSender
- this, // OnCompleteFrameCallback
+ this, // NackSender
+ nullptr, // Use default KeyFrameRequestSender
+ this, // OnCompleteFrameCallback
config_.frame_decryptor),
rtp_stream_sync_(this),
max_wait_for_keyframe_ms_(KeyframeIntervalSettings::ParseFromFieldTrials()
