From 6b642f730c6a56650e7e7c036e8b7717db45b4be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Fri, 8 Dec 2017 14:11:14 +0100 Subject: [PATCH] Delete EncodedFrameObserver::OnEncodeTiming. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This callback was used only by the PrintSamplesToFile feature of video_quality_test, which looks like it has been broken for some time (due to mixup of capture time and ntp time). Bug: webrtc:8504 Change-Id: I7d2b55405caeffda582ae0d6fb0e7dfdfce4c5a9 Reviewed-on: https://webrtc-review.googlesource.com/31420 Commit-Queue: Stefan Holmer Reviewed-by: Erik Språng Reviewed-by: Stefan Holmer Cr-Commit-Position: refs/heads/master@{#21211} --- common_video/include/frame_callback.h | 1 - video/encoder_rtcp_feedback_unittest.cc | 1 - video/overuse_frame_detector.cc | 26 ++++++--------------- video/overuse_frame_detector.h | 5 +--- video/overuse_frame_detector_unittest.cc | 8 +++---- video/video_quality_test.cc | 29 ++---------------------- video/video_send_stream.cc | 1 - video/video_stream_encoder.cc | 2 -- video/video_stream_encoder.h | 1 - video/video_stream_encoder_unittest.cc | 4 ---- 10 files changed, 13 insertions(+), 65 deletions(-) diff --git a/common_video/include/frame_callback.h b/common_video/include/frame_callback.h index 2101965287..a3883c1d8d 100644 --- a/common_video/include/frame_callback.h +++ b/common_video/include/frame_callback.h @@ -49,7 +49,6 @@ struct EncodedFrame { class EncodedFrameObserver { public: virtual void EncodedFrameCallback(const EncodedFrame& encoded_frame) = 0; - virtual void OnEncodeTiming(int64_t capture_ntp_ms, int encode_duration_ms) {} protected: virtual ~EncodedFrameObserver() {} diff --git a/video/encoder_rtcp_feedback_unittest.cc b/video/encoder_rtcp_feedback_unittest.cc index dd09540ce8..8acbec41f9 100644 --- a/video/encoder_rtcp_feedback_unittest.cc +++ b/video/encoder_rtcp_feedback_unittest.cc @@ -29,7 +29,6 @@ class MockVideoStreamEncoder : public VideoStreamEncoder { VideoSendStream::Config::EncoderSettings("fake", 0, nullptr), nullptr, - nullptr, std::unique_ptr()) {} ~MockVideoStreamEncoder() { Stop(); } diff --git a/video/overuse_frame_detector.cc b/video/overuse_frame_detector.cc index c45a0179ea..ffbeeedcca 100644 --- a/video/overuse_frame_detector.cc +++ b/video/overuse_frame_detector.cc @@ -20,7 +20,6 @@ #include #include "api/video/video_frame.h" -#include "common_video/include/frame_callback.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" #include "rtc_base/numerics/exp_filter.h" @@ -71,13 +70,11 @@ const auto kScaleReasonCpu = AdaptationObserverInterface::AdaptReason::kCpu; // captured frames). class SendProcessingUsage : public OveruseFrameDetector::ProcessingUsage { public: - explicit SendProcessingUsage(const CpuOveruseOptions& options, - EncodedFrameObserver* encoder_timing) + explicit SendProcessingUsage(const CpuOveruseOptions& options) : kWeightFactorFrameDiff(0.998f), kWeightFactorProcessing(0.995f), kInitialSampleDiffMs(40.0f), options_(options), - encoder_timing_(encoder_timing), count_(0), last_processed_capture_time_us_(-1), max_sample_diff_ms_(kDefaultSampleDiffMs * kMaxSampleDiffMarginFactor), @@ -142,12 +139,7 @@ class SendProcessingUsage : public OveruseFrameDetector::ProcessingUsage { if (timing.last_send_us != -1) { encode_duration_us.emplace( static_cast(timing.last_send_us - timing.capture_us)); - if (encoder_timing_) { - // TODO(nisse): Update encoder_timing_ to also use us units. - encoder_timing_->OnEncodeTiming( - timing.capture_time_us / rtc::kNumMicrosecsPerMillisec, - *encode_duration_us / rtc::kNumMicrosecsPerMillisec); - } + if (last_processed_capture_time_us_ != -1) { int64_t diff_us = timing.capture_us - last_processed_capture_time_us_; AddSample(1e-3 * (*encode_duration_us), 1e-3 * diff_us); @@ -211,7 +203,6 @@ class SendProcessingUsage : public OveruseFrameDetector::ProcessingUsage { const float kInitialSampleDiffMs; const CpuOveruseOptions options_; - EncodedFrameObserver* const encoder_timing_; std::list frame_timing_; uint64_t count_; int64_t last_processed_capture_time_us_; @@ -224,11 +215,10 @@ class SendProcessingUsage : public OveruseFrameDetector::ProcessingUsage { class OverdoseInjector : public SendProcessingUsage { public: OverdoseInjector(const CpuOveruseOptions& options, - EncodedFrameObserver* encoder_timing, int64_t normal_period_ms, int64_t overuse_period_ms, int64_t underuse_period_ms) - : SendProcessingUsage(options, encoder_timing), + : SendProcessingUsage(options), normal_period_ms_(normal_period_ms), overuse_period_ms_(overuse_period_ms), underuse_period_ms_(underuse_period_ms), @@ -350,8 +340,7 @@ CpuOveruseOptions::CpuOveruseOptions() std::unique_ptr OveruseFrameDetector::CreateProcessingUsage( - const CpuOveruseOptions& options, - EncodedFrameObserver* encoder_timing) { + const CpuOveruseOptions& options) { std::unique_ptr instance; std::string toggling_interval = field_trial::FindFullName("WebRTC-ForceSimulatedOveruseIntervalMs"); @@ -364,7 +353,7 @@ OveruseFrameDetector::CreateProcessingUsage( if (normal_period_ms > 0 && overuse_period_ms > 0 && underuse_period_ms > 0) { instance = rtc::MakeUnique( - options, encoder_timing, normal_period_ms, overuse_period_ms, + options, normal_period_ms, overuse_period_ms, underuse_period_ms); } else { RTC_LOG(LS_WARNING) @@ -380,7 +369,7 @@ OveruseFrameDetector::CreateProcessingUsage( if (!instance) { // No valid overuse simulation parameters set, use normal usage class. - instance = rtc::MakeUnique(options, encoder_timing); + instance = rtc::MakeUnique(options); } return instance; @@ -419,7 +408,6 @@ class OveruseFrameDetector::CheckOveruseTask : public rtc::QueuedTask { OveruseFrameDetector::OveruseFrameDetector( const CpuOveruseOptions& options, AdaptationObserverInterface* observer, - EncodedFrameObserver* encoder_timing, CpuOveruseMetricsObserver* metrics_observer) : check_overuse_task_(nullptr), options_(options), @@ -436,7 +424,7 @@ OveruseFrameDetector::OveruseFrameDetector( last_rampup_time_ms_(-1), in_quick_rampup_(false), current_rampup_delay_ms_(kStandardRampUpDelayMs), - usage_(CreateProcessingUsage(options, encoder_timing)) { + usage_(CreateProcessingUsage(options)) { task_checker_.Detach(); } diff --git a/video/overuse_frame_detector.h b/video/overuse_frame_detector.h index 76508bcd2c..6af13ac338 100644 --- a/video/overuse_frame_detector.h +++ b/video/overuse_frame_detector.h @@ -24,7 +24,6 @@ namespace webrtc { -class EncodedFrameObserver; class VideoFrame; struct CpuOveruseOptions { @@ -66,7 +65,6 @@ class OveruseFrameDetector { public: OveruseFrameDetector(const CpuOveruseOptions& options, AdaptationObserverInterface* overuse_observer, - EncodedFrameObserver* encoder_timing_, CpuOveruseMetricsObserver* metrics_observer); virtual ~OveruseFrameDetector(); @@ -122,8 +120,7 @@ class OveruseFrameDetector { void ResetAll(int num_pixels); static std::unique_ptr CreateProcessingUsage( - const CpuOveruseOptions& options, - EncodedFrameObserver* encoder_timing); + const CpuOveruseOptions& options); rtc::SequencedTaskChecker task_checker_; // Owned by the task queue from where StartCheckForOveruse is called. diff --git a/video/overuse_frame_detector_unittest.cc b/video/overuse_frame_detector_unittest.cc index 0f3dd8643e..a258b71d35 100644 --- a/video/overuse_frame_detector_unittest.cc +++ b/video/overuse_frame_detector_unittest.cc @@ -57,11 +57,9 @@ class OveruseFrameDetectorUnderTest : public OveruseFrameDetector { public: OveruseFrameDetectorUnderTest(const CpuOveruseOptions& options, AdaptationObserverInterface* overuse_observer, - EncodedFrameObserver* encoder_timing, CpuOveruseMetricsObserver* metrics_observer) : OveruseFrameDetector(options, overuse_observer, - encoder_timing, metrics_observer) {} ~OveruseFrameDetectorUnderTest() {} @@ -79,7 +77,7 @@ class OveruseFrameDetectorTest : public ::testing::Test, void ReinitializeOveruseDetector() { overuse_detector_.reset(new OveruseFrameDetectorUnderTest( - options_, observer_.get(), nullptr, this)); + options_, observer_.get(), this)); } void OnEncodedFrameTimeMeasured(int encode_time_ms, @@ -182,7 +180,7 @@ TEST_F(OveruseFrameDetectorTest, OveruseAndRecover) { TEST_F(OveruseFrameDetectorTest, OveruseAndRecoverWithNoObserver) { overuse_detector_.reset(new OveruseFrameDetectorUnderTest( - options_, nullptr, nullptr, this)); + options_, nullptr, this)); EXPECT_CALL(*(observer_.get()), AdaptDown(reason_)).Times(0); TriggerOveruse(options_.high_threshold_consecutive_count); EXPECT_CALL(*(observer_.get()), AdaptUp(reason_)).Times(0); @@ -202,7 +200,7 @@ TEST_F(OveruseFrameDetectorTest, TriggerUnderuseWithMinProcessCount) { options_.min_process_count = 1; CpuOveruseObserverImpl overuse_observer; overuse_detector_.reset(new OveruseFrameDetectorUnderTest( - options_, &overuse_observer, nullptr, this)); + options_, &overuse_observer, this)); InsertAndSendFramesWithInterval( 1200, kFrameIntervalUs, kWidth, kHeight, kProcessTimeUs); overuse_detector_->CheckForOveruse(); diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc index 5e3090a0f8..30a49d48fd 100644 --- a/video/video_quality_test.cc +++ b/video/video_quality_test.cc @@ -318,11 +318,6 @@ class VideoAnalyzer : public PacketReceiver, return receiver_->DeliverPacket(media_type, std::move(packet), packet_time); } - void MeasuredEncodeTiming(int64_t ntp_time_ms, int encode_time_ms) { - rtc::CritScope crit(&comparison_lock_); - samples_encode_time_ms_[ntp_time_ms] = encode_time_ms; - } - void PreEncodeOnFrame(const VideoFrame& video_frame) { rtc::CritScope lock(&crit_); if (!first_encoded_timestamp_) { @@ -592,9 +587,6 @@ class VideoAnalyzer : public PacketReceiver, public: explicit OnEncodeTimingProxy(VideoAnalyzer* parent) : parent_(parent) {} - void OnEncodeTiming(int64_t ntp_time_ms, int encode_time_ms) override { - parent_->MeasuredEncodeTiming(ntp_time_ms, encode_time_ms); - } void EncodedFrameCallback(const EncodedFrame& frame) override { parent_->PostEncodeFrameCallback(frame); } @@ -946,27 +938,12 @@ class VideoAnalyzer : public PacketReceiver, "psnr " "ssim " "encode_time_ms\n"); - int missing_encode_time_samples = 0; for (const Sample& sample : samples_) { - auto it = samples_encode_time_ms_.find(sample.input_time_ms); - int encode_time_ms; - if (it != samples_encode_time_ms_.end()) { - encode_time_ms = it->second; - } else { - ++missing_encode_time_samples; - encode_time_ms = -1; - } fprintf(out, "%d %" PRId64 " %" PRId64 " %" PRId64 " %" PRId64 " %" PRIuS - " %lf %lf %d\n", + " %lf %lf\n", sample.dropped, sample.input_time_ms, sample.send_time_ms, sample.recv_time_ms, sample.render_time_ms, - sample.encoded_frame_size, sample.psnr, sample.ssim, - encode_time_ms); - } - if (missing_encode_time_samples) { - fprintf(stderr, - "Warning: Missing encode_time_ms samples for %d frame(s).\n", - missing_encode_time_samples); + sample.encoded_frame_size, sample.psnr, sample.ssim); } } @@ -1061,8 +1038,6 @@ class VideoAnalyzer : public PacketReceiver, PreEncodeProxy pre_encode_proxy_; OnEncodeTimingProxy encode_timing_proxy_; std::vector samples_ RTC_GUARDED_BY(comparison_lock_); - std::map samples_encode_time_ms_ - RTC_GUARDED_BY(comparison_lock_); test::Statistics sender_time_ RTC_GUARDED_BY(comparison_lock_); test::Statistics receiver_time_ RTC_GUARDED_BY(comparison_lock_); test::Statistics psnr_ RTC_GUARDED_BY(comparison_lock_); diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc index 37e6f6ec10..6b406a0577 100644 --- a/video/video_send_stream.cc +++ b/video/video_send_stream.cc @@ -567,7 +567,6 @@ VideoSendStream::VideoSendStream( new VideoStreamEncoder(num_cpu_cores, &stats_proxy_, config_.encoder_settings, config_.pre_encode_callback, - config_.post_encode_callback, std::unique_ptr())); worker_queue_->PostTask(std::unique_ptr(new ConstructionTask( &send_stream_, &thread_sync_event_, &stats_proxy_, diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 77945dc0b4..a763f013fd 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -388,7 +388,6 @@ VideoStreamEncoder::VideoStreamEncoder( SendStatisticsProxy* stats_proxy, const VideoSendStream::Config::EncoderSettings& settings, rtc::VideoSinkInterface* pre_encode_callback, - EncodedFrameObserver* encoder_timing, std::unique_ptr overuse_detector) : shutdown_event_(true /* manual_reset */, false), number_of_cores_(number_of_cores), @@ -404,7 +403,6 @@ VideoStreamEncoder::VideoStreamEncoder( : new OveruseFrameDetector( GetCpuOveruseOptions(settings.full_overuse_time), this, - encoder_timing, stats_proxy)), stats_proxy_(stats_proxy), pre_encode_callback_(pre_encode_callback), diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h index 421b733961..c69f9c584b 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -75,7 +75,6 @@ class VideoStreamEncoder : public rtc::VideoSinkInterface, SendStatisticsProxy* stats_proxy, const VideoSendStream::Config::EncoderSettings& settings, rtc::VideoSinkInterface* pre_encode_callback, - EncodedFrameObserver* encoder_timing, std::unique_ptr overuse_detector); ~VideoStreamEncoder(); diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index 26e9d381cb..a4f799990b 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -67,11 +67,9 @@ class CpuOveruseDetectorProxy : public OveruseFrameDetector { public: CpuOveruseDetectorProxy(const CpuOveruseOptions& options, AdaptationObserverInterface* overuse_observer, - EncodedFrameObserver* encoder_timing_, CpuOveruseMetricsObserver* metrics_observer) : OveruseFrameDetector(options, overuse_observer, - encoder_timing_, metrics_observer), last_target_framerate_fps_(-1) {} virtual ~CpuOveruseDetectorProxy() {} @@ -101,12 +99,10 @@ class VideoStreamEncoderUnderTest : public VideoStreamEncoder { stats_proxy, settings, nullptr /* pre_encode_callback */, - nullptr /* encoder_timing */, std::unique_ptr( overuse_detector_proxy_ = new CpuOveruseDetectorProxy( GetCpuOveruseOptions(settings.full_overuse_time), this, - nullptr, stats_proxy))) {} void PostTaskAndWait(bool down, AdaptReason reason) {