Injecting Clock into video senders.
Bug: webrtc:10365
Change-Id: I1dc42345a95929970d4f390e04eff56ca0c6d60b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/125190
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26959}diff --git a/api/video/video_stream_encoder_create.cc b/api/video/video_stream_encoder_create.cc
index 3225c98..875edd9 100644
--- a/api/video/video_stream_encoder_create.cc
+++ b/api/video/video_stream_encoder_create.cc
@@ -20,17 +20,19 @@
uint32_t number_of_cores,
VideoStreamEncoderObserver* encoder_stats_observer,
const VideoStreamEncoderSettings& settings) {
- return CreateVideoStreamEncoder(&GlobalTaskQueueFactory(), number_of_cores,
+ return CreateVideoStreamEncoder(Clock::GetRealTimeClock(),
+ &GlobalTaskQueueFactory(), number_of_cores,
encoder_stats_observer, settings);
}
std::unique_ptr<VideoStreamEncoderInterface> CreateVideoStreamEncoder(
+ Clock* clock,
TaskQueueFactory* task_queue_factory,
uint32_t number_of_cores,
VideoStreamEncoderObserver* encoder_stats_observer,
const VideoStreamEncoderSettings& settings) {
return absl::make_unique<VideoStreamEncoder>(
- number_of_cores, encoder_stats_observer, settings,
+ clock, number_of_cores, encoder_stats_observer, settings,
absl::make_unique<OveruseFrameDetector>(encoder_stats_observer),
task_queue_factory);
}
diff --git a/api/video/video_stream_encoder_create.h b/api/video/video_stream_encoder_create.h
index 3923190..4241626 100644
--- a/api/video/video_stream_encoder_create.h
+++ b/api/video/video_stream_encoder_create.h
@@ -22,6 +22,8 @@
#include "api/video/video_stream_encoder_settings.h"
namespace webrtc {
+// TODO(srte): Find a way to avoid this forward declaration.
+class Clock;
std::unique_ptr<VideoStreamEncoderInterface> CreateVideoStreamEncoder(
uint32_t number_of_cores,
@@ -29,6 +31,7 @@
const VideoStreamEncoderSettings& settings);
std::unique_ptr<VideoStreamEncoderInterface> CreateVideoStreamEncoder(
+ Clock* clock,
TaskQueueFactory* task_queue_factory,
uint32_t number_of_cores,
VideoStreamEncoderObserver* encoder_stats_observer,
diff --git a/call/call.cc b/call/call.cc
index 35727b3..7c80779 100644
--- a/call/call.cc
+++ b/call/call.cc
@@ -805,7 +805,7 @@
// TODO(srte): VideoSendStream should call GetWorkerQueue directly rather than
// having it injected.
VideoSendStream* send_stream = new VideoSendStream(
- num_cpu_cores_, module_process_thread_.get(),
+ clock_, num_cpu_cores_, module_process_thread_.get(),
transport_send_ptr_->GetWorkerQueue(), task_queue_factory_,
call_stats_.get(), transport_send_ptr_, bitrate_allocator_.get(),
video_send_delay_stats_.get(), event_log_, std::move(config),
diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc
index b07e5ec..5d0734e 100644
--- a/call/rtp_transport_controller_send.cc
+++ b/call/rtp_transport_controller_send.cc
@@ -109,7 +109,7 @@
std::unique_ptr<FecController> fec_controller,
const RtpSenderFrameEncryptionConfig& frame_encryption_config) {
video_rtp_senders_.push_back(absl::make_unique<RtpVideoSender>(
- suspended_ssrcs, states, rtp_config, rtcp_report_interval_ms,
+ clock_, suspended_ssrcs, states, rtp_config, rtcp_report_interval_ms,
send_transport, observers,
// TODO(holmer): Remove this circular dependency by injecting
// the parts of RtpTransportControllerSendInterface that are really used.
diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index 6c22120..e5098db 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -58,6 +58,7 @@
using webrtc_internal_rtp_video_sender::RtpStreamSender;
std::vector<RtpStreamSender> CreateRtpStreamSenders(
+ Clock* clock,
const RtpConfig& rtp_config,
int rtcp_report_interval_ms,
Transport* send_transport,
@@ -79,6 +80,7 @@
RTC_DCHECK_GT(rtp_config.ssrcs.size(), 0);
RtpRtcp::Configuration configuration;
+ configuration.clock = clock;
configuration.audio = false;
configuration.clock = Clock::GetRealTimeClock();
configuration.receiver_only = false;
@@ -147,6 +149,7 @@
// TODO(brandtr): Update this function when we support multistream protection.
std::unique_ptr<FlexfecSender> MaybeCreateFlexfecSender(
+ Clock* clock,
const RtpConfig& rtp,
const std::map<uint32_t, RtpState>& suspended_ssrcs) {
if (rtp.flexfec.payload_type < 0) {
@@ -185,7 +188,7 @@
return absl::make_unique<FlexfecSender>(
rtp.flexfec.payload_type, rtp.flexfec.ssrc,
rtp.flexfec.protected_media_ssrcs[0], rtp.mid, rtp.extensions,
- RTPSender::FecExtensionSizes(), rtp_state, Clock::GetRealTimeClock());
+ RTPSender::FecExtensionSizes(), rtp_state, clock);
}
uint32_t CalculateOverheadRateBps(int packets_per_second,
@@ -205,6 +208,7 @@
} // namespace
RtpVideoSender::RtpVideoSender(
+ Clock* clock,
std::map<uint32_t, RtpState> suspended_ssrcs,
const std::map<uint32_t, RtpPayloadState>& states,
const RtpConfig& rtp_config,
@@ -224,9 +228,11 @@
active_(false),
module_process_thread_(nullptr),
suspended_ssrcs_(std::move(suspended_ssrcs)),
- flexfec_sender_(MaybeCreateFlexfecSender(rtp_config, suspended_ssrcs_)),
+ flexfec_sender_(
+ MaybeCreateFlexfecSender(clock, rtp_config, suspended_ssrcs_)),
fec_controller_(std::move(fec_controller)),
- rtp_streams_(CreateRtpStreamSenders(rtp_config,
+ rtp_streams_(CreateRtpStreamSenders(clock,
+ rtp_config,
rtcp_report_interval_ms,
send_transport,
observers.intra_frame_callback,
diff --git a/call/rtp_video_sender.h b/call/rtp_video_sender.h
index 69187a9..d50cb7c 100644
--- a/call/rtp_video_sender.h
+++ b/call/rtp_video_sender.h
@@ -70,6 +70,7 @@
public:
// Rtp modules are assumed to be sorted in simulcast index order.
RtpVideoSender(
+ Clock* clock,
std::map<uint32_t, RtpState> suspended_ssrcs,
const std::map<uint32_t, RtpPayloadState>& states,
const RtpConfig& rtp_config,
diff --git a/call/rtp_video_sender_unittest.cc b/call/rtp_video_sender_unittest.cc
index 8430642..a387932 100644
--- a/call/rtp_video_sender_unittest.cc
+++ b/call/rtp_video_sender_unittest.cc
@@ -80,7 +80,7 @@
int payload_type,
const std::map<uint32_t, RtpPayloadState>& suspended_payload_states,
FrameCountObserver* frame_count_observer)
- : clock_(0),
+ : clock_(1000000),
config_(&transport_),
send_delay_stats_(&clock_),
transport_controller_(&clock_,
@@ -101,7 +101,7 @@
config_.rtp.payload_type = payload_type;
std::map<uint32_t, RtpState> suspended_ssrcs;
router_ = absl::make_unique<RtpVideoSender>(
- suspended_ssrcs, suspended_payload_states, config_.rtp,
+ &clock_, suspended_ssrcs, suspended_payload_states, config_.rtp,
config_.rtcp_report_interval_ms, &transport_,
CreateObservers(&call_stats_, &encoder_feedback_, &stats_proxy_,
&stats_proxy_, &stats_proxy_, frame_count_observer,
diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc
index 222e6e4..978558d 100644
--- a/video/video_send_stream.cc
+++ b/video/video_send_stream.cc
@@ -65,6 +65,7 @@
namespace internal {
VideoSendStream::VideoSendStream(
+ Clock* clock,
int num_cpu_cores,
ProcessThread* module_process_thread,
rtc::TaskQueue* worker_queue,
@@ -80,26 +81,24 @@
const std::map<uint32_t, RtpPayloadState>& suspended_payload_states,
std::unique_ptr<FecController> fec_controller)
: worker_queue_(worker_queue),
- stats_proxy_(Clock::GetRealTimeClock(),
- config,
- encoder_config.content_type),
+ stats_proxy_(clock, config, encoder_config.content_type),
config_(std::move(config)),
content_type_(encoder_config.content_type) {
RTC_DCHECK(config_.encoder_settings.encoder_factory);
RTC_DCHECK(config_.encoder_settings.bitrate_allocator_factory);
video_stream_encoder_ =
- CreateVideoStreamEncoder(task_queue_factory, num_cpu_cores, &stats_proxy_,
- config_.encoder_settings);
+ CreateVideoStreamEncoder(clock, task_queue_factory, num_cpu_cores,
+ &stats_proxy_, config_.encoder_settings);
// TODO(srte): Initialization should not be done posted on a task queue.
// Note that the posted task must not outlive this scope since the closure
// references local variables.
worker_queue_->PostTask(rtc::NewClosure(
- [this, call_stats, transport, bitrate_allocator, send_delay_stats,
+ [this, clock, call_stats, transport, bitrate_allocator, send_delay_stats,
event_log, &suspended_ssrcs, &encoder_config, &suspended_payload_states,
&fec_controller]() {
send_stream_.reset(new VideoSendStreamImpl(
- &stats_proxy_, worker_queue_, call_stats, transport,
+ clock, &stats_proxy_, worker_queue_, call_stats, transport,
bitrate_allocator, send_delay_stats, video_stream_encoder_.get(),
event_log, &config_, encoder_config.max_bitrate_bps,
encoder_config.bitrate_priority, suspended_ssrcs,
diff --git a/video/video_send_stream.h b/video/video_send_stream.h
index 92600ce..240432f 100644
--- a/video/video_send_stream.h
+++ b/video/video_send_stream.h
@@ -53,6 +53,7 @@
using RtpPayloadStateMap = std::map<uint32_t, RtpPayloadState>;
VideoSendStream(
+ Clock* clock,
int num_cpu_cores,
ProcessThread* module_process_thread,
rtc::TaskQueue* worker_queue,
diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc
index b8088ea..9a56341 100644
--- a/video/video_send_stream_impl.cc
+++ b/video/video_send_stream_impl.cc
@@ -31,7 +31,6 @@
#include "rtc_base/numerics/safe_conversions.h"
#include "rtc_base/sequenced_task_checker.h"
#include "rtc_base/thread_checker.h"
-#include "rtc_base/time_utils.h"
#include "rtc_base/trace_event.h"
#include "system_wrappers/include/clock.h"
#include "system_wrappers/include/field_trial.h"
@@ -195,6 +194,7 @@
PacingConfig::~PacingConfig() = default;
VideoSendStreamImpl::VideoSendStreamImpl(
+ Clock* clock,
SendStatisticsProxy* stats_proxy,
rtc::TaskQueue* worker_queue,
CallStats* call_stats,
@@ -211,7 +211,8 @@
VideoEncoderConfig::ContentType content_type,
std::unique_ptr<FecController> fec_controller,
MediaTransportInterface* media_transport)
- : has_alr_probing_(config->periodic_alr_bandwidth_probing ||
+ : clock_(clock),
+ has_alr_probing_(config->periodic_alr_bandwidth_probing ||
GetAlrSettings(content_type)),
pacing_config_(PacingConfig()),
stats_proxy_(stats_proxy),
@@ -227,9 +228,7 @@
encoder_bitrate_priority_(initial_encoder_bitrate_priority),
has_packet_feedback_(false),
video_stream_encoder_(video_stream_encoder),
- encoder_feedback_(Clock::GetRealTimeClock(),
- config_->rtp.ssrcs,
- video_stream_encoder),
+ encoder_feedback_(clock, config_->rtp.ssrcs, video_stream_encoder),
bandwidth_observer_(transport->GetBandwidthObserver()),
rtp_video_sender_(transport_->CreateRtpVideoSender(
suspended_ssrcs,
@@ -454,7 +453,7 @@
RTC_DCHECK_RUN_ON(worker_queue_);
- int64_t now_ms = rtc::TimeMillis();
+ int64_t now_ms = clock_->TimeInMilliseconds();
if (encoder_target_rate_bps_ != 0) {
if (video_bitrate_allocation_context_) {
// If new allocation is within kMaxVbaSizeDifferencePercent larger than
diff --git a/video/video_send_stream_impl.h b/video/video_send_stream_impl.h
index 1cdad60..c9472fb 100644
--- a/video/video_send_stream_impl.h
+++ b/video/video_send_stream_impl.h
@@ -70,6 +70,7 @@
public VideoBitrateAllocationObserver {
public:
VideoSendStreamImpl(
+ Clock* clock,
SendStatisticsProxy* stats_proxy,
rtc::TaskQueue* worker_queue,
CallStats* call_stats,
@@ -141,7 +142,7 @@
void SignalEncoderActive();
MediaStreamAllocationConfig GetAllocationConfig() const
RTC_RUN_ON(worker_queue_);
-
+ Clock* const clock_;
const bool has_alr_probing_;
const PacingConfig pacing_config_;
diff --git a/video/video_send_stream_impl_unittest.cc b/video/video_send_stream_impl_unittest.cc
index 1852dc0..ea70408 100644
--- a/video/video_send_stream_impl_unittest.cc
+++ b/video/video_send_stream_impl_unittest.cc
@@ -118,11 +118,11 @@
std::map<uint32_t, RtpState> suspended_ssrcs;
std::map<uint32_t, RtpPayloadState> suspended_payload_states;
return absl::make_unique<VideoSendStreamImpl>(
- &stats_proxy_, &test_queue_, &call_stats_, &transport_controller_,
- &bitrate_allocator_, &send_delay_stats_, &video_stream_encoder_,
- &event_log_, &config_, initial_encoder_max_bitrate,
- initial_encoder_bitrate_priority, suspended_ssrcs,
- suspended_payload_states, content_type,
+ &clock_, &stats_proxy_, &test_queue_, &call_stats_,
+ &transport_controller_, &bitrate_allocator_, &send_delay_stats_,
+ &video_stream_encoder_, &event_log_, &config_,
+ initial_encoder_max_bitrate, initial_encoder_bitrate_priority,
+ suspended_ssrcs, suspended_payload_states, content_type,
absl::make_unique<FecControllerDefault>(&clock_),
/*media_transport=*/nullptr);
}
@@ -524,9 +524,6 @@
TEST_F(VideoSendStreamImplTest, ForwardsVideoBitrateAllocationAfterTimeout) {
test_queue_.SendTask([this] {
- rtc::ScopedFakeClock fake_clock;
- fake_clock.SetTimeMicros(clock_.TimeInMicroseconds());
-
auto vss_impl = CreateVideoSendStreamImpl(
kDefaultInitialBitrateBps, kDefaultBitratePriority,
VideoEncoderConfig::ContentType::kScreen);
@@ -571,7 +568,7 @@
observer->OnBitrateAllocationUpdated(alloc);
}
- fake_clock.AdvanceTimeMicros(kMaxVbaThrottleTimeMs * 1000);
+ clock_.AdvanceTimeMicroseconds(kMaxVbaThrottleTimeMs * 1000);
{
// Sending similar allocation again after timeout, should forward.
@@ -598,7 +595,7 @@
{
// Advance time and send encoded image, this should wake up and send
// cached bitrate allocation.
- fake_clock.AdvanceTimeMicros(kMaxVbaThrottleTimeMs * 1000);
+ clock_.AdvanceTimeMicroseconds(kMaxVbaThrottleTimeMs * 1000);
EXPECT_CALL(rtp_video_sender_, OnBitrateAllocationUpdated(alloc))
.Times(1);
static_cast<EncodedImageCallback*>(vss_impl.get())
@@ -608,7 +605,7 @@
{
// Advance time and send encoded image, there should be no cached
// allocation to send.
- fake_clock.AdvanceTimeMicros(kMaxVbaThrottleTimeMs * 1000);
+ clock_.AdvanceTimeMicroseconds(kMaxVbaThrottleTimeMs * 1000);
EXPECT_CALL(rtp_video_sender_, OnBitrateAllocationUpdated(alloc))
.Times(0);
static_cast<EncodedImageCallback*>(vss_impl.get())
diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc
index f39f77d..528a77e 100644
--- a/video/video_stream_encoder.cc
+++ b/video/video_stream_encoder.cc
@@ -419,6 +419,7 @@
};
VideoStreamEncoder::VideoStreamEncoder(
+ Clock* clock,
uint32_t number_of_cores,
VideoStreamEncoderObserver* encoder_stats_observer,
const VideoStreamEncoderSettings& settings,
@@ -446,7 +447,7 @@
max_data_payload_length_(0),
last_observed_bitrate_bps_(0),
encoder_paused_and_dropped_frame_(false),
- clock_(Clock::GetRealTimeClock()),
+ clock_(clock),
degradation_preference_(DegradationPreference::DISABLED),
posted_frames_waiting_for_encode_(0),
last_captured_timestamp_(0),
diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h
index 911627a..70e2c89 100644
--- a/video/video_stream_encoder.h
+++ b/video/video_stream_encoder.h
@@ -53,7 +53,8 @@
// Protected only to provide access to tests.
protected AdaptationObserverInterface {
public:
- VideoStreamEncoder(uint32_t number_of_cores,
+ VideoStreamEncoder(Clock* clock,
+ uint32_t number_of_cores,
VideoStreamEncoderObserver* encoder_stats_observer,
const VideoStreamEncoderSettings& settings,
std::unique_ptr<OveruseFrameDetector> overuse_detector,
diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc
index 75b47f2..23984e9 100644
--- a/video/video_stream_encoder_unittest.cc
+++ b/video/video_stream_encoder_unittest.cc
@@ -100,7 +100,8 @@
public:
VideoStreamEncoderUnderTest(SendStatisticsProxy* stats_proxy,
const VideoStreamEncoderSettings& settings)
- : VideoStreamEncoder(1 /* number_of_cores */,
+ : VideoStreamEncoder(Clock::GetRealTimeClock(),
+ 1 /* number_of_cores */,
stats_proxy,
settings,
std::unique_ptr<OveruseFrameDetector>(
