zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Only enable conference mode simulcast allocations with flag enabled

Browse Source 
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}
This commit is contained in:
Florent Castelli
2020-07-24 17:29:40 +02:00
committed by Commit Bot
parent c878d68282
commit 32ca95145c
18 changed files with 71 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
api/video/video_bitrate_allocator.cc
View File

@ -49,4 +49,6 @@ VideoBitrateAllocation VideoBitrateAllocator::Allocate(
return GetAllocation(parameters.total_bitrate.bps(), parameters.framerate);
}
void VideoBitrateAllocator::SetLegacyConferenceMode(bool enabled) {}
} // namespace webrtc

4
api/video/video_bitrate_allocator.h
View File

@ -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 {

1
api/video_codecs/video_codec.cc
View File

@ -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() {

3
api/video_codecs/video_codec.h
View File

@ -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;

3
api/video_codecs/video_encoder_config.cc
View File

@ -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;

3
api/video_codecs/video_encoder_config.h
View File

@ -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.

4
media/engine/simulcast_encoder_adapter.cc
View File

@ -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 {

6
media/engine/webrtc_video_engine.cc
View File

@ -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<webrtc::VideoStream> 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<webrtc::VideoStream>
EncoderStreamFactory::CreateSimulcastOrConfereceModeScreenshareStreams(
EncoderStreamFactory::CreateSimulcastOrConferenceModeScreenshareStreams(
int width,
int height,
const webrtc::VideoEncoderConfig& encoder_config,

2
media/engine/webrtc_video_engine.h
View File

@ -661,7 +661,7 @@ class EncoderStreamFactory
const absl::optional<webrtc::DataRate>& experimental_min_bitrate) const;
std::vector<webrtc::VideoStream>
CreateSimulcastOrConfereceModeScreenshareStreams(
CreateSimulcastOrConferenceModeScreenshareStreams(
int width,
int height,
const webrtc::VideoEncoderConfig& encoder_config,

2
modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc
View File

@ -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.

2
modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc
View File

@ -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));

16
modules/video_coding/utility/simulcast_rate_allocator.cc
View File

@ -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

3
modules/video_coding/utility/simulcast_rate_allocator.h
View File

@ -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<bool> stream_enabled_;
bool legacy_conference_mode_;
RTC_DISALLOW_COPY_AND_ASSIGN(SimulcastRateAllocator);
};

15
modules/video_coding/utility/simulcast_rate_allocator_unittest.cc
View File

@ -88,8 +88,9 @@ class SimulcastRateAllocatorTest : public ::testing::TestWithParam<bool> {
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<bool>& 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(

11
modules/video_coding/utility/simulcast_utility.cc
View File

@ -84,16 +84,7 @@ 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.legacy_conference_mode;
}
int SimulcastUtility::NumberOfTemporalLayers(const VideoCodec& codec,

2
modules/video_coding/video_codec_initializer.cc
View File

@ -274,6 +274,8 @@ VideoCodec VideoCodecInitializer::VideoEncoderConfigToVideoCodec(
}
}
video_codec.legacy_conference_mode = config.legacy_conference_mode;
return video_codec;
}

22
modules/video_coding/video_codec_initializer_unittest.cc
View File

@ -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());

2
video/video_stream_encoder.cc
View File

@ -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.