Remove conference_mode flag from AudioOptions and VideoOptions.

For audio, the flag is apparently unused. For video, the flag is moved to
VideoSendParameters, with the intention to keep only per-stream flags in
VideoOptions. The flag is used for the webrtcvideoengine2 logic commented like

  // Conference mode screencast uses 2 temporal layers split at 100kbit.

  // For screenshare in conference mode, tl0 and tl1 bitrates are piggybacked
  // on the VideoCodec struct as target and max bitrates, respectively.
  // See eg. webrtc::VP8EncoderImpl::SetRates().

BUG=webrtc:5426

Review URL: https://codereview.webrtc.org/1697163002

Cr-Commit-Position: refs/heads/master@{#11651}
This commit is contained in:
nisse 2016-02-17 05:25:36 -08:00 committed by Commit bot
parent 22785c7099
commit 4b4dc86c61
6 changed files with 31 additions and 22 deletions

View File

@ -129,7 +129,6 @@ struct AudioOptions {
change.audio_jitter_buffer_fast_accelerate);
SetFrom(&typing_detection, change.typing_detection);
SetFrom(&aecm_generate_comfort_noise, change.aecm_generate_comfort_noise);
SetFrom(&conference_mode, change.conference_mode);
SetFrom(&adjust_agc_delta, change.adjust_agc_delta);
SetFrom(&experimental_agc, change.experimental_agc);
SetFrom(&extended_filter_aec, change.extended_filter_aec);
@ -156,7 +155,6 @@ struct AudioOptions {
o.audio_jitter_buffer_fast_accelerate &&
typing_detection == o.typing_detection &&
aecm_generate_comfort_noise == o.aecm_generate_comfort_noise &&
conference_mode == o.conference_mode &&
experimental_agc == o.experimental_agc &&
extended_filter_aec == o.extended_filter_aec &&
delay_agnostic_aec == o.delay_agnostic_aec &&
@ -185,7 +183,6 @@ struct AudioOptions {
audio_jitter_buffer_fast_accelerate);
ost << ToStringIfSet("typing", typing_detection);
ost << ToStringIfSet("comfort_noise", aecm_generate_comfort_noise);
ost << ToStringIfSet("conference", conference_mode);
ost << ToStringIfSet("agc_delta", adjust_agc_delta);
ost << ToStringIfSet("experimental_agc", experimental_agc);
ost << ToStringIfSet("extended_filter_aec", extended_filter_aec);
@ -221,7 +218,6 @@ struct AudioOptions {
// Audio processing to detect typing.
rtc::Optional<bool> typing_detection;
rtc::Optional<bool> aecm_generate_comfort_noise;
rtc::Optional<bool> conference_mode;
rtc::Optional<int> adjust_agc_delta;
rtc::Optional<bool> experimental_agc;
rtc::Optional<bool> extended_filter_aec;
@ -256,14 +252,12 @@ struct AudioOptions {
struct VideoOptions {
void SetAll(const VideoOptions& change) {
SetFrom(&video_noise_reduction, change.video_noise_reduction);
SetFrom(&conference_mode, change.conference_mode);
SetFrom(&suspend_below_min_bitrate, change.suspend_below_min_bitrate);
SetFrom(&screencast_min_bitrate_kbps, change.screencast_min_bitrate_kbps);
}
bool operator==(const VideoOptions& o) const {
return video_noise_reduction == o.video_noise_reduction &&
conference_mode == o.conference_mode &&
suspend_below_min_bitrate == o.suspend_below_min_bitrate &&
screencast_min_bitrate_kbps == o.screencast_min_bitrate_kbps;
}
@ -272,7 +266,6 @@ struct VideoOptions {
std::ostringstream ost;
ost << "VideoOptions {";
ost << ToStringIfSet("noise reduction", video_noise_reduction);
ost << ToStringIfSet("conference mode", conference_mode);
ost << ToStringIfSet("suspend below min bitrate",
suspend_below_min_bitrate);
ost << ToStringIfSet("screencast min bitrate kbps",
@ -285,13 +278,6 @@ struct VideoOptions {
// constraint 'googNoiseReduction', and WebRtcVideoEngine2 passes it
// on to the codec options. Disabled by default.
rtc::Optional<bool> video_noise_reduction;
// Use conference mode? This flag comes from the remote
// description's SDP line 'a=x-google-flag:conference', copied over
// by VideoChannel::SetRemoteContent_w, and ultimately used by
// conference mode screencast logic in
// WebRtcVideoChannel2::WebRtcVideoSendStream::CreateVideoEncoderConfig.
// The special screencast behaviour is disabled by default.
rtc::Optional<bool> conference_mode;
// Enable WebRTC suspension of video. No video frames will be sent
// when the bitrate is below the configured minimum bitrate. This
// flag comes from the PeerConnection constraint
@ -946,6 +932,13 @@ class VoiceMediaChannel : public MediaChannel {
};
struct VideoSendParameters : RtpSendParameters<VideoCodec, VideoOptions> {
// Use conference mode? This flag comes from the remote
// description's SDP line 'a=x-google-flag:conference', copied over
// by VideoChannel::SetRemoteContent_w, and ultimately used by
// conference mode screencast logic in
// WebRtcVideoChannel2::WebRtcVideoSendStream::CreateVideoEncoderConfig.
// The special screencast behaviour is disabled by default.
bool conference_mode = false;
};
struct VideoRecvParameters : RtpParameters<VideoCodec> {

View File

@ -494,7 +494,7 @@ class VideoMediaChannelTest : public testing::Test,
EXPECT_TRUE(SetOneCodec(DefaultCodec()));
cricket::VideoSendParameters parameters;
parameters.codecs.push_back(DefaultCodec());
parameters.options.conference_mode = rtc::Optional<bool>(true);
parameters.conference_mode = true;
EXPECT_TRUE(channel_->SetSendParameters(parameters));
EXPECT_TRUE(SetSend(true));
EXPECT_TRUE(channel_->AddRecvStream(
@ -545,7 +545,7 @@ class VideoMediaChannelTest : public testing::Test,
EXPECT_TRUE(SetOneCodec(DefaultCodec()));
cricket::VideoSendParameters parameters;
parameters.codecs.push_back(DefaultCodec());
parameters.options.conference_mode = rtc::Optional<bool>(true);
parameters.conference_mode = true;
EXPECT_TRUE(channel_->SetSendParameters(parameters));
EXPECT_TRUE(channel_->AddRecvStream(
cricket::StreamParams::CreateLegacy(kSsrc)));
@ -725,7 +725,7 @@ class VideoMediaChannelTest : public testing::Test,
EXPECT_TRUE(SetDefaultCodec());
cricket::VideoSendParameters parameters;
parameters.codecs.push_back(DefaultCodec());
parameters.options.conference_mode = rtc::Optional<bool>(true);
parameters.conference_mode = true;
EXPECT_TRUE(channel_->SetSendParameters(parameters));
EXPECT_TRUE(SetSend(true));
EXPECT_TRUE(channel_->AddRecvStream(

View File

@ -733,6 +733,12 @@ bool WebRtcVideoChannel2::GetChangedSendParameters(
params.max_bandwidth_bps == 0 ? -1 : params.max_bandwidth_bps);
}
// Handle conference mode.
if (params.conference_mode != send_params_.conference_mode) {
changed_params->conference_mode =
rtc::Optional<bool>(params.conference_mode);
}
// Handle options.
// TODO(pbos): Require VideoSendParameters to contain a full set of options
// and check if params.options != options_ instead of applying a delta.
@ -1503,6 +1509,7 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::WebRtcVideoSendStream(
first_frame_timestamp_ms_(0),
last_frame_timestamp_ms_(0) {
parameters_.config.rtp.max_packet_size = kVideoMtu;
parameters_.conference_mode = send_params.conference_mode;
sp.GetPrimarySsrcs(&parameters_.config.rtp.ssrcs);
sp.GetFidSsrcs(parameters_.config.rtp.ssrcs,
@ -1792,6 +1799,9 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetSendParameters(
parameters_.max_bitrate_bps = *params.max_bandwidth_bps;
pending_encoder_reconfiguration_ = true;
}
if (params.conference_mode) {
parameters_.conference_mode = *params.conference_mode;
}
// Set codecs and options.
if (params.codec) {
SetCodecAndOptions(*params.codec,
@ -1806,6 +1816,10 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetSendParameters(
parameters_.options = *params.options;
}
}
else if (params.conference_mode && parameters_.codec_settings) {
SetCodecAndOptions(*parameters_.codec_settings, parameters_.options);
return;
}
if (recreate_stream) {
LOG(LS_INFO) << "RecreateWebRtcStream (send) because of SetSendParameters";
RecreateWebRtcStream();
@ -1855,8 +1869,8 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::CreateVideoEncoderConfig(
parameters_.max_bitrate_bps, stream_count);
// Conference mode screencast uses 2 temporal layers split at 100kbit.
if (parameters_.options.conference_mode.value_or(false) &&
dimensions.is_screencast && encoder_config.streams.size() == 1) {
if (parameters_.conference_mode && dimensions.is_screencast &&
encoder_config.streams.size() == 1) {
ScreenshareLayerConfig config = ScreenshareLayerConfig::GetDefault();
// For screenshare in conference mode, tl0 and tl1 bitrates are piggybacked

View File

@ -192,6 +192,7 @@ class WebRtcVideoChannel2 : public VideoMediaChannel,
rtc::Optional<VideoCodecSettings> codec;
rtc::Optional<std::vector<webrtc::RtpExtension>> rtp_header_extensions;
rtc::Optional<int> max_bandwidth_bps;
rtc::Optional<bool> conference_mode;
rtc::Optional<VideoOptions> options;
rtc::Optional<webrtc::RtcpMode> rtcp_mode;
};
@ -273,6 +274,7 @@ class WebRtcVideoChannel2 : public VideoMediaChannel,
webrtc::VideoSendStream::Config config;
VideoOptions options;
int max_bitrate_bps;
bool conference_mode;
rtc::Optional<VideoCodecSettings> codec_settings;
// Sent resolutions + bitrates etc. by the underlying VideoSendStream,
// typically changes when setting a new resolution or reconfiguring

View File

@ -1069,7 +1069,7 @@ TEST_F(WebRtcVideoChannel2Test, RecvStreamWithSimAndRtx) {
parameters.codecs = engine_.codecs();
EXPECT_TRUE(channel_->SetSendParameters(parameters));
EXPECT_TRUE(channel_->SetSend(true));
parameters.options.conference_mode = rtc::Optional<bool>(true);
parameters.conference_mode = true;
EXPECT_TRUE(channel_->SetSendParameters(parameters));
// Send side.
@ -1513,7 +1513,7 @@ TEST_F(WebRtcVideoChannel2Test,
ConferenceModeScreencastConfiguresTemporalLayer) {
static const int kConferenceScreencastTemporalBitrateBps =
ScreenshareLayerConfig::GetDefault().tl0_bitrate_kbps * 1000;
send_parameters_.options.conference_mode = rtc::Optional<bool>(true);
send_parameters_.conference_mode = true;
channel_->SetSendParameters(send_parameters_);
AddSendStream();

View File

@ -1798,7 +1798,7 @@ bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content,
VideoSendParameters send_params = last_send_params_;
RtpSendParametersFromMediaDescription(video, &send_params);
if (video->conference_mode()) {
send_params.options.conference_mode = rtc::Optional<bool>(true);
send_params.conference_mode = true;
}
if (!media_channel()->SetSendParameters(send_params)) {
SafeSetError("Failed to set remote video description send parameters.",