Revert "Only enable conference mode simulcast allocations with flag enabled"
This reverts commit 32ca95145c4636374266f5b5d4d1ac43658bc758. Reason for revert: Internal test failure Original change's description: > Only enable conference mode simulcast allocations with flag enabled > > Non-conference mode simulcast screenshares were mistakenly using the > conference mode semantics in the simulcast rate allocator, which broke > spec compliant usage in some situation. > > This behavior should only be used when explicitly using the SDP entry > "a=x-google-flag:conference" in both offer and answer. > > Bug: webrtc:11310, chromium:1093819 > Change-Id: Ibcba75c88a8405d60467546b33977a782e04e469 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/179081 > Reviewed-by: Harald Alvestrand <hta@webrtc.org> > Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org> > Commit-Queue: Florent Castelli <orphis@webrtc.org> > Cr-Commit-Position: refs/heads/master@{#31828} TBR=ilnik@webrtc.org,hta@webrtc.org,orphis@webrtc.org Change-Id: I5ccb6e87594f491ba09fe6b837ee24d63db878ca No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:11310 Bug: chromium:1093819 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/180801 Reviewed-by: Florent Castelli <orphis@webrtc.org> Commit-Queue: Florent Castelli <orphis@webrtc.org> Cr-Commit-Position: refs/heads/master@{#31829}
This commit is contained in:
committed by
Commit Bot
parent
32ca95145c
commit
834dc9cfa1
@ -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.
|
||||
|
||||
@ -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));
|
||||
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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<bool> stream_enabled_;
|
||||
bool legacy_conference_mode_;
|
||||
|
||||
RTC_DISALLOW_COPY_AND_ASSIGN(SimulcastRateAllocator);
|
||||
};
|
||||
|
||||
@ -88,9 +88,8 @@ class SimulcastRateAllocatorTest : public ::testing::TestWithParam<bool> {
|
||||
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<bool>& 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(
|
||||
|
||||
@ -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,
|
||||
|
||||
@ -274,8 +274,6 @@ VideoCodec VideoCodecInitializer::VideoEncoderConfigToVideoCodec(
|
||||
}
|
||||
}
|
||||
|
||||
video_codec.legacy_conference_mode = config.legacy_conference_mode;
|
||||
|
||||
return video_codec;
|
||||
}
|
||||
|
||||
|
||||
@ -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());
|
||||
|
||||
Reference in New Issue
Block a user