From b2db9890c55c4d20f7e42941fc905799e3d40ec3 Mon Sep 17 00:00:00 2001 From: Tommi Date: Mon, 23 Aug 2021 22:44:49 +0200 Subject: [PATCH] ReceiveStatisticsProxy: Remove dependency on VideoReceiveStream::Config. The config struct is big and in order to control access to its state, some of which isn't always const, we need to limit raw unlocked access to it from other classes. Bug: none Change-Id: I4513c41486e79ef6c5cfd6376122ab338ad94642 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/229921 Reviewed-by: Niels Moller Commit-Queue: Tommi Cr-Commit-Position: refs/heads/main@{#34835} --- video/receive_statistics_proxy2.cc | 11 +++++------ video/receive_statistics_proxy2.h | 2 +- video/receive_statistics_proxy2_unittest.cc | 15 +++------------ video/video_receive_stream2.cc | 4 ++-- 4 files changed, 11 insertions(+), 21 deletions(-) diff --git a/video/receive_statistics_proxy2.cc b/video/receive_statistics_proxy2.cc index 0e700e3bbc..22da793cdc 100644 --- a/video/receive_statistics_proxy2.cc +++ b/video/receive_statistics_proxy2.cc @@ -98,10 +98,9 @@ bool IsCurrentTaskQueueOrThread(TaskQueueBase* task_queue) { } // namespace -ReceiveStatisticsProxy::ReceiveStatisticsProxy( - const VideoReceiveStream::Config* config, - Clock* clock, - TaskQueueBase* worker_thread) +ReceiveStatisticsProxy::ReceiveStatisticsProxy(uint32_t remote_ssrc, + Clock* clock, + TaskQueueBase* worker_thread) : clock_(clock), start_ms_(clock->TimeInMilliseconds()), enable_decode_time_histograms_( @@ -121,7 +120,7 @@ ReceiveStatisticsProxy::ReceiveStatisticsProxy( kNumMeasurementsVariance), num_bad_states_(0), num_certain_states_(0), - remote_ssrc_(config->rtp.remote_ssrc), + remote_ssrc_(remote_ssrc), // 1000ms window, scale 1000 for ms to s. decode_fps_estimator_(1000, 1000), renders_fps_estimator_(1000, 1000), @@ -139,7 +138,7 @@ ReceiveStatisticsProxy::ReceiveStatisticsProxy( RTC_DCHECK(worker_thread); decode_queue_.Detach(); incoming_render_queue_.Detach(); - stats_.ssrc = config->rtp.remote_ssrc; + stats_.ssrc = remote_ssrc_; } ReceiveStatisticsProxy::~ReceiveStatisticsProxy() { diff --git a/video/receive_statistics_proxy2.h b/video/receive_statistics_proxy2.h index 2dda15e240..32269f7381 100644 --- a/video/receive_statistics_proxy2.h +++ b/video/receive_statistics_proxy2.h @@ -48,7 +48,7 @@ class ReceiveStatisticsProxy : public VCMReceiveStatisticsCallback, public RtcpCnameCallback, public RtcpPacketTypeCounterObserver { public: - ReceiveStatisticsProxy(const VideoReceiveStream::Config* config, + ReceiveStatisticsProxy(uint32_t remote_ssrc, Clock* clock, TaskQueueBase* worker_thread); ~ReceiveStatisticsProxy() override; diff --git a/video/receive_statistics_proxy2_unittest.cc b/video/receive_statistics_proxy2_unittest.cc index 50f16c5385..72b4e3c3a9 100644 --- a/video/receive_statistics_proxy2_unittest.cc +++ b/video/receive_statistics_proxy2_unittest.cc @@ -34,7 +34,6 @@ namespace webrtc { namespace internal { namespace { const int64_t kFreqOffsetProcessIntervalInMs = 40000; -const uint32_t kLocalSsrc = 123; const uint32_t kRemoteSsrc = 456; const int kMinRequiredSamples = 200; const int kWidth = 1280; @@ -44,10 +43,10 @@ const int kHeight = 720; // TODO(sakal): ReceiveStatisticsProxy is lacking unittesting. class ReceiveStatisticsProxy2Test : public ::testing::Test { public: - ReceiveStatisticsProxy2Test() : fake_clock_(1234), config_(GetTestConfig()) { + ReceiveStatisticsProxy2Test() : fake_clock_(1234) { metrics::Reset(); - statistics_proxy_.reset( - new ReceiveStatisticsProxy(&config_, &fake_clock_, loop_.task_queue())); + statistics_proxy_.reset(new ReceiveStatisticsProxy( + kRemoteSsrc, &fake_clock_, loop_.task_queue())); } ~ReceiveStatisticsProxy2Test() override { statistics_proxy_.reset(); } @@ -66,13 +65,6 @@ class ReceiveStatisticsProxy2Test : public ::testing::Test { statistics_proxy_->UpdateHistograms(fraction_lost, rtp_stats, rtx_stats); } - VideoReceiveStream::Config GetTestConfig() { - VideoReceiveStream::Config config(nullptr); - config.rtp.local_ssrc = kLocalSsrc; - config.rtp.remote_ssrc = kRemoteSsrc; - return config; - } - VideoFrame CreateFrame(int width, int height) { return CreateVideoFrame(width, height, 0); } @@ -111,7 +103,6 @@ class ReceiveStatisticsProxy2Test : public ::testing::Test { } SimulatedClock fake_clock_; - const VideoReceiveStream::Config config_; std::unique_ptr statistics_proxy_; test::RunLoop loop_; }; diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc index d7fcd086ac..c605ecaca9 100644 --- a/video/video_receive_stream2.cc +++ b/video/video_receive_stream2.cc @@ -202,7 +202,7 @@ VideoReceiveStream2::VideoReceiveStream2( clock_(clock), call_stats_(call_stats), source_tracker_(clock_), - stats_proxy_(&config_, clock_, call->worker_thread()), + stats_proxy_(config_.rtp.remote_ssrc, clock_, call->worker_thread()), rtp_receive_statistics_(ReceiveStatistics::Create(clock_)), timing_(timing), video_receiver_(clock_, timing_.get()), @@ -467,7 +467,7 @@ void VideoReceiveStream2::CreateAndRegisterExternalDecoder( char filename_buffer[256]; rtc::SimpleStringBuilder ssb(filename_buffer); ssb << decoded_output_file << "/webrtc_receive_stream_" - << this->config_.rtp.remote_ssrc << "-" << rtc::TimeMicros() << ".ivf"; + << config_.rtp.remote_ssrc << "-" << rtc::TimeMicros() << ".ivf"; video_decoder = CreateFrameDumpingDecoderWrapper( std::move(video_decoder), FileWrapper::OpenWriteOnly(ssb.str())); }