From 4b4dc86c611e8db9b75de40d2b6ddd5f215e95ab Mon Sep 17 00:00:00 2001 From: nisse Date: Wed, 17 Feb 2016 05:25:36 -0800 Subject: [PATCH] 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} --- webrtc/media/base/mediachannel.h | 21 +++++++------------ webrtc/media/base/videoengine_unittest.h | 6 +++--- webrtc/media/engine/webrtcvideoengine2.cc | 18 ++++++++++++++-- webrtc/media/engine/webrtcvideoengine2.h | 2 ++ .../engine/webrtcvideoengine2_unittest.cc | 4 ++-- webrtc/pc/channel.cc | 2 +- 6 files changed, 31 insertions(+), 22 deletions(-) diff --git a/webrtc/media/base/mediachannel.h b/webrtc/media/base/mediachannel.h index 3f6c8dda63..8f15878ecd 100644 --- a/webrtc/media/base/mediachannel.h +++ b/webrtc/media/base/mediachannel.h @@ -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 typing_detection; rtc::Optional aecm_generate_comfort_noise; - rtc::Optional conference_mode; rtc::Optional adjust_agc_delta; rtc::Optional experimental_agc; rtc::Optional 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 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 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 { + // 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 { diff --git a/webrtc/media/base/videoengine_unittest.h b/webrtc/media/base/videoengine_unittest.h index 659e7e0a25..0b190543e1 100644 --- a/webrtc/media/base/videoengine_unittest.h +++ b/webrtc/media/base/videoengine_unittest.h @@ -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(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(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(true); + parameters.conference_mode = true; EXPECT_TRUE(channel_->SetSendParameters(parameters)); EXPECT_TRUE(SetSend(true)); EXPECT_TRUE(channel_->AddRecvStream( diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc index 0dd8938dc3..26d6134a26 100644 --- a/webrtc/media/engine/webrtcvideoengine2.cc +++ b/webrtc/media/engine/webrtcvideoengine2.cc @@ -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(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(¶meters_.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 diff --git a/webrtc/media/engine/webrtcvideoengine2.h b/webrtc/media/engine/webrtcvideoengine2.h index 16b7cc5157..aef908a7dd 100644 --- a/webrtc/media/engine/webrtcvideoengine2.h +++ b/webrtc/media/engine/webrtcvideoengine2.h @@ -192,6 +192,7 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, rtc::Optional codec; rtc::Optional> rtp_header_extensions; rtc::Optional max_bandwidth_bps; + rtc::Optional conference_mode; rtc::Optional options; rtc::Optional 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 codec_settings; // Sent resolutions + bitrates etc. by the underlying VideoSendStream, // typically changes when setting a new resolution or reconfiguring diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc index 65b5c6b1e1..70ddad2b65 100644 --- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc +++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc @@ -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(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(true); + send_parameters_.conference_mode = true; channel_->SetSendParameters(send_parameters_); AddSendStream(); diff --git a/webrtc/pc/channel.cc b/webrtc/pc/channel.cc index 1dc8e7f6bd..6a696ea4b4 100644 --- a/webrtc/pc/channel.cc +++ b/webrtc/pc/channel.cc @@ -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(true); + send_params.conference_mode = true; } if (!media_channel()->SetSendParameters(send_params)) { SafeSetError("Failed to set remote video description send parameters.",