diff --git a/api/video/video_bitrate_allocator.cc b/api/video/video_bitrate_allocator.cc index 8ad5f75244..f4e843b348 100644 --- a/api/video/video_bitrate_allocator.cc +++ b/api/video/video_bitrate_allocator.cc @@ -49,4 +49,6 @@ 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 04de04c1b0..fdc86dbc57 100644 --- a/api/video/video_bitrate_allocator.h +++ b/api/video/video_bitrate_allocator.h @@ -40,6 +40,10 @@ 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 d03082b91e..65c7243c00 100644 --- a/api/video_codecs/video_codec.cc +++ b/api/video_codecs/video_codec.cc @@ -83,6 +83,7 @@ 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 c07fae9b8b..dd3d344084 100644 --- a/api/video_codecs/video_codec.h +++ b/api/video_codecs/video_codec.h @@ -146,6 +146,9 @@ 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 6efcbf2bdd..19b0135641 100644 --- a/api/video_codecs/video_encoder_config.cc +++ b/api/video_codecs/video_encoder_config.cc @@ -55,7 +55,8 @@ VideoEncoderConfig::VideoEncoderConfig() min_transmit_bitrate_bps(0), max_bitrate_bps(0), bitrate_priority(1.0), - number_of_streams(0) {} + number_of_streams(0), + legacy_conference_mode(false) {} VideoEncoderConfig::VideoEncoderConfig(VideoEncoderConfig&&) = default; diff --git a/api/video_codecs/video_encoder_config.h b/api/video_codecs/video_encoder_config.h index ef8db100a3..7a59dacd1b 100644 --- a/api/video_codecs/video_encoder_config.h +++ b/api/video_codecs/video_encoder_config.h @@ -176,6 +176,9 @@ 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 863ccc756e..60baed9daa 100644 --- a/media/engine/simulcast_encoder_adapter.cc +++ b/media/engine/simulcast_encoder_adapter.cc @@ -619,6 +619,10 @@ 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 26fa335cf6..a075f7b58a 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -2399,6 +2399,8 @@ 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 = @@ -3394,7 +3396,7 @@ std::vector EncoderStreamFactory::CreateEncoderStreams( ((absl::EqualsIgnoreCase(codec_name_, kVp8CodecName) || absl::EqualsIgnoreCase(codec_name_, kH264CodecName)) && is_screenshare_ && conference_mode_)) { - return CreateSimulcastOrConfereceModeScreenshareStreams( + return CreateSimulcastOrConferenceModeScreenshareStreams( width, height, encoder_config, experimental_min_bitrate); } @@ -3483,7 +3485,7 @@ EncoderStreamFactory::CreateDefaultVideoStreams( } std::vector -EncoderStreamFactory::CreateSimulcastOrConfereceModeScreenshareStreams( +EncoderStreamFactory::CreateSimulcastOrConferenceModeScreenshareStreams( 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 21033ffae7..9737dd8294 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 - CreateSimulcastOrConfereceModeScreenshareStreams( + CreateSimulcastOrConferenceModeScreenshareStreams( 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 ac04bc3e50..9b64d1654f 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_) || + if ((SimulcastUtility::IsConferenceModeScreenshare(codec_) && sid == 0) || 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 51595260dd..d3e4a1739e 100644 --- a/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc +++ b/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc @@ -523,6 +523,7 @@ 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, @@ -653,6 +654,7 @@ 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 fef74cdb45..13de8755d0 100644 --- a/modules/video_coding/utility/simulcast_rate_allocator.cc +++ b/modules/video_coding/utility/simulcast_rate_allocator.cc @@ -61,7 +61,8 @@ float SimulcastRateAllocator::GetTemporalRateAllocation( SimulcastRateAllocator::SimulcastRateAllocator(const VideoCodec& codec) : codec_(codec), stable_rate_settings_(StableTargetRateExperiment::ParseFromFieldTrials()), - rate_control_settings_(RateControlSettings::ParseFromFieldTrials()) {} + rate_control_settings_(RateControlSettings::ParseFromFieldTrials()), + legacy_conference_mode_(false) {} SimulcastRateAllocator::~SimulcastRateAllocator() = default; @@ -228,12 +229,7 @@ void SimulcastRateAllocator::DistributeAllocationToTemporalLayers( uint32_t max_bitrate_kbps; // Legacy temporal-layered only screenshare, or simulcast screenshare // with legacy mode for simulcast stream 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) { + if (legacy_conference_mode_ && simulcast_id == 0) { // 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 @@ -253,7 +249,7 @@ void SimulcastRateAllocator::DistributeAllocationToTemporalLayers( if (num_temporal_streams == 1) { tl_allocation.push_back(target_bitrate_kbps); } else { - if (conference_screenshare_mode) { + if (legacy_conference_mode_ && simulcast_id == 0) { tl_allocation = ScreenshareTemporalLayerAllocation( target_bitrate_kbps, max_bitrate_kbps, simulcast_id); } else { @@ -338,4 +334,8 @@ 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 d9d9627352..9b2f9696e6 100644 --- a/modules/video_coding/utility/simulcast_rate_allocator.h +++ b/modules/video_coding/utility/simulcast_rate_allocator.h @@ -38,6 +38,8 @@ class SimulcastRateAllocator : public VideoBitrateAllocator { int temporal_id, bool base_heavy_tl3_alloc); + void SetLegacyConferenceMode(bool mode) override; + private: void DistributeAllocationToSimulcastLayers( DataRate total_bitrate, @@ -58,6 +60,7 @@ 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 871e5a1692..69c14ff78e 100644 --- a/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc +++ b/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc @@ -88,8 +88,9 @@ class SimulcastRateAllocatorTest : public ::testing::TestWithParam { EXPECT_EQ(sum, actual.get_sum_bps()); } - void CreateAllocator() { + void CreateAllocator(bool legacy_conference_mode = false) { allocator_.reset(new SimulcastRateAllocator(codec_)); + allocator_->SetLegacyConferenceMode(legacy_conference_mode); } void SetupCodec3SL3TL(const std::vector& active_streams) { @@ -669,9 +670,9 @@ INSTANTIATE_TEST_SUITE_P(ScreenshareTest, ScreenshareRateAllocationTest, ::testing::Bool()); -TEST_P(ScreenshareRateAllocationTest, BitrateBelowTl0) { +TEST_P(ScreenshareRateAllocationTest, ConferenceBitrateBelowTl0) { SetupConferenceScreenshare(GetParam()); - CreateAllocator(); + CreateAllocator(true); VideoBitrateAllocation allocation = allocator_->Allocate(VideoBitrateAllocationParameters( @@ -684,9 +685,9 @@ TEST_P(ScreenshareRateAllocationTest, BitrateBelowTl0) { EXPECT_EQ(allocation.is_bw_limited(), GetParam()); } -TEST_P(ScreenshareRateAllocationTest, BitrateAboveTl0) { +TEST_P(ScreenshareRateAllocationTest, ConferenceBitrateAboveTl0) { SetupConferenceScreenshare(GetParam()); - CreateAllocator(); + CreateAllocator(true); uint32_t target_bitrate_kbps = (kLegacyScreenshareTargetBitrateKbps + kLegacyScreenshareMaxBitrateKbps) / @@ -704,10 +705,10 @@ TEST_P(ScreenshareRateAllocationTest, BitrateAboveTl0) { EXPECT_EQ(allocation.is_bw_limited(), GetParam()); } -TEST_F(ScreenshareRateAllocationTest, BitrateAboveTl1) { +TEST_F(ScreenshareRateAllocationTest, ConferenceBitrateAboveTl1) { // This test is only for the non-simulcast case. SetupConferenceScreenshare(false); - CreateAllocator(); + CreateAllocator(true); VideoBitrateAllocation allocation = allocator_->Allocate(VideoBitrateAllocationParameters( diff --git a/modules/video_coding/utility/simulcast_utility.cc b/modules/video_coding/utility/simulcast_utility.cc index 58cb991155..a407483edd 100644 --- a/modules/video_coding/utility/simulcast_utility.cc +++ b/modules/video_coding/utility/simulcast_utility.cc @@ -84,16 +84,8 @@ bool SimulcastUtility::ValidSimulcastParameters(const VideoCodec& codec, } bool SimulcastUtility::IsConferenceModeScreenshare(const VideoCodec& codec) { - 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); + return codec.mode == VideoCodecMode::kScreensharing && + codec.legacy_conference_mode; } 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 7f36f99f89..4023f6ba9e 100644 --- a/modules/video_coding/video_codec_initializer.cc +++ b/modules/video_coding/video_codec_initializer.cc @@ -68,6 +68,10 @@ VideoCodec VideoCodecInitializer::VideoEncoderConfigToVideoCodec( break; } + video_codec.legacy_conference_mode = + config.content_type == VideoEncoderConfig::ContentType::kScreen && + config.legacy_conference_mode; + // TODO(nisse): The plType field should be deleted. Luckily, our // callers don't need it. video_codec.plType = 0; diff --git a/modules/video_coding/video_codec_initializer_unittest.cc b/modules/video_coding/video_codec_initializer_unittest.cc index d5a18f0413..686396c51a 100644 --- a/modules/video_coding/video_codec_initializer_unittest.cc +++ b/modules/video_coding/video_codec_initializer_unittest.cc @@ -42,7 +42,8 @@ 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 = 200000; +static const uint32_t kScreenshareTl0BitrateBps = 120000; +static const uint32_t kScreenshareConferenceTl0BitrateBps = 200000; static const uint32_t kScreenshareCodecTargetBitrateBps = 200000; static const uint32_t kScreenshareDefaultFramerate = 5; // Bitrates for the temporal layers of the higher screenshare simulcast stream. @@ -126,7 +127,7 @@ class VideoCodecInitializerTest : public ::testing::Test { VideoStream DefaultScreenshareStream() { VideoStream stream = DefaultStream(); stream.min_bitrate_bps = 30000; - stream.target_bitrate_bps = kScreenshareTl0BitrateBps; + stream.target_bitrate_bps = kScreenshareCodecTargetBitrateBps; stream.max_bitrate_bps = 1000000; stream.max_framerate = kScreenshareDefaultFramerate; stream.num_temporal_layers = 2; @@ -174,6 +175,23 @@ 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 80b79635f1..f38d2a2990 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -780,6 +780,8 @@ 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.