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}
This commit is contained in:
Niels Möller
2019-05-29 13:35:14 +02:00
committed by Commit Bot
parent e8e7d7b0bc
commit 2f5554dae5
4 changed files with 23 additions and 59 deletions

View File

@ -85,32 +85,7 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver(
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 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver(
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::OnRtpPacket(const RtpPacketReceived& packet) {
}
void RtpVideoStreamReceiver::RequestKeyFrame() {
rtp_rtcp_->RequestKeyFrame();
if (keyframe_request_sender_) {
keyframe_request_sender_->RequestKeyFrame();
} else {
rtp_rtcp_->RequestKeyFrame();
}
}
void RtpVideoStreamReceiver::SendLossNotification(

View File

@ -77,25 +77,11 @@ class RtpVideoStreamReceiver : public LossNotificationSender,
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 @@ 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<NackModule> nack_module_;
std::unique_ptr<LossNotificationController> loss_notification_controller_;
rtc::scoped_refptr<video_coding::PacketBuffer> packet_buffer_;

View File

@ -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 @@ class RtpVideoStreamReceiverTest : public ::testing::Test {
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 @@ 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<ProcessThread> process_thread_;
@ -542,15 +543,6 @@ 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<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 @@ 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_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);
}

View File

@ -204,8 +204,9 @@ VideoReceiveStream::VideoReceiveStream(
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()