diff --git a/api/video_codecs/vp8_frame_buffer_controller.h b/api/video_codecs/vp8_frame_buffer_controller.h index 1826755360a..012d870ed61 100644 --- a/api/video_codecs/vp8_frame_buffer_controller.h +++ b/api/video_codecs/vp8_frame_buffer_controller.h @@ -11,6 +11,7 @@ #ifndef API_VIDEO_CODECS_VP8_FRAME_BUFFER_CONTROLLER_H_ #define API_VIDEO_CODECS_VP8_FRAME_BUFFER_CONTROLLER_H_ +#include #include #include @@ -46,34 +47,48 @@ namespace webrtc { struct CodecSpecificInfo; -// TODO(eladalon): This configuration is temporal-layers specific; refactor. +// Each member represents an override of the VPX configuration if the optional +// value is set. struct Vp8EncoderConfig { - static constexpr size_t kMaxPeriodicity = 16; - static constexpr size_t kMaxLayers = 5; - - // Number of active temporal layers. Set to 0 if not used. - uint32_t ts_number_layers; - // Arrays of length |ts_number_layers|, indicating (cumulative) target bitrate - // and rate decimator (e.g. 4 if every 4th frame is in the given layer) for - // each active temporal layer, starting with temporal id 0. - uint32_t ts_target_bitrate[kMaxLayers]; - uint32_t ts_rate_decimator[kMaxLayers]; - - // The periodicity of the temporal pattern. Set to 0 if not used. - uint32_t ts_periodicity; - // Array of length |ts_periodicity| indicating the sequence of temporal id's - // to assign to incoming frames. - uint32_t ts_layer_id[kMaxPeriodicity]; + struct TemporalLayerConfig { + bool operator!=(const TemporalLayerConfig& other) const { + return ts_number_layers != other.ts_number_layers || + ts_target_bitrate != other.ts_target_bitrate || + ts_rate_decimator != other.ts_rate_decimator || + ts_periodicity != other.ts_periodicity || + ts_layer_id != other.ts_layer_id; + } + + static constexpr size_t kMaxPeriodicity = 16; + static constexpr size_t kMaxLayers = 5; + + // Number of active temporal layers. Set to 0 if not used. + uint32_t ts_number_layers; + + // Arrays of length |ts_number_layers|, indicating (cumulative) target + // bitrate and rate decimator (e.g. 4 if every 4th frame is in the given + // layer) for each active temporal layer, starting with temporal id 0. + std::array ts_target_bitrate; + std::array ts_rate_decimator; + + // The periodicity of the temporal pattern. Set to 0 if not used. + uint32_t ts_periodicity; + + // Array of length |ts_periodicity| indicating the sequence of temporal id's + // to assign to incoming frames. + std::array ts_layer_id; + }; + + absl::optional temporal_layer_config; // Target bitrate, in bps. - uint32_t rc_target_bitrate; + absl::optional rc_target_bitrate; - // Clamp QP to min/max. Use 0 to disable clamping. - uint32_t rc_min_quantizer; - uint32_t rc_max_quantizer; + // Clamp QP to max. Use 0 to disable clamping. + absl::optional rc_max_quantizer; - // If has_value(), override error resilience to value(). - absl::optional error_resilient; + // Error resilience mode. + absl::optional g_error_resilient; }; // This interface defines a way of delegating the logic of buffer management. @@ -83,6 +98,10 @@ class Vp8FrameBufferController { public: virtual ~Vp8FrameBufferController() = default; + // Set limits on QP. + // The limits are suggestion-only; the controller is allowed to exceed them. + virtual void SetQpLimits(size_t stream_index, int min_qp, int max_qp) = 0; + // Number of streamed controlled by |this|. virtual size_t StreamCount() const = 0; @@ -103,12 +122,13 @@ class Vp8FrameBufferController { const std::vector& bitrates_bps, int framerate_fps) = 0; - // Called by the encoder before encoding a frame. |cfg| contains the current - // configuration. If the encoder wishes any part of that to be changed before - // the encode step, |cfg| should be changed and then return true. If false is - // returned, the encoder will proceed without updating the configuration. - virtual bool UpdateConfiguration(size_t stream_index, - Vp8EncoderConfig* cfg) = 0; + // Called by the encoder before encoding a frame. Returns a set of overrides + // the controller wishes to enact in the encoder's configuration. + // If a value is not overridden, previous overrides are still in effect. + // (It is therefore not possible to go from a specific override to + // no-override. Once the controller takes responsibility over a value, it + // must maintain responsibility for it.) + virtual Vp8EncoderConfig UpdateConfiguration(size_t stream_index) = 0; // Returns the recommended VP8 encode flags needed. // The timestamp may be used as both a time and a unique identifier, and so diff --git a/api/video_codecs/vp8_temporal_layers.cc b/api/video_codecs/vp8_temporal_layers.cc index 7f7d8ad3df8..22916a7dd51 100644 --- a/api/video_codecs/vp8_temporal_layers.cc +++ b/api/video_codecs/vp8_temporal_layers.cc @@ -28,6 +28,13 @@ Vp8TemporalLayers::Vp8TemporalLayers( })); } +void Vp8TemporalLayers::SetQpLimits(size_t stream_index, + int min_qp, + int max_qp) { + RTC_DCHECK_LT(stream_index, controllers_.size()); + return controllers_[stream_index]->SetQpLimits(0, min_qp, max_qp); +} + size_t Vp8TemporalLayers::StreamCount() const { return controllers_.size(); } @@ -47,10 +54,9 @@ void Vp8TemporalLayers::OnRatesUpdated( framerate_fps); } -bool Vp8TemporalLayers::UpdateConfiguration(size_t stream_index, - Vp8EncoderConfig* cfg) { +Vp8EncoderConfig Vp8TemporalLayers::UpdateConfiguration(size_t stream_index) { RTC_DCHECK_LT(stream_index, controllers_.size()); - return controllers_[stream_index]->UpdateConfiguration(0, cfg); + return controllers_[stream_index]->UpdateConfiguration(0); } Vp8FrameConfig Vp8TemporalLayers::NextFrameConfig(size_t stream_index, diff --git a/api/video_codecs/vp8_temporal_layers.h b/api/video_codecs/vp8_temporal_layers.h index 3864705c987..4194352ded4 100644 --- a/api/video_codecs/vp8_temporal_layers.h +++ b/api/video_codecs/vp8_temporal_layers.h @@ -35,6 +35,8 @@ class Vp8TemporalLayers final : public Vp8FrameBufferController { std::vector>&& controllers); ~Vp8TemporalLayers() override = default; + void SetQpLimits(size_t stream_index, int min_qp, int max_qp) override; + size_t StreamCount() const override; bool SupportsEncoderFrameDropping(size_t stream_index) const override; @@ -43,7 +45,7 @@ class Vp8TemporalLayers final : public Vp8FrameBufferController { const std::vector& bitrates_bps, int framerate_fps) override; - bool UpdateConfiguration(size_t stream_index, Vp8EncoderConfig* cfg) override; + Vp8EncoderConfig UpdateConfiguration(size_t stream_index) override; Vp8FrameConfig NextFrameConfig(size_t stream_index, uint32_t rtp_timestamp) override; diff --git a/modules/video_coding/codecs/vp8/default_temporal_layers.cc b/modules/video_coding/codecs/vp8/default_temporal_layers.cc index 8fa8f0d7740..339e81dd163 100644 --- a/modules/video_coding/codecs/vp8/default_temporal_layers.cc +++ b/modules/video_coding/codecs/vp8/default_temporal_layers.cc @@ -10,7 +10,6 @@ #include "modules/video_coding/codecs/vp8/default_temporal_layers.h" #include -#include #include #include @@ -251,6 +250,13 @@ DefaultTemporalLayers::DefaultTemporalLayers(int number_of_temporal_layers) DefaultTemporalLayers::~DefaultTemporalLayers() = default; +void DefaultTemporalLayers::SetQpLimits(size_t stream_index, + int min_qp, + int max_qp) { + RTC_DCHECK_LT(stream_index, StreamCount()); + // Ignore. +} + size_t DefaultTemporalLayers::StreamCount() const { return 1; } @@ -278,28 +284,34 @@ void DefaultTemporalLayers::OnRatesUpdated( } } -bool DefaultTemporalLayers::UpdateConfiguration(size_t stream_index, - Vp8EncoderConfig* cfg) { +Vp8EncoderConfig DefaultTemporalLayers::UpdateConfiguration( + size_t stream_index) { RTC_DCHECK_LT(stream_index, StreamCount()); + Vp8EncoderConfig config; + if (!new_bitrates_bps_) { - return false; + return config; } + config.temporal_layer_config.emplace(); + Vp8EncoderConfig::TemporalLayerConfig& ts_config = + config.temporal_layer_config.value(); + for (size_t i = 0; i < num_layers_; ++i) { - cfg->ts_target_bitrate[i] = (*new_bitrates_bps_)[i] / 1000; + ts_config.ts_target_bitrate[i] = (*new_bitrates_bps_)[i] / 1000; // ..., 4, 2, 1 - cfg->ts_rate_decimator[i] = 1 << (num_layers_ - i - 1); + ts_config.ts_rate_decimator[i] = 1 << (num_layers_ - i - 1); } - cfg->ts_number_layers = num_layers_; - cfg->ts_periodicity = temporal_ids_.size(); - memcpy(cfg->ts_layer_id, &temporal_ids_[0], - sizeof(unsigned int) * temporal_ids_.size()); + ts_config.ts_number_layers = num_layers_; + ts_config.ts_periodicity = temporal_ids_.size(); + std::copy(temporal_ids_.begin(), temporal_ids_.end(), + ts_config.ts_layer_id.begin()); new_bitrates_bps_.reset(); - return true; + return config; } bool DefaultTemporalLayers::IsSyncFrame(const Vp8FrameConfig& config) const { diff --git a/modules/video_coding/codecs/vp8/default_temporal_layers.h b/modules/video_coding/codecs/vp8/default_temporal_layers.h index 34ed711c87c..cb1cbbeef32 100644 --- a/modules/video_coding/codecs/vp8/default_temporal_layers.h +++ b/modules/video_coding/codecs/vp8/default_temporal_layers.h @@ -34,6 +34,8 @@ class DefaultTemporalLayers final : public Vp8FrameBufferController { explicit DefaultTemporalLayers(int number_of_temporal_layers); ~DefaultTemporalLayers() override; + void SetQpLimits(size_t stream_index, int min_qp, int max_qp) override; + size_t StreamCount() const override; bool SupportsEncoderFrameDropping(size_t stream_index) const override; @@ -48,7 +50,7 @@ class DefaultTemporalLayers final : public Vp8FrameBufferController { const std::vector& bitrates_bps, int framerate_fps) override; - bool UpdateConfiguration(size_t stream_index, Vp8EncoderConfig* cfg) override; + Vp8EncoderConfig UpdateConfiguration(size_t stream_index) override; void OnEncodeDone(size_t stream_index, uint32_t rtp_timestamp, diff --git a/modules/video_coding/codecs/vp8/default_temporal_layers_unittest.cc b/modules/video_coding/codecs/vp8/default_temporal_layers_unittest.cc index 8f01e597f6e..2c419dabe23 100644 --- a/modules/video_coding/codecs/vp8/default_temporal_layers_unittest.cc +++ b/modules/video_coding/codecs/vp8/default_temporal_layers_unittest.cc @@ -23,6 +23,8 @@ #include "test/gtest.h" #include "vpx/vp8cx.h" +// TODO(bugs.webrtc.org/10582): Test the behavior of UpdateConfiguration(). + namespace webrtc { namespace test { namespace { @@ -119,12 +121,11 @@ TEST_F(TemporalLayersTest, 2Layers) { constexpr int kNumLayers = 2; DefaultTemporalLayers tl(kNumLayers); DefaultTemporalLayersChecker checker(kNumLayers); - Vp8EncoderConfig cfg; tl.OnRatesUpdated(0, GetTemporalLayerRates(kDefaultBytesPerFrame, kDefaultFramerate, kNumLayers), kDefaultFramerate); - tl.UpdateConfiguration(0, &cfg); + tl.UpdateConfiguration(0); constexpr size_t kPatternSize = 4; constexpr size_t kRepetitions = 4; @@ -162,12 +163,11 @@ TEST_F(TemporalLayersTest, 3Layers) { constexpr int kNumLayers = 3; DefaultTemporalLayers tl(kNumLayers); DefaultTemporalLayersChecker checker(kNumLayers); - Vp8EncoderConfig cfg; tl.OnRatesUpdated(0, GetTemporalLayerRates(kDefaultBytesPerFrame, kDefaultFramerate, kNumLayers), kDefaultFramerate); - tl.UpdateConfiguration(0, &cfg); + tl.UpdateConfiguration(0); int expected_flags[16] = { kTemporalUpdateLast, @@ -217,12 +217,11 @@ TEST_F(TemporalLayersTest, Alternative3Layers) { ScopedFieldTrials field_trial("WebRTC-UseShortVP8TL3Pattern/Enabled/"); DefaultTemporalLayers tl(kNumLayers); DefaultTemporalLayersChecker checker(kNumLayers); - Vp8EncoderConfig cfg; tl.OnRatesUpdated(0, GetTemporalLayerRates(kDefaultBytesPerFrame, kDefaultFramerate, kNumLayers), kDefaultFramerate); - tl.UpdateConfiguration(0, &cfg); + tl.UpdateConfiguration(0); int expected_flags[8] = {kTemporalUpdateLast, kTemporalUpdateAltrefWithoutDependency, @@ -260,12 +259,11 @@ TEST_F(TemporalLayersTest, SearchOrder) { ScopedFieldTrials field_trial("WebRTC-UseShortVP8TL3Pattern/Enabled/"); DefaultTemporalLayers tl(kNumLayers); DefaultTemporalLayersChecker checker(kNumLayers); - Vp8EncoderConfig cfg; tl.OnRatesUpdated(0, GetTemporalLayerRates(kDefaultBytesPerFrame, kDefaultFramerate, kNumLayers), kDefaultFramerate); - tl.UpdateConfiguration(0, &cfg); + tl.UpdateConfiguration(0); // Use a repeating pattern of tl 0, 2, 1, 2. // Tl 0, 1, 2 update last, golden, altref respectively. @@ -304,12 +302,11 @@ TEST_F(TemporalLayersTest, SearchOrderWithDrop) { ScopedFieldTrials field_trial("WebRTC-UseShortVP8TL3Pattern/Enabled/"); DefaultTemporalLayers tl(kNumLayers); DefaultTemporalLayersChecker checker(kNumLayers); - Vp8EncoderConfig cfg; tl.OnRatesUpdated(0, GetTemporalLayerRates(kDefaultBytesPerFrame, kDefaultFramerate, kNumLayers), kDefaultFramerate); - tl.UpdateConfiguration(0, &cfg); + tl.UpdateConfiguration(0); // Use a repeating pattern of tl 0, 2, 1, 2. // Tl 0, 1, 2 update last, golden, altref respectively. @@ -344,12 +341,11 @@ TEST_F(TemporalLayersTest, 4Layers) { constexpr int kNumLayers = 4; DefaultTemporalLayers tl(kNumLayers); DefaultTemporalLayersChecker checker(kNumLayers); - Vp8EncoderConfig cfg; tl.OnRatesUpdated(0, GetTemporalLayerRates(kDefaultBytesPerFrame, kDefaultFramerate, kNumLayers), kDefaultFramerate); - tl.UpdateConfiguration(0, &cfg); + tl.UpdateConfiguration(0); int expected_flags[16] = { kTemporalUpdateLast, kTemporalUpdateNoneNoRefGoldenAltRef, @@ -400,12 +396,11 @@ TEST_F(TemporalLayersTest, DoesNotReferenceDroppedFrames) { ScopedFieldTrials field_trial("WebRTC-UseShortVP8TL3Pattern/Enabled/"); DefaultTemporalLayers tl(kNumLayers); DefaultTemporalLayersChecker checker(kNumLayers); - Vp8EncoderConfig cfg; tl.OnRatesUpdated(0, GetTemporalLayerRates(kDefaultBytesPerFrame, kDefaultFramerate, kNumLayers), kDefaultFramerate); - tl.UpdateConfiguration(0, &cfg); + tl.UpdateConfiguration(0); // Start with a keyframe. uint32_t timestamp = 0; @@ -487,12 +482,11 @@ TEST_F(TemporalLayersTest, DoesNotReferenceUnlessGuaranteedToExist) { // Tl 0, 1 updates last, golden respectively. Altref is always last keyframe. DefaultTemporalLayers tl(kNumLayers); DefaultTemporalLayersChecker checker(kNumLayers); - Vp8EncoderConfig cfg; tl.OnRatesUpdated(0, GetTemporalLayerRates(kDefaultBytesPerFrame, kDefaultFramerate, kNumLayers), kDefaultFramerate); - tl.UpdateConfiguration(0, &cfg); + tl.UpdateConfiguration(0); // Start with a keyframe. uint32_t timestamp = 0; @@ -557,12 +551,11 @@ TEST_F(TemporalLayersTest, DoesNotReferenceUnlessGuaranteedToExistLongDelay) { ScopedFieldTrials field_trial("WebRTC-UseShortVP8TL3Pattern/Enabled/"); DefaultTemporalLayers tl(kNumLayers); DefaultTemporalLayersChecker checker(kNumLayers); - Vp8EncoderConfig cfg; tl.OnRatesUpdated(0, GetTemporalLayerRates(kDefaultBytesPerFrame, kDefaultFramerate, kNumLayers), kDefaultFramerate); - tl.UpdateConfiguration(0, &cfg); + tl.UpdateConfiguration(0); // Start with a keyframe. uint32_t timestamp = 0; @@ -618,12 +611,11 @@ TEST_F(TemporalLayersTest, KeyFrame) { constexpr int kNumLayers = 3; DefaultTemporalLayers tl(kNumLayers); DefaultTemporalLayersChecker checker(kNumLayers); - Vp8EncoderConfig cfg; tl.OnRatesUpdated(0, GetTemporalLayerRates(kDefaultBytesPerFrame, kDefaultFramerate, kNumLayers), kDefaultFramerate); - tl.UpdateConfiguration(0, &cfg); + tl.UpdateConfiguration(0); int expected_flags[8] = { kTemporalUpdateLastRefAltRef, @@ -735,11 +727,10 @@ INSTANTIATE_TEST_SUITE_P(DefaultTemporalLayersTest, TEST_P(TemporalLayersReferenceTest, ValidFrameConfigs) { const int num_layers = GetParam(); DefaultTemporalLayers tl(num_layers); - Vp8EncoderConfig cfg; tl.OnRatesUpdated( 0, GetTemporalLayerRates(kDefaultBytesPerFrame, kDefaultFramerate, 1), kDefaultFramerate); - tl.UpdateConfiguration(0, &cfg); + tl.UpdateConfiguration(0); // Run through the pattern and store the frame dependencies, plus keep track // of the buffer state; which buffers references which temporal layers (if diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc index 5f305fd2db6..9b6c6c63432 100644 --- a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc +++ b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc @@ -15,6 +15,7 @@ #include #include +#include #include #include #include @@ -145,58 +146,74 @@ void UpdateRateSettings(vpx_codec_enc_cfg_t* config, config->rc_dropframe_thresh = new_settings.rc_dropframe_thresh; } -static_assert(Vp8EncoderConfig::kMaxPeriodicity == VPX_TS_MAX_PERIODICITY, +static_assert(Vp8EncoderConfig::TemporalLayerConfig::kMaxPeriodicity == + VPX_TS_MAX_PERIODICITY, "Vp8EncoderConfig::kMaxPeriodicity must be kept in sync with the " "constant in libvpx."); -static_assert(Vp8EncoderConfig::kMaxLayers == VPX_TS_MAX_LAYERS, +static_assert(Vp8EncoderConfig::TemporalLayerConfig::kMaxLayers == + VPX_TS_MAX_LAYERS, "Vp8EncoderConfig::kMaxLayers must be kept in sync with the " "constant in libvpx."); -static Vp8EncoderConfig GetEncoderConfig(vpx_codec_enc_cfg* vpx_config) { - Vp8EncoderConfig config; - - config.ts_number_layers = vpx_config->ts_number_layers; - memcpy(config.ts_target_bitrate, vpx_config->ts_target_bitrate, - sizeof(unsigned int) * Vp8EncoderConfig::kMaxLayers); - memcpy(config.ts_rate_decimator, vpx_config->ts_rate_decimator, - sizeof(unsigned int) * Vp8EncoderConfig::kMaxLayers); - config.ts_periodicity = vpx_config->ts_periodicity; - memcpy(config.ts_layer_id, vpx_config->ts_layer_id, - sizeof(unsigned int) * Vp8EncoderConfig::kMaxPeriodicity); - config.rc_target_bitrate = vpx_config->rc_target_bitrate; - config.rc_min_quantizer = vpx_config->rc_min_quantizer; - config.rc_max_quantizer = vpx_config->rc_max_quantizer; +// Allow a newer value to override a current value only if the new value +// is set. +template +bool MaybeSetNewValue(const absl::optional& new_value, + absl::optional* base_value) { + if (new_value.has_value() && new_value != *base_value) { + *base_value = new_value; + return true; + } else { + return false; + } +} - return config; +// Adds configuration from |new_config| to |base_config|. Both configs consist +// of optionals, and only optionals which are set in |new_config| can have +// an effect. (That is, set values in |base_config| cannot be unset.) +// Returns |true| iff any changes were made to |base_config|. +bool MaybeExtendVp8EncoderConfig(const Vp8EncoderConfig& new_config, + Vp8EncoderConfig* base_config) { + bool changes_made = false; + changes_made |= MaybeSetNewValue(new_config.temporal_layer_config, + &base_config->temporal_layer_config); + changes_made |= MaybeSetNewValue(new_config.rc_target_bitrate, + &base_config->rc_target_bitrate); + changes_made |= MaybeSetNewValue(new_config.rc_max_quantizer, + &base_config->rc_max_quantizer); + changes_made |= MaybeSetNewValue(new_config.g_error_resilient, + &base_config->g_error_resilient); + return changes_made; } -static void FillInEncoderConfig(vpx_codec_enc_cfg* vpx_config, - const Vp8EncoderConfig& config) { - vpx_config->ts_number_layers = config.ts_number_layers; - memcpy(vpx_config->ts_target_bitrate, config.ts_target_bitrate, - sizeof(unsigned int) * Vp8EncoderConfig::kMaxLayers); - memcpy(vpx_config->ts_rate_decimator, config.ts_rate_decimator, - sizeof(unsigned int) * Vp8EncoderConfig::kMaxLayers); - vpx_config->ts_periodicity = config.ts_periodicity; - memcpy(vpx_config->ts_layer_id, config.ts_layer_id, - sizeof(unsigned int) * Vp8EncoderConfig::kMaxPeriodicity); - vpx_config->rc_target_bitrate = config.rc_target_bitrate; - vpx_config->rc_min_quantizer = config.rc_min_quantizer; - vpx_config->rc_max_quantizer = config.rc_max_quantizer; - if (config.error_resilient.has_value()) { - vpx_config->g_error_resilient = config.error_resilient.value(); +void ApplyVp8EncoderConfigToVpxConfig(const Vp8EncoderConfig& encoder_config, + vpx_codec_enc_cfg_t* vpx_config) { + if (encoder_config.temporal_layer_config.has_value()) { + const Vp8EncoderConfig::TemporalLayerConfig& ts_config = + encoder_config.temporal_layer_config.value(); + vpx_config->ts_number_layers = ts_config.ts_number_layers; + std::copy(ts_config.ts_target_bitrate.begin(), + ts_config.ts_target_bitrate.end(), + std::begin(vpx_config->ts_target_bitrate)); + std::copy(ts_config.ts_rate_decimator.begin(), + ts_config.ts_rate_decimator.end(), + std::begin(vpx_config->ts_rate_decimator)); + vpx_config->ts_periodicity = ts_config.ts_periodicity; + std::copy(ts_config.ts_layer_id.begin(), ts_config.ts_layer_id.end(), + std::begin(vpx_config->ts_layer_id)); + } + + if (encoder_config.rc_target_bitrate.has_value()) { + vpx_config->rc_target_bitrate = encoder_config.rc_target_bitrate.value(); + } + + if (encoder_config.rc_max_quantizer.has_value()) { + vpx_config->rc_max_quantizer = encoder_config.rc_max_quantizer.value(); } -} -bool UpdateVpxConfiguration(size_t stream_index, - Vp8FrameBufferController* frame_buffer_controller, - vpx_codec_enc_cfg_t* cfg) { - Vp8EncoderConfig config = GetEncoderConfig(cfg); - const bool res = - frame_buffer_controller->UpdateConfiguration(stream_index, &config); - if (res) - FillInEncoderConfig(cfg, config); - return res; + if (encoder_config.g_error_resilient.has_value()) { + vpx_config->g_error_resilient = encoder_config.g_error_resilient.value(); + } } } // namespace @@ -282,6 +299,7 @@ LibvpxVp8Encoder::LibvpxVp8Encoder( cpu_speed_.assign(kMaxSimulcastStreams, cpu_speed_default_); encoders_.reserve(kMaxSimulcastStreams); configurations_.reserve(kMaxSimulcastStreams); + config_overrides_.reserve(kMaxSimulcastStreams); downsampling_factors_.reserve(kMaxSimulcastStreams); } @@ -304,6 +322,7 @@ int LibvpxVp8Encoder::Release() { encoders_.clear(); configurations_.clear(); + config_overrides_.clear(); send_stream_.clear(); cpu_speed_.clear(); @@ -367,8 +386,9 @@ void LibvpxVp8Encoder::SetRates(const RateControlParameters& parameters) { } } - size_t stream_idx = encoders_.size() - 1; - for (size_t i = 0; i < encoders_.size(); ++i, --stream_idx) { + for (size_t i = 0; i < encoders_.size(); ++i) { + const size_t stream_idx = encoders_.size() - 1 - i; + unsigned int target_bitrate_kbps = parameters.bitrate.GetSpatialLayerSum(stream_idx) / 1000; @@ -383,8 +403,7 @@ void LibvpxVp8Encoder::SetRates(const RateControlParameters& parameters) { static_cast(parameters.framerate_fps + 0.5)); } - UpdateVpxConfiguration(stream_idx, frame_buffer_controller_.get(), - &configurations_[i]); + UpdateVpxConfiguration(stream_idx); if (rate_control_settings_.Vp8DynamicRateSettings()) { // Tweak rate control settings based on available network headroom. @@ -486,6 +505,7 @@ int LibvpxVp8Encoder::InitEncode(const VideoCodec* inst, encoded_images_.resize(number_of_streams); encoders_.resize(number_of_streams); configurations_.resize(number_of_streams); + config_overrides_.resize(number_of_streams); downsampling_factors_.resize(number_of_streams); raw_images_.resize(number_of_streams); send_stream_.resize(number_of_streams); @@ -596,7 +616,7 @@ int LibvpxVp8Encoder::InitEncode(const VideoCodec* inst, // Note the order we use is different from webm, we have lowest resolution // at position 0 and they have highest resolution at position 0. - int stream_idx = encoders_.size() - 1; + const size_t stream_idx_cfg_0 = encoders_.size() - 1; SimulcastRateAllocator init_allocator(codec_); VideoBitrateAllocation allocation = init_allocator.GetAllocation( inst->startBitrate * 1000, inst->maxFramerate); @@ -606,18 +626,21 @@ int LibvpxVp8Encoder::InitEncode(const VideoCodec* inst, stream_bitrates.push_back(bitrate); } - configurations_[0].rc_target_bitrate = stream_bitrates[stream_idx]; - if (stream_bitrates[stream_idx] > 0) { + configurations_[0].rc_target_bitrate = stream_bitrates[stream_idx_cfg_0]; + if (stream_bitrates[stream_idx_cfg_0] > 0) { frame_buffer_controller_->OnRatesUpdated( - stream_idx, allocation.GetTemporalLayerAllocation(stream_idx), + stream_idx_cfg_0, + allocation.GetTemporalLayerAllocation(stream_idx_cfg_0), inst->maxFramerate); } - UpdateVpxConfiguration(stream_idx, frame_buffer_controller_.get(), - &configurations_[0]); - configurations_[0].rc_dropframe_thresh = FrameDropThreshold(stream_idx); + frame_buffer_controller_->SetQpLimits(stream_idx_cfg_0, + configurations_[0].rc_min_quantizer, + configurations_[0].rc_max_quantizer); + UpdateVpxConfiguration(stream_idx_cfg_0); + configurations_[0].rc_dropframe_thresh = FrameDropThreshold(stream_idx_cfg_0); - --stream_idx; - for (size_t i = 1; i < encoders_.size(); ++i, --stream_idx) { + for (size_t i = 1; i < encoders_.size(); ++i) { + const size_t stream_idx = encoders_.size() - 1 - i; memcpy(&configurations_[i], &configurations_[0], sizeof(configurations_[0])); @@ -643,8 +666,10 @@ int LibvpxVp8Encoder::InitEncode(const VideoCodec* inst, stream_idx, allocation.GetTemporalLayerAllocation(stream_idx), inst->maxFramerate); } - UpdateVpxConfiguration(stream_idx, frame_buffer_controller_.get(), - &configurations_[i]); + frame_buffer_controller_->SetQpLimits(stream_idx, + configurations_[i].rc_min_quantizer, + configurations_[i].rc_max_quantizer); + UpdateVpxConfiguration(stream_idx); } return InitAndSetControlSettings(); @@ -852,6 +877,27 @@ size_t LibvpxVp8Encoder::SteadyStateSize(int sid, int tid) { 0.5); } +bool LibvpxVp8Encoder::UpdateVpxConfiguration(size_t stream_index) { + RTC_DCHECK(frame_buffer_controller_); + + const size_t config_index = configurations_.size() - 1 - stream_index; + + RTC_DCHECK_LT(config_index, config_overrides_.size()); + Vp8EncoderConfig* config = &config_overrides_[config_index]; + + const Vp8EncoderConfig new_config = + frame_buffer_controller_->UpdateConfiguration(stream_index); + + const bool changes_made = MaybeExtendVp8EncoderConfig(new_config, config); + + // Note that overrides must be applied even if they haven't changed. + RTC_DCHECK_LT(config_index, configurations_.size()); + vpx_codec_enc_cfg_t* vpx_config = &configurations_[config_index]; + ApplyVp8EncoderConfigToVpxConfig(*config, vpx_config); + + return changes_made; +} + int LibvpxVp8Encoder::Encode(const VideoFrame& frame, const std::vector* frame_types) { RTC_DCHECK_EQ(frame.width(), codec_.width); @@ -973,16 +1019,11 @@ int LibvpxVp8Encoder::Encode(const VideoFrame& frame, // Note that streams are defined starting from lowest resolution at // position 0 to highest resolution at position |encoders_.size() - 1|, // whereas |encoder_| is from highest to lowest resolution. - size_t stream_idx = encoders_.size() - 1; - for (size_t i = 0; i < encoders_.size(); ++i, --stream_idx) { - // Allow the layers adapter to temporarily modify the configuration. This - // change isn't stored in configurations_ so change will be discarded at - // the next update. - vpx_codec_enc_cfg_t temp_config; - memcpy(&temp_config, &configurations_[i], sizeof(vpx_codec_enc_cfg_t)); - if (UpdateVpxConfiguration(stream_idx, frame_buffer_controller_.get(), - &temp_config)) { - if (libvpx_->codec_enc_config_set(&encoders_[i], &temp_config)) + for (size_t i = 0; i < encoders_.size(); ++i) { + const size_t stream_idx = encoders_.size() - 1 - i; + + if (UpdateVpxConfiguration(stream_idx)) { + if (libvpx_->codec_enc_config_set(&encoders_[i], &configurations_[i])) return WEBRTC_VIDEO_CODEC_ERROR; } diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h index 0913f5bf48e..1e17096a87c 100644 --- a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h +++ b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h @@ -93,6 +93,8 @@ class LibvpxVp8Encoder : public VideoEncoder { size_t SteadyStateSize(int sid, int tid); + bool UpdateVpxConfiguration(size_t stream_index); + const std::unique_ptr libvpx_; const absl::optional> @@ -117,6 +119,7 @@ class LibvpxVp8Encoder : public VideoEncoder { std::vector encoded_images_; std::vector encoders_; std::vector configurations_; + std::vector config_overrides_; std::vector downsampling_factors_; // Variable frame-rate screencast related fields and methods. diff --git a/modules/video_coding/codecs/vp8/screenshare_layers.cc b/modules/video_coding/codecs/vp8/screenshare_layers.cc index 2f4b4820339..c8ea6ca1c42 100644 --- a/modules/video_coding/codecs/vp8/screenshare_layers.cc +++ b/modules/video_coding/codecs/vp8/screenshare_layers.cc @@ -55,8 +55,6 @@ ScreenshareLayers::ScreenshareLayers(int num_temporal_layers) last_sync_timestamp_(-1), last_emitted_tl0_timestamp_(-1), last_frame_time_ms_(-1), - min_qp_(-1), - max_qp_(-1), max_debt_bytes_(0), encode_framerate_(1000.0f, 1000.0f), // 1 second window, second scale. bitrate_updated_(false), @@ -71,6 +69,24 @@ ScreenshareLayers::~ScreenshareLayers() { UpdateHistograms(); } +void ScreenshareLayers::SetQpLimits(size_t stream_index, + int min_qp, + int max_qp) { + RTC_DCHECK_LT(stream_index, StreamCount()); + // 0 < min_qp <= max_qp + RTC_DCHECK_LT(0, min_qp); + RTC_DCHECK_LE(min_qp, max_qp); + + RTC_DCHECK_EQ(min_qp_.has_value(), max_qp_.has_value()); + if (!min_qp_.has_value()) { + min_qp_ = min_qp; + max_qp_ = max_qp; + } else { + RTC_DCHECK_EQ(min_qp, min_qp_.value()); + RTC_DCHECK_EQ(max_qp, max_qp_.value()); + } +} + size_t ScreenshareLayers::StreamCount() const { return 1; } @@ -478,19 +494,12 @@ uint32_t ScreenshareLayers::GetCodecTargetBitrateKbps() const { return std::max(layers_[0].target_rate_kbps_, target_bitrate_kbps); } -bool ScreenshareLayers::UpdateConfiguration(size_t stream_index, - Vp8EncoderConfig* cfg) { +Vp8EncoderConfig ScreenshareLayers::UpdateConfiguration(size_t stream_index) { RTC_DCHECK_LT(stream_index, StreamCount()); + RTC_DCHECK(min_qp_.has_value()); + RTC_DCHECK(max_qp_.has_value()); - if (min_qp_ == -1 || max_qp_ == -1) { - // Store the valid qp range. This must not change during the lifetime of - // this class. - min_qp_ = cfg->rc_min_quantizer; - max_qp_ = cfg->rc_max_quantizer; - } - - bool cfg_updated = false; - uint32_t target_bitrate_kbps = GetCodecTargetBitrateKbps(); + const uint32_t target_bitrate_kbps = GetCodecTargetBitrateKbps(); // TODO(sprang): We _really_ need to make an overhaul of this class. :( // If we're dropping frames in order to meet a target framerate, adjust the @@ -503,12 +512,16 @@ bool ScreenshareLayers::UpdateConfiguration(size_t stream_index, } if (bitrate_updated_ || - cfg->rc_target_bitrate != encoder_config_bitrate_kbps) { - cfg->rc_target_bitrate = encoder_config_bitrate_kbps; + encoder_config_.rc_target_bitrate != + absl::make_optional(encoder_config_bitrate_kbps)) { + encoder_config_.rc_target_bitrate = encoder_config_bitrate_kbps; // Don't reconfigure qp limits during quality boost frames. if (active_layer_ == -1 || layers_[active_layer_].state != TemporalLayer::State::kQualityBoost) { + const int min_qp = min_qp_.value(); + const int max_qp = max_qp_.value(); + // After a dropped frame, a frame with max qp will be encoded and the // quality will then ramp up from there. To boost the speed of recovery, // encode the next frame with lower max qp, if there is sufficient @@ -517,10 +530,8 @@ bool ScreenshareLayers::UpdateConfiguration(size_t stream_index, // will propagate to TL1. // Currently, reduce max qp by 20% for TL0 and 15% for TL1. if (layers_[1].target_rate_kbps_ >= kMinBitrateKbpsForQpBoost) { - layers_[0].enhanced_max_qp = - min_qp_ + (((max_qp_ - min_qp_) * 80) / 100); - layers_[1].enhanced_max_qp = - min_qp_ + (((max_qp_ - min_qp_) * 85) / 100); + layers_[0].enhanced_max_qp = min_qp + (((max_qp - min_qp) * 80) / 100); + layers_[1].enhanced_max_qp = min_qp + (((max_qp - min_qp) * 85) / 100); } else { layers_[0].enhanced_max_qp = -1; layers_[1].enhanced_max_qp = -1; @@ -538,20 +549,19 @@ bool ScreenshareLayers::UpdateConfiguration(size_t stream_index, } bitrate_updated_ = false; - cfg_updated = true; } // Don't try to update boosts state if not active yet. if (active_layer_ == -1) - return cfg_updated; + return encoder_config_; - if (max_qp_ == -1 || number_of_temporal_layers_ <= 1) - return cfg_updated; + if (number_of_temporal_layers_ <= 1) + return encoder_config_; // If layer is in the quality boost state (following a dropped frame), update // the configuration with the adjusted (lower) qp and set the state back to // normal. - unsigned int adjusted_max_qp = max_qp_; // Set the normal max qp. + unsigned int adjusted_max_qp = max_qp_.value(); // Set the normal max qp. if (layers_[active_layer_].state == TemporalLayer::State::kQualityBoost) { if (layers_[active_layer_].enhanced_max_qp != -1) { // Bitrate is high enough for quality boost, update max qp. @@ -560,14 +570,9 @@ bool ScreenshareLayers::UpdateConfiguration(size_t stream_index, // Regardless of qp, reset the boost state for the next frame. layers_[active_layer_].state = TemporalLayer::State::kNormal; } + encoder_config_.rc_max_quantizer = adjusted_max_qp; - if (adjusted_max_qp == cfg->rc_max_quantizer) - return cfg_updated; - - cfg->rc_max_quantizer = adjusted_max_qp; - cfg_updated = true; - - return cfg_updated; + return encoder_config_; } void ScreenshareLayers::TemporalLayer::UpdateDebt(int64_t delta_ms) { diff --git a/modules/video_coding/codecs/vp8/screenshare_layers.h b/modules/video_coding/codecs/vp8/screenshare_layers.h index 06d45534fe5..d7de52711c8 100644 --- a/modules/video_coding/codecs/vp8/screenshare_layers.h +++ b/modules/video_coding/codecs/vp8/screenshare_layers.h @@ -36,6 +36,8 @@ class ScreenshareLayers final : public Vp8FrameBufferController { explicit ScreenshareLayers(int num_temporal_layers); ~ScreenshareLayers() override; + void SetQpLimits(size_t stream_index, int min_qp, int max_qp) override; + size_t StreamCount() const override; bool SupportsEncoderFrameDropping(size_t stream_index) const override; @@ -50,9 +52,7 @@ class ScreenshareLayers final : public Vp8FrameBufferController { const std::vector& bitrates_bps, int framerate_fps) override; - // Update the encoder configuration with target bitrates or other parameters. - // Returns true iff the configuration was actually modified. - bool UpdateConfiguration(size_t stream_index, Vp8EncoderConfig* cfg) override; + Vp8EncoderConfig UpdateConfiguration(size_t stream_index) override; void OnEncodeDone(size_t stream_index, uint32_t rtp_timestamp, @@ -89,15 +89,18 @@ class ScreenshareLayers final : public Vp8FrameBufferController { bool TimeToSync(int64_t timestamp) const; uint32_t GetCodecTargetBitrateKbps() const; - int number_of_temporal_layers_; + const int number_of_temporal_layers_; + + // TODO(eladalon/sprang): These should be made into const-int set in the ctor. + absl::optional min_qp_; + absl::optional max_qp_; + int active_layer_; int64_t last_timestamp_; int64_t last_sync_timestamp_; int64_t last_emitted_tl0_timestamp_; int64_t last_frame_time_ms_; rtc::TimestampWrapAroundHandler time_wrap_handler_; - int min_qp_; - int max_qp_; uint32_t max_debt_bytes_; std::map pending_frame_configs_; @@ -152,6 +155,8 @@ class ScreenshareLayers final : public Vp8FrameBufferController { int64_t tl1_target_bitrate_sum_ = 0; } stats_; + Vp8EncoderConfig encoder_config_; + // Optional utility used to verify reference validity. std::unique_ptr checker_; }; diff --git a/modules/video_coding/codecs/vp8/screenshare_layers_unittest.cc b/modules/video_coding/codecs/vp8/screenshare_layers_unittest.cc index 032c5280bd1..1f85cb6ec25 100644 --- a/modules/video_coding/codecs/vp8/screenshare_layers_unittest.cc +++ b/modules/video_coding/codecs/vp8/screenshare_layers_unittest.cc @@ -30,7 +30,6 @@ using ::testing::_; using ::testing::ElementsAre; using ::testing::NiceMock; -using ::testing::Return; namespace webrtc { namespace { @@ -93,7 +92,19 @@ class ScreenshareLayerTest : public ::testing::Test { if (tl_config_.drop_frame) { return -1; } - config_updated_ = layers_->UpdateConfiguration(0, &cfg_); + const uint32_t prev_rc_target_bitrate = cfg_.rc_target_bitrate.value_or(-1); + const uint32_t prev_rc_max_quantizer = cfg_.rc_max_quantizer.value_or(-1); + + cfg_ = layers_->UpdateConfiguration(0); + + config_updated_ = + cfg_.temporal_layer_config.has_value() || + (cfg_.rc_target_bitrate.has_value() && + cfg_.rc_target_bitrate.value() != prev_rc_target_bitrate) || + (cfg_.rc_max_quantizer.has_value() && + cfg_.rc_max_quantizer.value() != prev_rc_max_quantizer) || + cfg_.g_error_resilient.has_value(); + int flags = LibvpxVp8Encoder::EncodeFlags(tl_config_); EXPECT_NE(-1, frame_size_); return flags; @@ -110,13 +121,11 @@ class ScreenshareLayerTest : public ::testing::Test { } Vp8EncoderConfig ConfigureBitrates() { - Vp8EncoderConfig vp8_cfg; - memset(&vp8_cfg, 0, sizeof(Vp8EncoderConfig)); - vp8_cfg.rc_min_quantizer = min_qp_; - vp8_cfg.rc_max_quantizer = max_qp_; + layers_->SetQpLimits(0, min_qp_, max_qp_); layers_->OnRatesUpdated(0, kDefault2TlBitratesBps, kFrameRate); - EXPECT_TRUE(layers_->UpdateConfiguration(0, &vp8_cfg)); - frame_size_ = FrameSizeForBitrate(vp8_cfg.rc_target_bitrate); + const Vp8EncoderConfig vp8_cfg = layers_->UpdateConfiguration(0); + EXPECT_TRUE(vp8_cfg.rc_target_bitrate.has_value()); + frame_size_ = FrameSizeForBitrate(vp8_cfg.rc_target_bitrate.value()); return vp8_cfg; } @@ -246,7 +255,7 @@ TEST_F(ScreenshareLayerTest, 2LayersSyncAfterTimeout) { CodecSpecificInfo info; tl_config_ = NextFrameConfig(0, timestamp_); - config_updated_ = layers_->UpdateConfiguration(0, &cfg_); + cfg_ = layers_->UpdateConfiguration(0); // Simulate TL1 being at least 8 qp steps better. if (tl_config_.packetizer_temporal_idx == 0) { @@ -394,7 +403,7 @@ TEST_F(ScreenshareLayerTest, TargetBitrateCappedByTL0) { const std::vector layer_rates = {kTl0_kbps * 1000, (kTl1_kbps - kTl0_kbps) * 1000}; layers_->OnRatesUpdated(0, layer_rates, kFrameRate); - EXPECT_TRUE(layers_->UpdateConfiguration(0, &cfg_)); + cfg_ = layers_->UpdateConfiguration(0); EXPECT_EQ(static_cast( ScreenshareLayers::kMaxTL0FpsReduction * kTl0_kbps + 0.5), @@ -407,7 +416,7 @@ TEST_F(ScreenshareLayerTest, TargetBitrateCappedByTL1) { const std::vector layer_rates = {kTl0_kbps * 1000, (kTl1_kbps - kTl0_kbps) * 1000}; layers_->OnRatesUpdated(0, layer_rates, kFrameRate); - EXPECT_TRUE(layers_->UpdateConfiguration(0, &cfg_)); + cfg_ = layers_->UpdateConfiguration(0); EXPECT_EQ(static_cast( kTl1_kbps / ScreenshareLayers::kAcceptableTargetOvershoot), @@ -418,7 +427,7 @@ TEST_F(ScreenshareLayerTest, TargetBitrateBelowTL0) { const int kTl0_kbps = 100; const std::vector layer_rates = {kTl0_kbps * 1000}; layers_->OnRatesUpdated(0, layer_rates, kFrameRate); - EXPECT_TRUE(layers_->UpdateConfiguration(0, &cfg_)); + cfg_ = layers_->UpdateConfiguration(0); EXPECT_EQ(static_cast(kTl0_kbps), cfg_.rc_target_bitrate); } @@ -484,7 +493,7 @@ TEST_F(ScreenshareLayerTest, RespectsMaxIntervalBetweenFrames) { const std::vector layer_rates = {kLowBitrateKbps * 1000}; layers_->OnRatesUpdated(0, layer_rates, kFrameRate); - layers_->UpdateConfiguration(0, &cfg_); + cfg_ = layers_->UpdateConfiguration(0); EXPECT_EQ(kTl0Flags, LibvpxVp8Encoder::EncodeFlags(NextFrameConfig(0, kStartTimestamp))); @@ -527,7 +536,7 @@ TEST_F(ScreenshareLayerTest, UpdatesHistograms) { } int flags = LibvpxVp8Encoder::EncodeFlags(tl_config_); if (flags != -1) - layers_->UpdateConfiguration(0, &cfg_); + cfg_ = layers_->UpdateConfiguration(0); if (timestamp >= kTimestampDelta5Fps * 5 && !overshoot && flags != -1) { // Simulate one overshoot. @@ -592,13 +601,6 @@ TEST_F(ScreenshareLayerTest, UpdatesHistograms) { kDefaultTl1BitrateKbps)); } -TEST_F(ScreenshareLayerTest, AllowsUpdateConfigBeforeSetRates) { - layers_.reset(new ScreenshareLayers(2)); - // New layer instance, OnRatesUpdated() never called. - // UpdateConfiguration() call should not cause crash. - layers_->UpdateConfiguration(0, &cfg_); -} - TEST_F(ScreenshareLayerTest, RespectsConfiguredFramerate) { int64_t kTestSpanMs = 2000; int64_t kFrameIntervalsMs = 1000 / kFrameRate; @@ -654,7 +656,7 @@ TEST_F(ScreenshareLayerTest, 2LayersSyncAtOvershootDrop) { // Simulate overshoot of this frame. layers_->OnEncodeDone(0, timestamp_, 0, false, 0, nullptr); - config_updated_ = layers_->UpdateConfiguration(0, &cfg_); + cfg_ = layers_->UpdateConfiguration(0); EXPECT_EQ(kTl1SyncFlags, LibvpxVp8Encoder::EncodeFlags(tl_config_)); CodecSpecificInfo new_info; @@ -687,7 +689,8 @@ TEST_F(ScreenshareLayerTest, DropOnTooShortFrameInterval) { TEST_F(ScreenshareLayerTest, AdjustsBitrateWhenDroppingFrames) { const uint32_t kTimestampDelta10Fps = kTimestampDelta5Fps / 2; const int kNumFrames = 30; - uint32_t default_bitrate = cfg_.rc_target_bitrate; + ASSERT_TRUE(cfg_.rc_target_bitrate.has_value()); + const uint32_t default_bitrate = cfg_.rc_target_bitrate.value(); layers_->OnRatesUpdated(0, kDefault2TlBitratesBps, 10); int num_dropped_frames = 0; @@ -696,7 +699,7 @@ TEST_F(ScreenshareLayerTest, AdjustsBitrateWhenDroppingFrames) { ++num_dropped_frames; timestamp_ += kTimestampDelta10Fps; } - layers_->UpdateConfiguration(0, &cfg_); + cfg_ = layers_->UpdateConfiguration(0); EXPECT_EQ(num_dropped_frames, kNumFrames / 2); EXPECT_EQ(cfg_.rc_target_bitrate, default_bitrate * 2); @@ -705,20 +708,20 @@ TEST_F(ScreenshareLayerTest, AdjustsBitrateWhenDroppingFrames) { TEST_F(ScreenshareLayerTest, UpdatesConfigurationAfterRateChange) { // Set inital rate again, no need to update configuration. layers_->OnRatesUpdated(0, kDefault2TlBitratesBps, kFrameRate); - EXPECT_FALSE(layers_->UpdateConfiguration(0, &cfg_)); + cfg_ = layers_->UpdateConfiguration(0); // Rate changed, now update config. std::vector bitrates = kDefault2TlBitratesBps; bitrates[1] -= 100000; layers_->OnRatesUpdated(0, bitrates, 5); - EXPECT_TRUE(layers_->UpdateConfiguration(0, &cfg_)); + cfg_ = layers_->UpdateConfiguration(0); // Changed rate, but then set changed rate again before trying to update // configuration, update should still apply. bitrates[1] -= 100000; layers_->OnRatesUpdated(0, bitrates, 5); layers_->OnRatesUpdated(0, bitrates, 5); - EXPECT_TRUE(layers_->UpdateConfiguration(0, &cfg_)); + cfg_ = layers_->UpdateConfiguration(0); } TEST_F(ScreenshareLayerTest, MaxQpRestoredAfterDoubleDrop) { @@ -744,7 +747,8 @@ TEST_F(ScreenshareLayerTest, MaxQpRestoredAfterDoubleDrop) { EXPECT_EQ(kTl1Flags, SkipUntilTlAndSync(1, false)); EXPECT_TRUE(config_updated_); EXPECT_LT(cfg_.rc_max_quantizer, max_qp_); - uint32_t adjusted_qp = cfg_.rc_max_quantizer; + ASSERT_TRUE(cfg_.rc_max_quantizer.has_value()); + const uint32_t adjusted_qp = cfg_.rc_max_quantizer.value(); // Simulate overshoot of this frame. layers_->OnEncodeDone(0, timestamp_, 0, false, -1, nullptr); diff --git a/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc b/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc index eefb743d63f..471fcd0d609 100644 --- a/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc +++ b/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc @@ -15,6 +15,7 @@ #include #include +#include "api/video_codecs/vp8_frame_buffer_controller.h" #include "api/video_codecs/vp8_frame_config.h" #include "api/video_codecs/vp8_temporal_layers.h" #include "rtc_base/checks.h" @@ -39,7 +40,7 @@ class MockTemporalLayers : public Vp8FrameBufferController { public: MOCK_METHOD2(NextFrameConfig, Vp8FrameConfig(size_t, uint32_t)); MOCK_METHOD3(OnRatesUpdated, void(size_t, const std::vector&, int)); - MOCK_METHOD2(UpdateConfiguration, bool(size_t, Vp8EncoderConfig*)); + MOCK_METHOD1(UpdateConfiguration, Vp8EncoderConfig(size_t)); MOCK_METHOD6(OnEncodeDone, void(size_t, uint32_t, size_t, bool, int, CodecSpecificInfo*)); MOCK_METHOD4(FrameEncoded, void(size_t, uint32_t, size_t, int)); diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 167988b79fa..1fc4df70aef 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -734,9 +734,7 @@ void VideoStreamEncoder::ReconfigureEncoder() { } // Reset (release existing encoder) if one exists and anything except - // start bitrate or max framerate has changed. Don't call Release() if - // |pending_encoder_creation_| as that means this is a new encoder - // that has not yet been initialized. + // start bitrate or max framerate has changed. const bool reset_required = RequiresEncoderReset(codec, send_codec_); send_codec_ = codec;