From 157b7814b97ffa4df36afd817dc2b363affad17a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Mon, 13 May 2019 11:37:12 +0200 Subject: [PATCH] Remove deprecated SetRates/SetRateAllocation from VideoEncoder. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL removes two deprecated methods from the VideoEncoder interface: * int32_t SetRates(uint32_t, uint32_t); * int32_t SetRateAllocation(const VideoBitrateAllocation&, uint32_t); These are no longer used, instead the new version must be implemented: void SetRates(const RateControlParameters&) = 0; Migrating is straight forward. For the old SetRates, simple replace: int32_t MyEncoder::SetRates(uint32_t bitrate, uint32_t framerate) { with void MyEncoder::SetRates(const RateControlParameters& parameters) { uint32_t bitrate = parameters.bitrate.get_sum_kbps(); uint32_t framerate = static_cast(parameters.framerate_fps + 0.5); For SetRateAllocation, replace: int32_t MyEncoder::SetRateAllocation( const VideoBitrateAllocation& allocation, uint32_t framerate) { with void MyEncoder::SetRates(const RateControlParameters& parameters) { const VideoBitrateAllocation& allocation = parameters.bitrate; uint32_t framerate = static_cast(parameters.framerate_fps + 0.5); Two more things to note: 1. The new method is void. Previously the only use of the return value in production code was to log a more or less generic error message. Instead, log the actual error from the encoder when it happens, then just return. 2. The new method is pure virtual; it must be overriden even in test. This CL is intended to be landed two weeks after creation, on Thursday May 9th 2019. Bug: webrtc:10481 Change-Id: I61349571a280bd40cd100ca9f93c4aa7748ed30d Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/134214 Reviewed-by: Stefan Holmer Reviewed-by: Niels Moller Commit-Queue: Erik Språng Cr-Commit-Position: refs/heads/master@{#27926} --- api/test/mock_video_encoder.h | 4 ---- api/video_codecs/video_encoder.cc | 16 ---------------- api/video_codecs/video_encoder.h | 18 +----------------- 3 files changed, 1 insertion(+), 37 deletions(-) diff --git a/api/test/mock_video_encoder.h b/api/test/mock_video_encoder.h index f99f7e325d..9333b89375 100644 --- a/api/test/mock_video_encoder.h +++ b/api/test/mock_video_encoder.h @@ -44,10 +44,6 @@ class MockVideoEncoder : public VideoEncoder { int32_t(EncodedImageCallback* callback)); MOCK_METHOD0(Release, int32_t()); MOCK_METHOD0(Reset, int32_t()); - MOCK_METHOD2(SetRates, int32_t(uint32_t newBitRate, uint32_t frameRate)); - MOCK_METHOD2(SetRateAllocation, - int32_t(const VideoBitrateAllocation& newBitRate, - uint32_t frameRate)); MOCK_METHOD1(SetRates, void(const RateControlParameters& parameters)); MOCK_CONST_METHOD0(GetEncoderInfo, EncoderInfo(void)); }; diff --git a/api/video_codecs/video_encoder.cc b/api/video_codecs/video_encoder.cc index e4dbd6739d..710d90d2d6 100644 --- a/api/video_codecs/video_encoder.cc +++ b/api/video_codecs/video_encoder.cc @@ -118,22 +118,6 @@ VideoEncoder::RateControlParameters::RateControlParameters( VideoEncoder::RateControlParameters::~RateControlParameters() = default; -int32_t VideoEncoder::SetRates(uint32_t bitrate, uint32_t framerate) { - RTC_NOTREACHED() << "SetRate(uint32_t, uint32_t) is deprecated."; - return -1; -} - -int32_t VideoEncoder::SetRateAllocation( - const VideoBitrateAllocation& allocation, - uint32_t framerate) { - return SetRates(allocation.get_sum_kbps(), framerate); -} - -void VideoEncoder::SetRates(const RateControlParameters& parameters) { - SetRateAllocation(parameters.bitrate, - static_cast(parameters.framerate_fps + 0.5)); -} - void VideoEncoder::OnPacketLossRateUpdate(float packet_loss_rate) {} void VideoEncoder::OnRttUpdate(int64_t rtt_ms) {} diff --git a/api/video_codecs/video_encoder.h b/api/video_codecs/video_encoder.h index 3af7dfda11..c01309f3e6 100644 --- a/api/video_codecs/video_encoder.h +++ b/api/video_codecs/video_encoder.h @@ -289,26 +289,10 @@ class RTC_EXPORT VideoEncoder { virtual int32_t Encode(const VideoFrame& frame, const std::vector* frame_types) = 0; - // DEPRECATED! Instead use the one below: - // void SetRateAllocation(const VideoBitrateAllocation&, DataRate, uint32) - // For now has a default implementation that call RTC_NOTREACHED(). - // TODO(bugs.webrtc.org/10481): Remove this once all usage is gone. - virtual int32_t SetRates(uint32_t bitrate, uint32_t framerate); - - // DEPRECATED! Instead, use void SetRates(const RateControlParameters&); - // For now has a default implementation that calls - // int32_t SetRates(uin32_t, uint32_t) with |allocation.get_sum_kbps()| and - // |framerate| as arguments. This will be removed. - // TODO(bugs.webrtc.org/10481): Remove this once all usage is gone. - virtual int32_t SetRateAllocation(const VideoBitrateAllocation& allocation, - uint32_t framerate); - // Sets rate control parameters: bitrate, framerate, etc. These settings are // instantaneous (i.e. not moving averages) and should apply from now until // the next call to SetRates(). - // Default implementation will call SetRateAllocation() with appropriate - // members of |parameters| as parameters. - virtual void SetRates(const RateControlParameters& parameters); + virtual void SetRates(const RateControlParameters& parameters) = 0; // Inform the encoder when the packet loss rate changes. //