Refactoring: Merge VideoSenderConfig and AudioSenderConfig.

chrome/renderer/extensions/cast_streaming_native_handler.cc

Index: chrome/renderer/extensions/cast_streaming_native_handler.cc
diff --git a/chrome/renderer/extensions/cast_streaming_native_handler.cc b/chrome/renderer/extensions/cast_streaming_native_handler.cc
index c03f63fc5607ae012e854405569efef0f9a5d0d0..7f9ceb82e86e5dc57be2f7708b19c2df4cc92eeb 100644
--- a/chrome/renderer/extensions/cast_streaming_native_handler.cc
+++ b/chrome/renderer/extensions/cast_streaming_native_handler.cc
@@ -21,6 +21,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/sys_info.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/common/extensions/api/cast_streaming_receiver_session.h"
 #include "chrome/common/extensions/api/cast_streaming_rtp_stream.h"
@@ -33,6 +34,7 @@
 #include "content/public/renderer/media_stream_utils.h"
 #include "extensions/renderer/script_context.h"
 #include "media/base/audio_parameters.h"
+#include "media/base/limits.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/ip_address.h"
 #include "third_party/WebKit/public/platform/WebMediaStream.h"
@@ -43,10 +45,10 @@
 #include "url/gurl.h"
 
 using content::V8ValueConverter;
+using media::cast::FrameSenderConfig;
 
 // Extension types.
 using extensions::api::cast_streaming_receiver_session::RtpReceiverParams;
-using extensions::api::cast_streaming_rtp_stream::CodecSpecificParams;
 using extensions::api::cast_streaming_rtp_stream::RtpParams;
 using extensions::api::cast_streaming_rtp_stream::RtpPayloadParams;
 using extensions::api::cast_streaming_udp_transport::IPEndPoint;
@@ -54,6 +56,7 @@ using extensions::api::cast_streaming_udp_transport::IPEndPoint;
 namespace extensions {
 
 namespace {
+
 const char kInvalidAesIvMask[] = "Invalid value for AES IV mask";
 const char kInvalidAesKey[] = "Invalid value for AES key";
 const char kInvalidAudioParams[] = "Invalid audio params";
@@ -68,22 +71,13 @@ const char kRtpStreamNotFound[] = "The RTP stream cannot be found";
 const char kUdpTransportNotFound[] = "The UDP transport cannot be found";
 const char kUnableToConvertArgs[] = "Unable to convert arguments";
 const char kUnableToConvertParams[] = "Unable to convert params";
+const char kCodecNameOpus[] = "OPUS";
+const char kCodecNameVp8[] = "VP8";
+const char kCodecNameH264[] = "H264";
 
-// These helper methods are used to convert between Extension API
-// types and Cast types.
-void ToCastCodecSpecificParams(const CodecSpecificParams& ext_params,
-                               CastCodecSpecificParams* cast_params) {
-  cast_params->key = ext_params.key;
-  cast_params->value = ext_params.value;
-}
-
-void FromCastCodecSpecificParams(const CastCodecSpecificParams& cast_params,
-                                 CodecSpecificParams* ext_params) {
-  ext_params->key = cast_params.key;
-  ext_params->value = cast_params.value;
-}
+// To convert from kilobits per second to bits to per second.
+const int kBitrateMultiplier = 1000;

[miu, 2016/06/30 21:59:42]

nit: s/const/constexpr/

[xjz, 2016/07/01 23:52:08]

Done.

 
-namespace {
 bool HexDecode(const std::string& input, std::string* output) {
   std::vector<uint8_t> bytes;
   if (!base::HexStringToBytes(input, &bytes))
@@ -91,97 +85,168 @@ bool HexDecode(const std::string& input, std::string* output) {
   output->assign(reinterpret_cast<const char*>(&bytes[0]), bytes.size());
   return true;
 }
-}  // namespace
 
-bool ToCastRtpPayloadParamsOrThrow(v8::Isolate* isolate,
-                                   const RtpPayloadParams& ext_params,
-                                   CastRtpPayloadParams* cast_params) {
-  cast_params->payload_type = ext_params.payload_type;
-  cast_params->max_latency_ms = ext_params.max_latency;
-  cast_params->min_latency_ms =
-      ext_params.min_latency ? *ext_params.min_latency : ext_params.max_latency;
-  cast_params->animated_latency_ms = ext_params.animated_latency
-                                         ? *ext_params.animated_latency
-                                         : ext_params.max_latency;
-  cast_params->codec_name = ext_params.codec_name;
-  cast_params->ssrc = ext_params.ssrc;
-  cast_params->feedback_ssrc = ext_params.feedback_ssrc;
-  cast_params->clock_rate = ext_params.clock_rate ? *ext_params.clock_rate : 0;
-  cast_params->min_bitrate =
-      ext_params.min_bitrate ? *ext_params.min_bitrate : 0;
-  cast_params->max_bitrate =
-      ext_params.max_bitrate ? *ext_params.max_bitrate : 0;
-  cast_params->channels = ext_params.channels ? *ext_params.channels : 0;
-  cast_params->max_frame_rate =
-      ext_params.max_frame_rate ? *ext_params.max_frame_rate : 0.0;
-  if (ext_params.aes_key &&
-      !HexDecode(*ext_params.aes_key, &cast_params->aes_key)) {
+int NumberOfEncodeThreads() {
+  // Do not saturate CPU utilization just for encoding. On a lower-end system
+  // with only 1 or 2 cores, use only one thread for encoding. On systems with
+  // more cores, allow half of the cores to be used for encoding.
+  return std::min(8, (base::SysInfo::NumberOfProcessors() + 1) / 2);
+}
+
+bool ToFrameSenderConfigOrThrow(v8::Isolate* isolate,
+                                const RtpPayloadParams& ext_params,
+                                FrameSenderConfig* config) {
+  config->sender_ssrc = ext_params.ssrc;
+  config->receiver_ssrc = ext_params.feedback_ssrc;
+  if (config->sender_ssrc == config->receiver_ssrc) {
+    DVLOG(1) << "sender_ssrc " << config->sender_ssrc
+             << " cannot be equal to receiver_ssrc";
+    isolate->ThrowException(v8::Exception::Error(
+        v8::String::NewFromUtf8(isolate, kInvalidRtpParams)));
+    return false;
+  }
+  config->min_playout_delay = base::TimeDelta::FromMilliseconds(
+      ext_params.min_latency ? *ext_params.min_latency
+                             : ext_params.max_latency);
+  config->max_playout_delay =
+      base::TimeDelta::FromMilliseconds(ext_params.max_latency);
+  config->animated_playout_delay = base::TimeDelta::FromMilliseconds(
+      ext_params.animated_latency ? *ext_params.animated_latency
+                                  : ext_params.max_latency);
+  if (config->min_playout_delay <= base::TimeDelta()) {
+    DVLOG(1) << "min_playout_delay " << config->min_playout_delay

[miu, 2016/06/30 21:59:42]

nit: Let's say "must be greater than zero" rather than "is too small."

[xjz, 2016/07/01 23:52:09]

Done.

+             << " is too small";
+    isolate->ThrowException(v8::Exception::Error(
+        v8::String::NewFromUtf8(isolate, kInvalidRtpParams)));
+    return false;
+  }
+  if (config->min_playout_delay > config->max_playout_delay) {
+    DVLOG(1) << "min_playout_delay " << config->min_playout_delay

[miu, 2016/06/30 21:59:42]

nit: Let's say "must be less than max playout delay" rather than "is too big."

[xjz, 2016/07/01 23:52:09]

Done.

+             << " is too big";
+    isolate->ThrowException(v8::Exception::Error(
+        v8::String::NewFromUtf8(isolate, kInvalidRtpParams)));
+    return false;
+  }
+  if (config->animated_playout_delay < config->min_playout_delay ||
+      config->animated_playout_delay > config->max_playout_delay) {
+    DVLOG(1) << "animated_playout_delay " << config->animated_playout_delay
+             << " is out of range";

[miu, 2016/06/30 21:59:42]

nit: Let's say "must be between (inclusive) the min and max playout delay"
rather than "is out of range."

[xjz, 2016/07/01 23:52:09]

Done.

+    isolate->ThrowException(v8::Exception::Error(
+        v8::String::NewFromUtf8(isolate, kInvalidRtpParams)));
+    return false;
+  }
+  config->min_bitrate = ext_params.min_bitrate
+                            ? (*ext_params.min_bitrate) * kBitrateMultiplier
+                            : 0;
+  config->max_bitrate = ext_params.max_bitrate
+                            ? (*ext_params.max_bitrate) * kBitrateMultiplier
+                            : 0;

[miu, 2016/06/30 21:59:42]

Instead of 0, can the default be kDefaultMaxVideoKbps (or
kDefaultAudioEncoderBitrate for audio)?

[xjz, 2016/07/01 23:52:08]

Done.

+  config->channels = ext_params.channels ? *ext_params.channels : 0;

[miu, 2016/06/30 21:59:42]

Let's default this to 1 for video and 2 for audio. Also, can we validate the
number (i.e., it must be 1 or 2 for audio, and 1 for video)?

[xjz, 2016/07/01 23:52:09]

Done.

+  config->frequency = ext_params.clock_rate ? *ext_params.clock_rate : 0;

[miu, 2016/06/30 21:59:42]

Let's default this to kDefaultAudioSamplingRate.

[xjz, 2016/07/01 23:52:08]

Done.

+  if (ext_params.codec_name == kCodecNameOpus) {
+    config->rtp_payload_type = media::cast::RtpPayloadType::AUDIO_OPUS;
+    config->codec = media::cast::CODEC_AUDIO_OPUS;
+    config->use_external_encoder = false;
+    config->min_bitrate = config->max_bitrate;

[miu, 2016/06/30 21:59:42]

also: config->start_bitrate = config->max_bitrate  (see comment in cast_config.h
about start_bitrate)

[xjz, 2016/07/01 23:52:08]

Done.

+    // Sampling rate must be one of the Opus-supported values.
+    switch (config->frequency) {
+      case 48000:
+      case 24000:
+      case 16000:
+      case 12000:
+      case 8000:
+        break;
+      default:
+        DVLOG(1) << "frequency " << config->frequency << " is invalid";
+        isolate->ThrowException(v8::Exception::Error(
+            v8::String::NewFromUtf8(isolate, kInvalidRtpParams)));
+        return false;
+    }
+    if (config->channels < 1) {
+      isolate->ThrowException(v8::Exception::Error(
+          v8::String::NewFromUtf8(isolate, kInvalidRtpParams)));
+      DVLOG(1) << "channels " << config->channels << " is invalid";
+      return false;
+    }

[miu, 2016/06/30 21:59:42]

Also for Opus: Let's set config->max_frame_rate to 100 (10ms frames) for
completeness.

[xjz, 2016/07/01 23:52:08]

Done.

+  } else if (ext_params.codec_name == kCodecNameVp8 ||
+             ext_params.codec_name == kCodecNameH264) {
+    config->frequency = media::cast::kVideoFrequency;
+    if (config->min_bitrate > config->max_bitrate) {
+      DVLOG(1) << "min_bitrate " << config->min_bitrate << " is larger than "
+               << "max_bitrate " << config->max_bitrate;
+      isolate->ThrowException(v8::Exception::Error(
+          v8::String::NewFromUtf8(isolate, kInvalidRtpParams)));
+      return false;
+    }
+    config->codec_specific_params.start_bitrate = config->min_bitrate;
+    config->max_frame_rate = std::max(
+        1.0, ext_params.max_frame_rate ? *ext_params.max_frame_rate : 0.0);
+    if (config->max_frame_rate > media::limits::kMaxFramesPerSecond) {
+      DVLOG(1) << "max_frame_rate " << config->max_frame_rate << " is invalid";
+      isolate->ThrowException(v8::Exception::Error(
+          v8::String::NewFromUtf8(isolate, kInvalidRtpParams)));
+      return false;
+    }
+    if (ext_params.codec_name == kCodecNameVp8) {
+      config->rtp_payload_type = media::cast::RtpPayloadType::VIDEO_VP8;
+      config->codec = media::cast::CODEC_VIDEO_VP8;
+      config->use_external_encoder =
+          CastRtpStream::IsHardwareVP8EncodingSupported();
+    } else {
+      config->rtp_payload_type = media::cast::RtpPayloadType::VIDEO_H264;
+      config->codec = media::cast::CODEC_VIDEO_H264;
+      config->use_external_encoder =
+          CastRtpStream::IsHardwareH264EncodingSupported();
+    }
+    if (!config->use_external_encoder)
+      config->codec_specific_params.number_of_encode_threads =
+          NumberOfEncodeThreads();
+  } else {
+    DVLOG(1) << "codec_name " << ext_params.codec_name << " is invalid";
+    isolate->ThrowException(v8::Exception::Error(
+        v8::String::NewFromUtf8(isolate, kInvalidRtpParams)));
+    return false;
+  }
+  if (ext_params.aes_key && !HexDecode(*ext_params.aes_key, &config->aes_key)) {
     isolate->ThrowException(v8::Exception::Error(
         v8::String::NewFromUtf8(isolate, kInvalidAesKey)));
     return false;
   }
   if (ext_params.aes_iv_mask &&
-      !HexDecode(*ext_params.aes_iv_mask, &cast_params->aes_iv_mask)) {
+      !HexDecode(*ext_params.aes_iv_mask, &config->aes_iv_mask)) {
     isolate->ThrowException(v8::Exception::Error(
         v8::String::NewFromUtf8(isolate, kInvalidAesIvMask)));
     return false;
   }
-  for (size_t i = 0; i < ext_params.codec_specific_params.size(); ++i) {
-    CastCodecSpecificParams cast_codec_params;
-    ToCastCodecSpecificParams(ext_params.codec_specific_params[i],
-                              &cast_codec_params);
-    cast_params->codec_specific_params.push_back(cast_codec_params);
-  }
   return true;
 }
 
-void FromCastRtpPayloadParams(const CastRtpPayloadParams& cast_params,
-                              RtpPayloadParams* ext_params) {
-  ext_params->payload_type = cast_params.payload_type;
-  ext_params->max_latency = cast_params.max_latency_ms;
-  ext_params->min_latency.reset(new int(cast_params.min_latency_ms));
-  ext_params->animated_latency.reset(new int(cast_params.animated_latency_ms));
-  ext_params->codec_name = cast_params.codec_name;
-  ext_params->ssrc = cast_params.ssrc;
-  ext_params->feedback_ssrc = cast_params.feedback_ssrc;
-  if (cast_params.clock_rate)
-    ext_params->clock_rate.reset(new int(cast_params.clock_rate));
-  if (cast_params.min_bitrate)
-    ext_params->min_bitrate.reset(new int(cast_params.min_bitrate));
-  if (cast_params.max_bitrate)
-    ext_params->max_bitrate.reset(new int(cast_params.max_bitrate));
-  if (cast_params.channels)
-    ext_params->channels.reset(new int(cast_params.channels));
-  if (cast_params.max_frame_rate > 0.0)
-    ext_params->max_frame_rate.reset(new double(cast_params.max_frame_rate));
-  for (size_t i = 0; i < cast_params.codec_specific_params.size(); ++i) {
-    CodecSpecificParams ext_codec_params;
-    FromCastCodecSpecificParams(cast_params.codec_specific_params[i],
-                                &ext_codec_params);
-    ext_params->codec_specific_params.push_back(std::move(ext_codec_params));
-  }
-}
-
-void FromCastRtpParams(const CastRtpParams& cast_params,
-                       RtpParams* ext_params) {
-  std::copy(cast_params.rtcp_features.begin(),
-            cast_params.rtcp_features.end(),
-            std::back_inserter(ext_params->rtcp_features));
-  FromCastRtpPayloadParams(cast_params.payload, &ext_params->payload);
-}
-
-bool ToCastRtpParamsOrThrow(v8::Isolate* isolate,
-                            const RtpParams& ext_params,
-                            CastRtpParams* cast_params) {
-  std::copy(ext_params.rtcp_features.begin(),
-            ext_params.rtcp_features.end(),
-            std::back_inserter(cast_params->rtcp_features));
-  if (!ToCastRtpPayloadParamsOrThrow(isolate,
-                                     ext_params.payload,
-                                     &cast_params->payload)) {
-    return false;
-  }
-  return true;
+void FromFrameSenderConfig(const FrameSenderConfig& config,
+                           RtpPayloadParams* ext_params) {
+  ext_params->payload_type = static_cast<int>(config.rtp_payload_type);
+  ext_params->max_latency = config.max_playout_delay.InMilliseconds();
+  ext_params->min_latency.reset(
+      new int(config.min_playout_delay.InMilliseconds()));
+  ext_params->animated_latency.reset(
+      new int(config.animated_playout_delay.InMilliseconds()));
+  if (config.codec == media::cast::CODEC_AUDIO_OPUS)

[miu, 2016/06/30 21:59:42]

With this if...elseif...else statement: I'm thinking it would be more robust to
do this instead for safety:

  switch (config.codec) {
  case CODEC_AUDIO_OPUS: codec_name = kCodecNameOpus;
  case CODEC_VIDEO_VP8: codec_name = kCodecNameVp8;
  case CODEC_VIDEO_H264: codec_name = kCodecNameH264;
  }

This would ensure the compiler let's a developer know they have to also change
this code if they add any codecs to the enum.

[xjz, 2016/07/01 23:52:08]

Done.

+    ext_params->codec_name = kCodecNameOpus;
+  else if (config.codec == media::cast::CODEC_VIDEO_VP8)
+    ext_params->codec_name = kCodecNameVp8;
+  else
+    ext_params->codec_name = kCodecNameH264;
+  ext_params->ssrc = config.sender_ssrc;
+  ext_params->feedback_ssrc = config.receiver_ssrc;
+  if (config.frequency)
+    ext_params->clock_rate.reset(new int(config.frequency));
+  if (config.min_bitrate)
+    ext_params->min_bitrate.reset(new int(config.min_bitrate));
+  if (config.max_bitrate)
+    ext_params->max_bitrate.reset(new int(config.max_bitrate));
+  if (config.channels)
+    ext_params->channels.reset(new int(config.channels));
+  if (config.max_frame_rate > 0.0)
+    ext_params->max_frame_rate.reset(new double(config.max_frame_rate));
 }
 
 }  // namespace
@@ -389,13 +454,12 @@ void CastStreamingNativeHandler::GetSupportedParamsCastRtpStream(
     return;
 
   std::unique_ptr<V8ValueConverter> converter(V8ValueConverter::create());
-  std::vector<CastRtpParams> cast_params = transport->GetSupportedParams();
+  std::vector<FrameSenderConfig> configs = transport->GetSupportedConfigs();
   v8::Local<v8::Array> result =
-      v8::Array::New(args.GetIsolate(),
-                     static_cast<int>(cast_params.size()));
-  for (size_t i = 0; i < cast_params.size(); ++i) {
+      v8::Array::New(args.GetIsolate(), static_cast<int>(configs.size()));
+  for (size_t i = 0; i < configs.size(); ++i) {
     RtpParams params;
-    FromCastRtpParams(cast_params[i], &params);
+    FromFrameSenderConfig(configs[i], &params.payload);
     std::unique_ptr<base::DictionaryValue> params_value = params.ToValue();
     result->Set(
         static_cast<int>(i),
@@ -430,9 +494,9 @@ void CastStreamingNativeHandler::StartCastRtpStream(
     return;
   }
 
-  CastRtpParams cast_params;
+  FrameSenderConfig config;
   v8::Isolate* isolate = context()->v8_context()->GetIsolate();
-  if (!ToCastRtpParamsOrThrow(isolate, *params, &cast_params))
+  if (!ToFrameSenderConfigOrThrow(isolate, params->payload, &config))
     return;
 
   base::Closure start_callback =
@@ -447,7 +511,7 @@ void CastStreamingNativeHandler::StartCastRtpStream(
       base::Bind(&CastStreamingNativeHandler::CallErrorCallback,
                  weak_factory_.GetWeakPtr(),
                  transport_id);
-  transport->Start(cast_params, start_callback, stop_callback, error_callback);
+  transport->Start(config, start_callback, stop_callback, error_callback);
 }
 
 void CastStreamingNativeHandler::StopCastRtpStream(
@@ -676,23 +740,23 @@ bool CastStreamingNativeHandler::FrameReceiverConfigFromArg(
   if (params->codec_name == "OPUS") {
     config->codec = media::cast::CODEC_AUDIO_OPUS;
     config->rtp_timebase = 48000;
-    config->rtp_payload_type = media::cast::kDefaultRtpAudioPayloadType;
+    config->rtp_payload_type = media::cast::RtpPayloadType::AUDIO_OPUS;
   } else if (params->codec_name == "PCM16") {
     config->codec = media::cast::CODEC_AUDIO_PCM16;
     config->rtp_timebase = 48000;
-    config->rtp_payload_type = media::cast::kDefaultRtpAudioPayloadType;
+    config->rtp_payload_type = media::cast::RtpPayloadType::AUDIO_PCM16;
   } else if (params->codec_name == "AAC") {
     config->codec = media::cast::CODEC_AUDIO_AAC;
     config->rtp_timebase = 48000;
-    config->rtp_payload_type = media::cast::kDefaultRtpAudioPayloadType;
+    config->rtp_payload_type = media::cast::RtpPayloadType::AUDIO_AAC;
   } else if (params->codec_name == "VP8") {
     config->codec = media::cast::CODEC_VIDEO_VP8;
     config->rtp_timebase = 90000;
-    config->rtp_payload_type = media::cast::kDefaultRtpVideoPayloadType;
+    config->rtp_payload_type = media::cast::RtpPayloadType::VIDEO_VP8;
   } else if (params->codec_name == "H264") {
     config->codec = media::cast::CODEC_VIDEO_H264;
     config->rtp_timebase = 90000;
-    config->rtp_payload_type = media::cast::kDefaultRtpVideoPayloadType;
+    config->rtp_payload_type = media::cast::RtpPayloadType::VIDEO_H264;
   }
   if (params->rtp_timebase) {
     config->rtp_timebase = *params->rtp_timebase;