Define remote playback proto buffer

media/remoting/rpc/rpc.cc

Index: media/remoting/rpc/rpc.cc
diff --git a/media/remoting/rpc/rpc.cc b/media/remoting/rpc/rpc.cc
new file mode 100644
index 0000000000000000000000000000000000000000..b6e5da41c8958c0332cbf5ab1aa21a23b404cca0
--- /dev/null
+++ b/media/remoting/rpc/rpc.cc
@@ -0,0 +1,325 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "media/remoting/rpc/rpc.h"
+
+#include <algorithm>
+
+#include "base/logging.h"
+#include "base/time/time.h"
+#include "base/values.h"
+#include "media/base/encryption_scheme.h"
+
+namespace media {
+namespace remoting {
+
+const int kInvalidHandle = -1;
+const int kReceiverHandle = 0;
+
+void SerializeEncryptionScheme(
+    pb::EncryptionScheme* message,
+    const ::media::EncryptionScheme& encryption_scheme) {
+  message->set_mode(
+      static_cast<pb::EncryptionScheme::CipherMode>(encryption_scheme.mode()));
+  message->set_encrypt_blocks(encryption_scheme.pattern().encrypt_blocks());
+  message->set_skip_blocks(encryption_scheme.pattern().skip_blocks());
+}
+
+void DeserializeEncryptionScheme(const pb::EncryptionScheme& message,
+                                 ::media::EncryptionScheme* encryption_scheme) {
+  ::media::EncryptionScheme::CipherMode mode =
+      ::media::EncryptionScheme::CIPHER_MODE_UNENCRYPTED;
+  ::media::EncryptionScheme::Pattern pattern;
+  uint32_t encrypt_blocks = 0;
+  uint32_t skip_blocks = 0;
+
+  if (message.has_mode())
+    mode = static_cast<::media::EncryptionScheme::CipherMode>(message.mode());

[miu, 2016/09/13 05:40:57]

The static_cast between enum types worries me: There's no compile-time check as
to whether the set of possible values changes (or changes in ordering) in either
the media/base files or our remoting proto. Without this compile-time check,
there's nothing to stop another developer from breaking Media Remoting, and we
don't want to have to reactively keep fixing code forever and ever.

Also, it's possible developers will want to make changes to the media/base enums
without breaking media remoting. Conversion functions would de-couple things so
that can happen safely (e.g., for backwards-compatibility).

So, we should have explicit enum type conversion functions to ensure the
compiler won't allow anyone to commit breaking changes. Since this is tedious, I
went ahead and wrote a script to generate the code for you. I'll e-mail it to
you for inclusion in this change.

+  if (message.has_encrypt_blocks())

[miu, 2016/09/13 05:40:57]

Do we need "has" guards here? The default value will be zero, right?

[erickung1, 2016/09/15 02:13:33]

Done.

+    encrypt_blocks = message.encrypt_blocks();
+  if (message.has_skip_blocks())
+    skip_blocks = message.skip_blocks();
+  DCHECK(encrypt_blocks >= 0 && skip_blocks >= 0);

[miu, 2016/09/13 05:40:57]

This will crash the process on bad external input. Instead, we should return
false when this happens and shut down media remoting.

[erickung1, 2016/09/15 02:13:33]

Done. Actually I just realized that both are unit32_t so it's always true.
remove this DCHECK

+
+  pattern = ::media::EncryptionScheme::Pattern(encrypt_blocks, skip_blocks);
+  *encryption_scheme = ::media::EncryptionScheme(mode, pattern);
+}
+
+void SerializeAudioConfig(const ::media::AudioDecoderConfig& audio_config,
+                          pb::AudioDecoderConfig* audio_message) {
+  if (!audio_config.IsValidConfig())

[miu, 2016/09/13 05:40:57]

Since this is the sender generating an RPC message, trying to serialize an
invalid audio config would be a bug. So, this should be:

  DCHECK(audio_config.IsValidConfig());

[erickung1, 2016/09/15 02:13:33]

Done.

+    return;
+
+  audio_message->set_codec(
+      static_cast<pb::AudioDecoderConfig_Codec>(audio_config.codec()));
+  audio_message->set_sample_format(
+      static_cast<pb::AudioDecoderConfig_SampleFormat>(
+          audio_config.sample_format()));
+  audio_message->set_channel_layout(
+      static_cast<pb::AudioDecoderConfig_ChannelLayout>(
+          audio_config.channel_layout()));
+  audio_message->set_samples_per_second(audio_config.samples_per_second());
+  audio_message->set_seek_preroll_usec(
+      audio_config.seek_preroll().InMicroseconds());
+  audio_message->set_codec_delay(audio_config.codec_delay());
+
+  if (!audio_config.extra_data().empty()) {
+    audio_message->set_extra_data(audio_config.extra_data().data(),
+                                  audio_config.extra_data().size());
+  }
+
+  if (audio_config.is_encrypted()) {
+    pb::EncryptionScheme* encryption_scheme_message =
+        audio_message->mutable_encryption_scheme();
+    SerializeEncryptionScheme(encryption_scheme_message,
+                              audio_config.encryption_scheme());
+  }
+}
+
+bool DeserializeAudioConfig(const pb::AudioDecoderConfig& audio_message,
+                            ::media::AudioDecoderConfig* audio_config) {
+  DCHECK(audio_config);
+
+  ::media::AudioCodec codec = ::media::kUnknownAudioCodec;
+  ::media::SampleFormat sample_format = ::media::kUnknownSampleFormat;
+  ::media::ChannelLayout channel_layout = ::media::CHANNEL_LAYOUT_NONE;
+  int samples_per_second = 0;
+  base::TimeDelta seek_preroll = base::TimeDelta();
+  int codec_delay = 0;
+  ::media::EncryptionScheme encryption_scheme;
+  std::vector<uint8_t> extra_data;
+
+  codec = static_cast<::media::AudioCodec>(audio_message.codec());
+  sample_format =
+      static_cast<::media::SampleFormat>(audio_message.sample_format());
+  channel_layout =
+      static_cast<::media::ChannelLayout>(audio_message.channel_layout());
+  samples_per_second = audio_message.samples_per_second();
+  seek_preroll =
+      base::TimeDelta::FromMicroseconds(audio_message.seek_preroll_usec());
+  codec_delay = audio_message.codec_delay();
+
+  if (audio_message.has_extra_data()) {
+    extra_data.assign(audio_message.extra_data().begin(),
+                      audio_message.extra_data().end());
+  }
+
+  const pb::EncryptionScheme encryption_scheme_message =
+      audio_message.encryption_scheme();
+  DeserializeEncryptionScheme(encryption_scheme_message, &encryption_scheme);
+
+  audio_config->Initialize(codec, sample_format, channel_layout,

[miu, 2016/09/13 05:40:58]

Please see my comments from last week about simplifying this function, and also
DeserializeVideoConfig() and DeserializeCdmPromiseResult().

[erickung1, 2016/09/15 02:13:33]

Done.

+                           samples_per_second, extra_data, encryption_scheme,
+                           seek_preroll, codec_delay);
+  return true;

[miu, 2016/09/13 05:40:58]

Should be: return audio_config->IsValidConfig();

[erickung1, 2016/09/15 02:13:33]

Done.

+}
+
+bool DeserializeVideoConfig(const pb::VideoDecoderConfig& video_message,
+                            ::media::VideoDecoderConfig* video_config) {
+  ::media::VideoCodec codec = ::media::kUnknownVideoCodec;
+  ::media::VideoCodecProfile profile = ::media::VIDEO_CODEC_PROFILE_UNKNOWN;
+  ::media::VideoPixelFormat format = ::media::PIXEL_FORMAT_UNKNOWN;
+  ::media::ColorSpace color_space = ::media::COLOR_SPACE_UNSPECIFIED;
+  ::media::EncryptionScheme encryption_scheme;
+  std::vector<uint8_t> extra_data;
+
+  codec = static_cast<::media::VideoCodec>(video_message.codec());
+  profile = static_cast<::media::VideoCodecProfile>(video_message.profile());
+  format = static_cast<::media::VideoPixelFormat>(video_message.format());
+  color_space = static_cast<::media::ColorSpace>(video_message.color_space());
+
+  if (video_message.has_extra_data()) {
+    extra_data.assign(video_message.extra_data().begin(),
+                      video_message.extra_data().end());
+  }
+
+  gfx::Size coded_size(video_message.coded_size().width(),
+                       video_message.coded_size().height());
+  gfx::Rect visible_rect(video_message.visible_rect().x(),
+                         video_message.visible_rect().y(),
+                         video_message.visible_rect().width(),
+                         video_message.visible_rect().height());
+  gfx::Size natural_size(video_message.natural_size().width(),
+                         video_message.natural_size().height());
+
+  const pb::EncryptionScheme& encryption_scheme_message =
+      video_message.encryption_scheme();
+  DeserializeEncryptionScheme(encryption_scheme_message, &encryption_scheme);
+
+  video_config->Initialize(codec, profile, format, color_space, coded_size,
+                           visible_rect, natural_size, extra_data,
+                           encryption_scheme);
+  return true;

[miu, 2016/09/13 05:40:57]

Should be: return video_config->IsValidConfig();

[erickung1, 2016/09/15 02:13:33]

Done.

+}
+
+bool DeserializeCdmPromiseResult(const pb::CdmPromise& promise_message,
+                                 CdmPromiseResult* result) {
+  DCHECK(promise_message.has_success());
+  bool success = promise_message.success();
+  if (success) {
+    *result = CdmPromiseResult::SuccessResult();
+    return true;
+  }
+
+  ::media::MediaKeys::Exception exception = ::media::MediaKeys::UNKNOWN_ERROR;
+  uint32_t system_code = 0;
+  std::string error_message;
+
+  exception =
+      static_cast<::media::MediaKeys::Exception>(promise_message.exception());
+  system_code = promise_message.system_code();
+  error_message = promise_message.error_message();
+  *result = CdmPromiseResult(exception, system_code, error_message);
+  return true;
+}
+
+void SerializeVideoConfig(const ::media::VideoDecoderConfig& video_config,
+                          pb::VideoDecoderConfig* video_message) {
+  if (!video_config.IsValidConfig())

[miu, 2016/09/13 05:40:57]

ditto here: Should be DCHECK(...);

[erickung1, 2016/09/15 02:13:33]

Done.

+    return;
+
+  video_message->set_codec(
+      static_cast<pb::VideoDecoderConfig_Codec>(video_config.codec()));
+  video_message->set_profile(
+      static_cast<pb::VideoDecoderConfig_Profile>(video_config.profile()));
+  video_message->set_format(
+      static_cast<pb::VideoDecoderConfig_Format>(video_config.format()));
+  video_message->set_color_space(static_cast<pb::VideoDecoderConfig_ColorSpace>(
+      video_config.color_space()));
+
+  pb::Size* coded_size_message = video_message->mutable_coded_size();
+  coded_size_message->set_width(video_config.coded_size().width());
+  coded_size_message->set_height(video_config.coded_size().height());
+
+  pb::Rect* visible_rect_message = video_message->mutable_visible_rect();
+  visible_rect_message->set_x(video_config.visible_rect().x());
+  visible_rect_message->set_y(video_config.visible_rect().y());
+  visible_rect_message->set_width(video_config.visible_rect().width());
+  visible_rect_message->set_height(video_config.visible_rect().height());
+
+  pb::Size* natural_size_message = video_message->mutable_natural_size();
+  natural_size_message->set_width(video_config.natural_size().width());
+  natural_size_message->set_height(video_config.natural_size().height());
+
+  if (!video_config.extra_data().empty()) {
+    video_message->set_extra_data(video_config.extra_data().data(),
+                                  video_config.extra_data().size());
+  }
+
+  if (video_config.is_encrypted()) {
+    pb::EncryptionScheme* encryption_scheme_message =
+        video_message->mutable_encryption_scheme();
+    SerializeEncryptionScheme(encryption_scheme_message,
+                              video_config.encryption_scheme());
+  }
+}
+
+void SerializeCdmConfig(const ::media::CdmConfig& cdm_config,
+                        pb::CdmInitialize* message) {
+  std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
+
+  message->set_allow_distinctive_identifier(
+      cdm_config.allow_distinctive_identifier);
+  message->set_allow_persistent_state(cdm_config.allow_persistent_state);
+  message->set_use_hw_secure_codecs(cdm_config.use_hw_secure_codecs);
+}
+
+void SerializeCdmPromiseResult(pb::CdmPromise* promise_message,
+                               const CdmPromiseResult& result) {
+  promise_message->set_success(result.success());
+  if (!result.success()) {
+    promise_message->set_exception(
+        static_cast<pb::MediaKeysException>(result.exception()));
+    promise_message->set_system_code(result.system_code());
+    promise_message->set_error_message(result.error_message());
+  }
+}
+
+void SerializeCdmKeyInformation(
+    const ::media::CdmKeysInfo& keys_information,
+    pb::CdmClientOnSessionKeysChange* key_change_message) {
+  for (const auto& info : keys_information) {
+    pb::CdmKeyInformation* key = key_change_message->add_key_information();
+    key->set_key_id(info->key_id.data(), info->key_id.size());
+    key->set_status(
+        static_cast<pb::CdmKeyInformation::KeyStatus>(info->status));
+    key->set_system_code(info->system_code);
+  }
+}
+
+bool DeserializeCdmKeyInformation(
+    const pb::CdmClientOnSessionKeysChange keychange_message,
+    CdmKeysInfo* key_information) {
+  DCHECK(key_information);
+  for (int i = 0; i < keychange_message.key_information_size(); ++i) {
+    const pb::CdmKeyInformation key_info_msg =
+        keychange_message.key_information(i);
+
+    std::unique_ptr<::media::CdmKeyInformation> key(
+        new ::media::CdmKeyInformation(
+            key_info_msg.key_id(),
+            static_cast<::media::CdmKeyInformation::KeyStatus>(
+                key_info_msg.status()),
+            key_info_msg.system_code()));
+    key_information->push_back(std::move(key));
+  }
+  return true;
+}
+
+bool CdmPromiseFromMessage(const pb::RpcMessage& message,
+                           CdmPromiseResult* result,
+                           int* cdm_id,
+                           std::string* session_id) {
+  DCHECK(message.has_cdm_promise_rpc());

[miu, 2016/09/13 05:40:57]

ditto: Return false instead.

[erickung1, 2016/09/15 02:13:33]

Done.

+  const auto& promise_message = message.cdm_promise_rpc();
+
+  DCHECK(DeserializeCdmPromiseResult(promise_message, result));

[miu, 2016/09/13 05:40:57]

This function won't be called in release builds.

[erickung1, 2016/09/15 02:13:33]

Done.

+
+  if (cdm_id)
+    *cdm_id = promise_message.cdm_id();
+  if (session_id)
+    *session_id = promise_message.session_id();
+
+  return true;
+}
+
+void CdmPromiseToMessage(pb::CdmPromise* promise_message,
+                         const CdmPromiseResult& result,
+                         const std::string& session_id) {
+  SerializeCdmPromiseResult(promise_message, result);
+  if (!session_id.empty())

[miu, 2016/09/13 05:40:57]

Should this be a DCHECK(). Or, is it valid to sometimes send this message
without a session_id?

[erickung1, 2016/09/15 02:13:33]

Done.

+    promise_message->set_session_id(session_id);
+}
+
+void CdmPromiseWithCdmIdToMessage(pb::CdmPromise* promise_message,
+                                  const CdmPromiseResult& result,
+                                  int cdm_id) {
+  SerializeCdmPromiseResult(promise_message, result);
+  promise_message->set_cdm_id(cdm_id);
+}
+
+//==============================================================================
+CdmPromiseResult::CdmPromiseResult()
+    : CdmPromiseResult(::media::MediaKeys::UNKNOWN_ERROR, 0, "") {}
+
+CdmPromiseResult::CdmPromiseResult(::media::MediaKeys::Exception exception,
+                                   uint32_t system_code,
+                                   std::string error_message)
+    : success_(false),
+      exception_(exception),
+      system_code_(system_code),
+      error_message_(error_message) {}
+
+CdmPromiseResult::CdmPromiseResult(const CdmPromiseResult& other) = default;
+
+CdmPromiseResult::~CdmPromiseResult() = default;
+
+CdmPromiseResult CdmPromiseResult::SuccessResult() {
+  CdmPromiseResult result(static_cast<::media::MediaKeys::Exception>(0), 0, "");
+  result.success_ = true;
+  return result;
+}
+
+}  // namespace remoting
+}  // namespace media