Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(35)

Unified Diff: talk/media/webrtc/webrtcvideoengine2.cc

Issue 1647103002: Rebased changes to apply VideoOptions per stream. (Closed) Base URL: https://chromium.googlesource.com/external/webrtc.git@master
Patch Set: Created 4 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « talk/media/webrtc/webrtcvideoengine2.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: talk/media/webrtc/webrtcvideoengine2.cc
diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc
index 2d5ec5383b3f30ff6ae6e2bb7c9b8ddfd655b102..b266ab8d774af0d65237887e42d219809dbeca74 100644
--- a/talk/media/webrtc/webrtcvideoengine2.cc
+++ b/talk/media/webrtc/webrtcvideoengine2.cc
@@ -55,6 +55,10 @@
namespace cricket {
namespace {
+// Default values, used when options are unset.
+const bool kDefault_suspend_below_min_bitrate = false;
+const int kDefault_screencast_min_bitrate_kbps = 0;
+
// Wrap cricket::WebRtcVideoEncoderFactory as a webrtc::VideoEncoderFactory.
class EncoderFactoryAdapter : public webrtc::VideoEncoderFactory {
public:
@@ -645,13 +649,15 @@ WebRtcVideoChannel2::WebRtcVideoChannel2(
WebRtcVideoDecoderFactory* external_decoder_factory)
: call_(call),
unsignalled_ssrc_handler_(&default_unsignalled_ssrc_handler_),
+ signal_cpu_adaptation_(true),
+ disable_prerenderer_smoothing_(false),
external_encoder_factory_(external_encoder_factory),
external_decoder_factory_(external_decoder_factory) {
RTC_DCHECK(thread_checker_.CalledOnValidThread());
- SetDefaultOptions();
- options_.SetAll(options);
- if (options_.cpu_overuse_detection)
- signal_cpu_adaptation_ = *options_.cpu_overuse_detection;
+
+ SetSharedOptions(options);
+ send_params_.options = options;
+
rtcp_receiver_report_ssrc_ = kDefaultRtcpReceiverReportSsrc;
sending_ = false;
default_send_ssrc_ = 0;
@@ -659,13 +665,6 @@ WebRtcVideoChannel2::WebRtcVideoChannel2(
recv_codecs_ = FilterSupportedCodecs(MapCodecs(recv_codecs));
}
-void WebRtcVideoChannel2::SetDefaultOptions() {
- options_.cpu_overuse_detection = rtc::Optional<bool>(true);
- options_.dscp = rtc::Optional<bool>(false);
- options_.suspend_below_min_bitrate = rtc::Optional<bool>(false);
- options_.screencast_min_bitrate_kbps = rtc::Optional<int>(0);
-}
-
WebRtcVideoChannel2::~WebRtcVideoChannel2() {
for (auto& kv : send_streams_)
delete kv.second;
@@ -778,11 +777,23 @@ bool WebRtcVideoChannel2::GetChangedSendParameters(
// Handle options.
// TODO(pbos): Require VideoSendParameters to contain a full set of options
// and check if params.options != options_ instead of applying a delta.
- VideoOptions new_options = options_;
+
+ // Keep the default options. TODO(nisse): This is a bit convoluted.
+ // It might be cleaner to change SetDefaultOptions to only set
+ // options with no current value. Or delete SetDefaultOptions; I
+ // think the problem is references to suspend_below_min_bitrate and
+ // screencast_min_bitrate which require values to be set, but those
+ // references could be changed to use .value_or(the_default_value)
+ // instead.
+
+ VideoOptions new_options = send_params_.options;
new_options.SetAll(params.options);
- if (!(new_options == options_)) {
+ if (!(new_options == send_params_.options)) {
changed_params->options = rtc::Optional<VideoOptions>(new_options);
}
+#if 0
+ send_params_.options = new_options;
+#endif
// Handle RTCP mode.
if (params.rtcp.reduced_size != send_params_.rtcp.reduced_size) {
@@ -791,12 +802,41 @@ bool WebRtcVideoChannel2::GetChangedSendParameters(
: webrtc::RtcpMode::kCompound);
}
+#if 0
+ SetSharedOptions(params.options);
+ // TODO(nisse): Call each stream's SetSendParameters only for
+ // changed parameters? Used to check params.rtcp.reduced_size, but
+ // that broke the
+ // WebRtcVideoChannel2Test.SetOptionsWithSuspendBelowMinBitrate
+ // test.
+
+ VideoOptions options = send_params_.options;
+ send_params_ = params;
+
+ // Take care to keep old values of options, if the new params
+ // doesn't specify any value.
+ options.SetAll(params.options);
+ send_params_.options = options;
+
+ SetSharedOptions(send_params_.options);
+ // Call each stream's SetSendParameters method. Leaves to the callee
+ // to check if there's any change.
+
+ {
+ rtc::CritScope stream_lock(&stream_crit_);
+ for (auto& kv : send_streams_) {
+ kv.second->SetSendParameters(params);
+ }
+ }
+#endif
+
return true;
}
bool WebRtcVideoChannel2::SetSendParameters(const VideoSendParameters& params) {
TRACE_EVENT0("webrtc", "WebRtcVideoChannel2::SetSendParameters");
LOG(LS_INFO) << "SetSendParameters: " << params.ToString();
+ // TODO(nisse): Should GetChangedSendParameters call SetSharedOptions?
ChangedSendParameters changed_params;
if (!GetChangedSendParameters(params, &changed_params)) {
return false;
@@ -838,6 +878,7 @@ bool WebRtcVideoChannel2::SetSendParameters(const VideoSendParameters& params) {
call_->SetBitrateConfig(bitrate_config_);
}
+#if 0
if (changed_params.options) {
options_.SetAll(*changed_params.options);
{
@@ -850,7 +891,7 @@ bool WebRtcVideoChannel2::SetSendParameters(const VideoSendParameters& params) {
options_.dscp.value_or(false) ? rtc::DSCP_AF41 : rtc::DSCP_DEFAULT;
MediaChannel::SetDscp(dscp);
}
-
+#endif
{
rtc::CritScope stream_lock(&stream_crit_);
for (auto& kv : send_streams_) {
@@ -1009,9 +1050,14 @@ bool WebRtcVideoChannel2::SetVideoSend(uint32_t ssrc, bool enable,
return false;
}
if (enable && options) {
+ // TODO(nisse): Rebase problem.
+#if 0
VideoSendParameters new_params = send_params_;
new_params.options.SetAll(*options);
SetSendParameters(send_params_);
+#else
+ return SetOptions(ssrc, *options);
+#endif
}
return true;
}
@@ -1056,7 +1102,7 @@ bool WebRtcVideoChannel2::AddSendStream(const StreamParams& sp) {
config.overuse_callback = this;
WebRtcVideoSendStream* stream = new WebRtcVideoSendStream(
- call_, sp, config, external_encoder_factory_, options_,
+ call_, sp, config, external_encoder_factory_,
bitrate_config_.max_bitrate_bps, send_codec_, send_rtp_extensions_,
send_params_);
@@ -1186,7 +1232,7 @@ bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp,
receive_streams_[ssrc] = new WebRtcVideoReceiveStream(
call_, sp, config, external_decoder_factory_, default_stream,
- recv_codecs_, options_.disable_prerenderer_smoothing.value_or(false));
+ recv_codecs_, disable_prerenderer_smoothing_);
return true;
}
@@ -1452,11 +1498,44 @@ bool WebRtcVideoChannel2::MuteStream(uint32_t ssrc, bool mute) {
}
// TODO(pbos): Remove SetOptions in favor of SetSendParameters.
+#if 0
+// TODO(nisse): Rebase leftover.
void WebRtcVideoChannel2::SetOptions(const VideoOptions& options) {
VideoSendParameters new_params = send_params_;
new_params.options.SetAll(options);
SetSendParameters(send_params_);
}
+#endif
+void WebRtcVideoChannel2::SetSharedOptions(const VideoOptions& options) {
+ {
+ rtc::CritScope lock(&capturer_crit_);
+ if (options.cpu_overuse_detection)
+ signal_cpu_adaptation_ = *options.cpu_overuse_detection;
+ }
+ if (options.disable_prerenderer_smoothing)
+ disable_prerenderer_smoothing_ = *options.disable_prerenderer_smoothing;
+
+ if (options.dscp) {
+ rtc::DiffServCodePoint dscp =
+ *options.dscp ? rtc::DSCP_AF41 : rtc::DSCP_DEFAULT;
+ MediaChannel::SetDscp(dscp);
+ }
+}
+
+bool WebRtcVideoChannel2::SetOptions(uint32_t ssrc,
+ const VideoOptions& options) {
+ TRACE_EVENT0("webrtc", "WebRtcVideoChannel2::SetOptions");
+ LOG(LS_INFO) << "SetOptions: ssrc " << ssrc << ": " << options.ToString();
+ SetSharedOptions(options);
+
+ rtc::CritScope stream_lock(&stream_crit_);
+ if (send_streams_.find(ssrc) == send_streams_.end()) {
+ return false;
+ }
+ send_streams_[ssrc]->SetOptions(options);
+
+ return true;
+}
void WebRtcVideoChannel2::SetInterface(NetworkInterface* iface) {
MediaChannel::SetInterface(iface);
@@ -1565,7 +1644,6 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::WebRtcVideoSendStream(
const StreamParams& sp,
const webrtc::VideoSendStream::Config& config,
WebRtcVideoEncoderFactory* external_encoder_factory,
- const VideoOptions& options,
int max_bitrate_bps,
const rtc::Optional<VideoCodecSettings>& codec_settings,
const std::vector<webrtc::RtpExtension>& rtp_extensions,
@@ -1577,7 +1655,7 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::WebRtcVideoSendStream(
call_(call),
external_encoder_factory_(external_encoder_factory),
stream_(NULL),
- parameters_(config, options, max_bitrate_bps, codec_settings),
+ parameters_(config, send_params.options, max_bitrate_bps, codec_settings),
pending_encoder_reconfiguration_(false),
allocated_encoder_(NULL, webrtc::kVideoCodecUnknown, false),
capturer_(NULL),
@@ -1868,9 +1946,9 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetCodecAndOptions(
parameters_.config.rtp.nack.rtp_history_ms =
HasNack(codec_settings.codec) ? kNackHistoryMs : 0;
- RTC_CHECK(options.suspend_below_min_bitrate);
parameters_.config.suspend_below_min_bitrate =
- *options.suspend_below_min_bitrate;
+ options.suspend_below_min_bitrate.value_or(
+ kDefault_suspend_below_min_bitrate);
parameters_.codec_settings =
rtc::Optional<WebRtcVideoChannel2::VideoCodecSettings>(codec_settings);
@@ -1936,9 +2014,9 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::CreateVideoEncoderConfig(
const VideoCodec& codec) const {
webrtc::VideoEncoderConfig encoder_config;
if (dimensions.is_screencast) {
- RTC_CHECK(parameters_.options.screencast_min_bitrate_kbps);
encoder_config.min_transmit_bitrate_bps =
- *parameters_.options.screencast_min_bitrate_kbps * 1000;
+ 1000 * parameters_.options.screencast_min_bitrate_kbps.value_or(
+ kDefault_screencast_min_bitrate_kbps);
encoder_config.content_type =
webrtc::VideoEncoderConfig::ContentType::kScreen;
} else {
« no previous file with comments | « talk/media/webrtc/webrtcvideoengine2.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698