diff --git a/api/video/video_bitrate_allocator.cc b/api/video/video_bitrate_allocator.cc index f4e843b348..8ad5f75244 100644 --- a/api/video/video_bitrate_allocator.cc +++ b/api/video/video_bitrate_allocator.cc @@ -49,6 +49,4 @@ VideoBitrateAllocation VideoBitrateAllocator::Allocate( return GetAllocation(parameters.total_bitrate.bps(), parameters.framerate); } -void VideoBitrateAllocator::SetLegacyConferenceMode(bool enabled) {} - } // namespace webrtc diff --git a/api/video/video_bitrate_allocator.h b/api/video/video_bitrate_allocator.h index fdc86dbc57..04de04c1b0 100644 --- a/api/video/video_bitrate_allocator.h +++ b/api/video/video_bitrate_allocator.h @@ -40,10 +40,6 @@ class VideoBitrateAllocator { virtual VideoBitrateAllocation Allocate( VideoBitrateAllocationParameters parameters); - - // Deprecated: Only used to work around issues with the legacy conference - // screenshare mode and shouldn't be needed by any subclasses. - virtual void SetLegacyConferenceMode(bool enabled); }; class VideoBitrateAllocationObserver { diff --git a/api/video_codecs/video_codec.cc b/api/video_codecs/video_codec.cc index 65c7243c00..d03082b91e 100644 --- a/api/video_codecs/video_codec.cc +++ b/api/video_codecs/video_codec.cc @@ -83,7 +83,6 @@ VideoCodec::VideoCodec() mode(VideoCodecMode::kRealtimeVideo), expect_encode_from_texture(false), timing_frame_thresholds({0, 0}), - legacy_conference_mode(false), codec_specific_() {} VideoCodecVP8* VideoCodec::VP8() { diff --git a/api/video_codecs/video_codec.h b/api/video_codecs/video_codec.h index dd3d344084..c07fae9b8b 100644 --- a/api/video_codecs/video_codec.h +++ b/api/video_codecs/video_codec.h @@ -146,9 +146,6 @@ class RTC_EXPORT VideoCodec { uint16_t outlier_ratio_percent; } timing_frame_thresholds; - // Legacy Google conference mode flag for simulcast screenshare - bool legacy_conference_mode; - bool operator==(const VideoCodec& other) const = delete; bool operator!=(const VideoCodec& other) const = delete; diff --git a/api/video_codecs/video_encoder_config.cc b/api/video_codecs/video_encoder_config.cc index 19b0135641..6efcbf2bdd 100644 --- a/api/video_codecs/video_encoder_config.cc +++ b/api/video_codecs/video_encoder_config.cc @@ -55,8 +55,7 @@ VideoEncoderConfig::VideoEncoderConfig() min_transmit_bitrate_bps(0), max_bitrate_bps(0), bitrate_priority(1.0), - number_of_streams(0), - legacy_conference_mode(false) {} + number_of_streams(0) {} VideoEncoderConfig::VideoEncoderConfig(VideoEncoderConfig&&) = default; diff --git a/api/video_codecs/video_encoder_config.h b/api/video_codecs/video_encoder_config.h index 7a59dacd1b..ef8db100a3 100644 --- a/api/video_codecs/video_encoder_config.h +++ b/api/video_codecs/video_encoder_config.h @@ -176,9 +176,6 @@ class VideoEncoderConfig { // Max number of encoded VideoStreams to produce. size_t number_of_streams; - // Legacy Google conference mode flag for simulcast screenshare - bool legacy_conference_mode; - private: // Access to the copy constructor is private to force use of the Copy() // method for those exceptional cases where we do use it. diff --git a/media/engine/simulcast_encoder_adapter.cc b/media/engine/simulcast_encoder_adapter.cc index 60baed9daa..863ccc756e 100644 --- a/media/engine/simulcast_encoder_adapter.cc +++ b/media/engine/simulcast_encoder_adapter.cc @@ -619,10 +619,6 @@ void SimulcastEncoderAdapter::PopulateStreamCodec( // TODO(ronghuawu): what to do with targetBitrate. stream_codec->startBitrate = start_bitrate_kbps; - - // Legacy screenshare mode is only enabled for the first simulcast layer - stream_codec->legacy_conference_mode = - inst.legacy_conference_mode && stream_index == 0; } bool SimulcastEncoderAdapter::Initialized() const { diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index a075f7b58a..26fa335cf6 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -2399,8 +2399,6 @@ WebRtcVideoChannel::WebRtcVideoSendStream::CreateVideoEncoderConfig( } } - encoder_config.legacy_conference_mode = parameters_.conference_mode; - int max_qp = kDefaultQpMax; codec.GetParam(kCodecParamMaxQuantization, &max_qp); encoder_config.video_stream_factory = @@ -3396,7 +3394,7 @@ std::vector EncoderStreamFactory::CreateEncoderStreams( ((absl::EqualsIgnoreCase(codec_name_, kVp8CodecName) || absl::EqualsIgnoreCase(codec_name_, kH264CodecName)) && is_screenshare_ && conference_mode_)) { - return CreateSimulcastOrConferenceModeScreenshareStreams( + return CreateSimulcastOrConfereceModeScreenshareStreams( width, height, encoder_config, experimental_min_bitrate); } @@ -3485,7 +3483,7 @@ EncoderStreamFactory::CreateDefaultVideoStreams( } std::vector -EncoderStreamFactory::CreateSimulcastOrConferenceModeScreenshareStreams( +EncoderStreamFactory::CreateSimulcastOrConfereceModeScreenshareStreams( int width, int height, const webrtc::VideoEncoderConfig& encoder_config, diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h index 9737dd8294..21033ffae7 100644 --- a/media/engine/webrtc_video_engine.h +++ b/media/engine/webrtc_video_engine.h @@ -661,7 +661,7 @@ class EncoderStreamFactory const absl::optional& experimental_min_bitrate) const; std::vector - CreateSimulcastOrConferenceModeScreenshareStreams( + CreateSimulcastOrConfereceModeScreenshareStreams( int width, int height, const webrtc::VideoEncoderConfig& encoder_config, diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc index 9b64d1654f..ac04bc3e50 100644 --- a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc +++ b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc @@ -879,7 +879,7 @@ size_t LibvpxVp8Encoder::SteadyStateSize(int sid, int tid) { const int encoder_id = encoders_.size() - 1 - sid; size_t bitrate_bps; float fps; - if ((SimulcastUtility::IsConferenceModeScreenshare(codec_) && sid == 0) || + if (SimulcastUtility::IsConferenceModeScreenshare(codec_) || vpx_configs_[encoder_id].ts_number_layers <= 1) { // In conference screenshare there's no defined per temporal layer bitrate // and framerate. diff --git a/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc b/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc index d3e4a1739e..51595260dd 100644 --- a/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc +++ b/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc @@ -523,7 +523,6 @@ TEST_F(TestVp8Impl, KeepsTimestampOnReencode) { codec_settings_.maxBitrate = 1000; codec_settings_.mode = VideoCodecMode::kScreensharing; codec_settings_.VP8()->numberOfTemporalLayers = 2; - codec_settings_.legacy_conference_mode = true; EXPECT_CALL(*vpx, img_wrap(_, _, _, _, _, _)) .WillOnce(Invoke([](vpx_image_t* img, vpx_img_fmt_t fmt, unsigned int d_w, @@ -654,7 +653,6 @@ TEST_F(TestVp8Impl, GetEncoderInfoFpsAllocationScreenshareLayers) { codec_settings_.simulcastStream[0].maxBitrate = kLegacyScreenshareTl1BitrateKbps; codec_settings_.simulcastStream[0].numberOfTemporalLayers = 2; - codec_settings_.legacy_conference_mode = true; EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, encoder_->InitEncode(&codec_settings_, kSettings)); diff --git a/modules/video_coding/utility/simulcast_rate_allocator.cc b/modules/video_coding/utility/simulcast_rate_allocator.cc index 13de8755d0..fef74cdb45 100644 --- a/modules/video_coding/utility/simulcast_rate_allocator.cc +++ b/modules/video_coding/utility/simulcast_rate_allocator.cc @@ -61,8 +61,7 @@ float SimulcastRateAllocator::GetTemporalRateAllocation( SimulcastRateAllocator::SimulcastRateAllocator(const VideoCodec& codec) : codec_(codec), stable_rate_settings_(StableTargetRateExperiment::ParseFromFieldTrials()), - rate_control_settings_(RateControlSettings::ParseFromFieldTrials()), - legacy_conference_mode_(false) {} + rate_control_settings_(RateControlSettings::ParseFromFieldTrials()) {} SimulcastRateAllocator::~SimulcastRateAllocator() = default; @@ -229,7 +228,12 @@ void SimulcastRateAllocator::DistributeAllocationToTemporalLayers( uint32_t max_bitrate_kbps; // Legacy temporal-layered only screenshare, or simulcast screenshare // with legacy mode for simulcast stream 0. - if (legacy_conference_mode_ && simulcast_id == 0) { + const bool conference_screenshare_mode = + codec_.mode == VideoCodecMode::kScreensharing && + ((num_spatial_streams == 1 && num_temporal_streams == 2) || // Legacy. + (num_spatial_streams > 1 && simulcast_id == 0 && + num_temporal_streams == 2)); // Simulcast. + if (conference_screenshare_mode) { // TODO(holmer): This is a "temporary" hack for screensharing, where we // interpret the startBitrate as the encoder target bitrate. This is // to allow for a different max bitrate, so if the codec can't meet @@ -249,7 +253,7 @@ void SimulcastRateAllocator::DistributeAllocationToTemporalLayers( if (num_temporal_streams == 1) { tl_allocation.push_back(target_bitrate_kbps); } else { - if (legacy_conference_mode_ && simulcast_id == 0) { + if (conference_screenshare_mode) { tl_allocation = ScreenshareTemporalLayerAllocation( target_bitrate_kbps, max_bitrate_kbps, simulcast_id); } else { @@ -334,8 +338,4 @@ int SimulcastRateAllocator::NumTemporalStreams(size_t simulcast_id) const { : codec_.simulcastStream[simulcast_id].numberOfTemporalLayers); } -void SimulcastRateAllocator::SetLegacyConferenceMode(bool enabled) { - legacy_conference_mode_ = enabled; -} - } // namespace webrtc diff --git a/modules/video_coding/utility/simulcast_rate_allocator.h b/modules/video_coding/utility/simulcast_rate_allocator.h index 9b2f9696e6..d9d9627352 100644 --- a/modules/video_coding/utility/simulcast_rate_allocator.h +++ b/modules/video_coding/utility/simulcast_rate_allocator.h @@ -38,8 +38,6 @@ class SimulcastRateAllocator : public VideoBitrateAllocator { int temporal_id, bool base_heavy_tl3_alloc); - void SetLegacyConferenceMode(bool mode) override; - private: void DistributeAllocationToSimulcastLayers( DataRate total_bitrate, @@ -60,7 +58,6 @@ class SimulcastRateAllocator : public VideoBitrateAllocator { const StableTargetRateExperiment stable_rate_settings_; const RateControlSettings rate_control_settings_; std::vector stream_enabled_; - bool legacy_conference_mode_; RTC_DISALLOW_COPY_AND_ASSIGN(SimulcastRateAllocator); }; diff --git a/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc b/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc index 69c14ff78e..871e5a1692 100644 --- a/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc +++ b/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc @@ -88,9 +88,8 @@ class SimulcastRateAllocatorTest : public ::testing::TestWithParam { EXPECT_EQ(sum, actual.get_sum_bps()); } - void CreateAllocator(bool legacy_conference_mode = false) { + void CreateAllocator() { allocator_.reset(new SimulcastRateAllocator(codec_)); - allocator_->SetLegacyConferenceMode(legacy_conference_mode); } void SetupCodec3SL3TL(const std::vector& active_streams) { @@ -670,9 +669,9 @@ INSTANTIATE_TEST_SUITE_P(ScreenshareTest, ScreenshareRateAllocationTest, ::testing::Bool()); -TEST_P(ScreenshareRateAllocationTest, ConferenceBitrateBelowTl0) { +TEST_P(ScreenshareRateAllocationTest, BitrateBelowTl0) { SetupConferenceScreenshare(GetParam()); - CreateAllocator(true); + CreateAllocator(); VideoBitrateAllocation allocation = allocator_->Allocate(VideoBitrateAllocationParameters( @@ -685,9 +684,9 @@ TEST_P(ScreenshareRateAllocationTest, ConferenceBitrateBelowTl0) { EXPECT_EQ(allocation.is_bw_limited(), GetParam()); } -TEST_P(ScreenshareRateAllocationTest, ConferenceBitrateAboveTl0) { +TEST_P(ScreenshareRateAllocationTest, BitrateAboveTl0) { SetupConferenceScreenshare(GetParam()); - CreateAllocator(true); + CreateAllocator(); uint32_t target_bitrate_kbps = (kLegacyScreenshareTargetBitrateKbps + kLegacyScreenshareMaxBitrateKbps) / @@ -705,10 +704,10 @@ TEST_P(ScreenshareRateAllocationTest, ConferenceBitrateAboveTl0) { EXPECT_EQ(allocation.is_bw_limited(), GetParam()); } -TEST_F(ScreenshareRateAllocationTest, ConferenceBitrateAboveTl1) { +TEST_F(ScreenshareRateAllocationTest, BitrateAboveTl1) { // This test is only for the non-simulcast case. SetupConferenceScreenshare(false); - CreateAllocator(true); + CreateAllocator(); VideoBitrateAllocation allocation = allocator_->Allocate(VideoBitrateAllocationParameters( diff --git a/modules/video_coding/utility/simulcast_utility.cc b/modules/video_coding/utility/simulcast_utility.cc index 043be3d5c3..58cb991155 100644 --- a/modules/video_coding/utility/simulcast_utility.cc +++ b/modules/video_coding/utility/simulcast_utility.cc @@ -84,7 +84,16 @@ bool SimulcastUtility::ValidSimulcastParameters(const VideoCodec& codec, } bool SimulcastUtility::IsConferenceModeScreenshare(const VideoCodec& codec) { - return codec.legacy_conference_mode; + if (codec.mode != VideoCodecMode::kScreensharing || + NumberOfTemporalLayers(codec, 0) != 2) { + return false; + } + + // Fixed default bitrates for legacy screenshare layers mode. + return (codec.numberOfSimulcastStreams == 0 && codec.maxBitrate == 1000) || + (codec.numberOfSimulcastStreams >= 1 && + codec.simulcastStream[0].maxBitrate == 1000 && + codec.simulcastStream[0].targetBitrate == 200); } int SimulcastUtility::NumberOfTemporalLayers(const VideoCodec& codec, diff --git a/modules/video_coding/video_codec_initializer.cc b/modules/video_coding/video_codec_initializer.cc index ccc435a7c4..7f36f99f89 100644 --- a/modules/video_coding/video_codec_initializer.cc +++ b/modules/video_coding/video_codec_initializer.cc @@ -274,8 +274,6 @@ VideoCodec VideoCodecInitializer::VideoEncoderConfigToVideoCodec( } } - video_codec.legacy_conference_mode = config.legacy_conference_mode; - return video_codec; } diff --git a/modules/video_coding/video_codec_initializer_unittest.cc b/modules/video_coding/video_codec_initializer_unittest.cc index 686396c51a..d5a18f0413 100644 --- a/modules/video_coding/video_codec_initializer_unittest.cc +++ b/modules/video_coding/video_codec_initializer_unittest.cc @@ -42,8 +42,7 @@ static const uint32_t kDefaultTargetBitrateBps = 2000000; static const uint32_t kDefaultMaxBitrateBps = 2000000; static const uint32_t kDefaultMinTransmitBitrateBps = 400000; static const int kDefaultMaxQp = 48; -static const uint32_t kScreenshareTl0BitrateBps = 120000; -static const uint32_t kScreenshareConferenceTl0BitrateBps = 200000; +static const uint32_t kScreenshareTl0BitrateBps = 200000; static const uint32_t kScreenshareCodecTargetBitrateBps = 200000; static const uint32_t kScreenshareDefaultFramerate = 5; // Bitrates for the temporal layers of the higher screenshare simulcast stream. @@ -127,7 +126,7 @@ class VideoCodecInitializerTest : public ::testing::Test { VideoStream DefaultScreenshareStream() { VideoStream stream = DefaultStream(); stream.min_bitrate_bps = 30000; - stream.target_bitrate_bps = kScreenshareCodecTargetBitrateBps; + stream.target_bitrate_bps = kScreenshareTl0BitrateBps; stream.max_bitrate_bps = 1000000; stream.max_framerate = kScreenshareDefaultFramerate; stream.num_temporal_layers = 2; @@ -175,23 +174,6 @@ TEST_F(VideoCodecInitializerTest, SingleStreamVp8ScreenshareInactive) { EXPECT_EQ(0U, bitrate_allocation.get_sum_bps()); } -TEST_F(VideoCodecInitializerTest, TemporalLayeredVp8ScreenshareConference) { - SetUpFor(VideoCodecType::kVideoCodecVP8, 1, 2, true); - streams_.push_back(DefaultScreenshareStream()); - EXPECT_TRUE(InitializeCodec()); - bitrate_allocator_->SetLegacyConferenceMode(true); - - EXPECT_EQ(1u, codec_out_.numberOfSimulcastStreams); - EXPECT_EQ(2u, codec_out_.VP8()->numberOfTemporalLayers); - VideoBitrateAllocation bitrate_allocation = - bitrate_allocator_->Allocate(VideoBitrateAllocationParameters( - kScreenshareCodecTargetBitrateBps, kScreenshareDefaultFramerate)); - EXPECT_EQ(kScreenshareCodecTargetBitrateBps, - bitrate_allocation.get_sum_bps()); - EXPECT_EQ(kScreenshareConferenceTl0BitrateBps, - bitrate_allocation.GetBitrate(0, 0)); -} - TEST_F(VideoCodecInitializerTest, TemporalLayeredVp8Screenshare) { SetUpFor(VideoCodecType::kVideoCodecVP8, 1, 2, true); streams_.push_back(DefaultScreenshareStream()); diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index f38d2a2990..80b79635f1 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -780,8 +780,6 @@ void VideoStreamEncoder::ReconfigureEncoder() { rate_allocator_ = settings_.bitrate_allocator_factory->CreateVideoBitrateAllocator(codec); - rate_allocator_->SetLegacyConferenceMode( - encoder_config_.legacy_conference_mode); // Reset (release existing encoder) if one exists and anything except // start bitrate or max framerate has changed.