WIP - Add EME

media/remoting/remoting_renderer_controller.cc

Index: media/remoting/remoting_renderer_controller.cc
diff --git a/media/remoting/remoting_renderer_controller.cc b/media/remoting/remoting_renderer_controller.cc
new file mode 100644
index 0000000000000000000000000000000000000000..3809e7872dd546e3188758422f0466ea473b16c8
--- /dev/null
+++ b/media/remoting/remoting_renderer_controller.cc
@@ -0,0 +1,215 @@
+// 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/remoting_renderer_controller.h"
+
+#include "base/bind.h"
+#include "base/logging.h"
+#include "base/threading/thread_checker.h"
+#include "media/remoting/remoting_cdm.h"
+
+namespace media {
+
+RemotingRendererController::RemotingRendererController(
+    scoped_refptr<RemotingSourceImpl> remoting_source)
+    : remoting_source_(remoting_source), weak_factory_(this) {
+  remoting_source_->AddClient(this);
+}
+
+RemotingRendererController::~RemotingRendererController() {
+  remoting_source_->RemoveClient(this);

[miu, 2016/10/25 04:21:26]

Above this line: DCHECK(thread_checker_.CalledOnValidThread());

[xjz, 2016/10/26 22:00:26]

Done.

+}
+
+void RemotingRendererController::OnStarted(bool success) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  if (success) {
+    VLOG(1) << "Remoting started successively.";
+    if (should_be_remoting_)
+      switch_renderer_cb_.Run();

[miu, 2016/10/25 04:21:26]

For safety, you might want to guard this:

  if (!switch_renderer_cb_.is_null())
    switch_renderer_cb_.Run();

[xjz, 2016/10/26 22:00:26]

This callback should never be null here. Added DCHECK.

+    else
+      remoting_source_->StopRemoting(this);
+  } else {
+    VLOG(1) << "Failed to start remoting.";
+    should_be_remoting_ = false;
+  }
+}
+
+void RemotingRendererController::OnSessionStateChanged() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  VLOG(1) << "OnSessionStateChanged: " << remoting_source_->state();
+  bool can_start_remoting = can_start_remoting_;

[miu, 2016/10/25 04:21:26]

naming nits: The class data member |can_start_remoting_| could mean a lot of
things. How about |session_can_remote_| to be more precise?

Also, consider naming this local variable |session_could_remote| (past tense),
to make the if-statement below more readable. Also, const please. ;)

[xjz, 2016/10/26 22:00:26]

Done.

+  switch (remoting_source_->state()) {
+    case SESSION_CAN_START:
+    case SESSION_STARTING:
+    case SESSION_STARTED:
+      can_start_remoting_ = true;
+      break;
+    case SESSION_STOPPING:
+    case SESSION_UNAVAILABLE:
+    case SESSION_PERMANENTLY_STOPPED:
+      can_start_remoting_ = false;
+      break;
+  }
+  if (can_start_remoting_ != can_start_remoting)
+    UpdateAndMaybeSwitch();
+}
+
+void RemotingRendererController::OnEnteredFullscreen() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  is_fullscreen_ = true;
+  UpdateAndMaybeSwitch();
+}
+
+void RemotingRendererController::OnExitedFullscreen() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  is_fullscreen_ = false;
+  UpdateAndMaybeSwitch();
+}
+
+void RemotingRendererController::OnSetCdm(CdmContext* cdm_context) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  auto* cdm = RemotingCdm::From(cdm_context);
+  if (!cdm)
+    return;
+
+  remoting_source_ = cdm->GetRemotingSource();
+  remoting_source_->AddClient(this);

[miu, 2016/10/25 04:21:26]

Suggest adding this helpful comment for readability:

  remoting_source_->AddClient(this);  // Calls OnSessionStateChanged().

[xjz, 2016/10/26 22:00:26]

Done.

+  UpdateAndMaybeSwitch();
+}
+
+void RemotingRendererController::SetSwitchRendererCallback(
+    const SwitchRendererCallback& cb) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(!cb.is_null());
+
+  switch_renderer_cb_ = cb;
+  UpdateAndMaybeSwitch();
+}
+
+void RemotingRendererController::OnMetadataChanged(
+    const PipelineMetadata& metadata) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  has_video_ =
+      metadata.has_video && metadata.video_decoder_config.IsValidConfig();
+  has_audio_ =
+      metadata.has_audio && metadata.audio_decoder_config.IsValidConfig();
+  if (!has_video_ && !has_audio_)
+    return;

[miu, 2016/10/25 04:21:26]

I think you should remove the early return here. Let UpdateAndMaybeSwitch()
decide what to do (and maybe it would stop an existing remoting session?)

[xjz, 2016/10/26 22:00:26]

Done.

+
+  if (has_video_) {

[miu, 2016/10/25 04:21:26]

Before this line, add:

  is_encrypted_ = false;

(To make sure it will become false if it was true before, but now both tracks
have stopped using encryption.)

[xjz, 2016/10/26 22:00:26]

Done.

+    video_decoder_config_ = metadata.video_decoder_config;
+    is_encrypted_ |= video_decoder_config_.is_encrypted();
+  }
+  if (has_audio_) {
+    audio_decoder_config_ = metadata.audio_decoder_config;
+    is_encrypted_ |= audio_decoder_config_.is_encrypted();
+  }
+  UpdateAndMaybeSwitch();
+}
+
+bool RemotingRendererController::IsVideoCodecSupported() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(has_video_);
+
+  switch (video_decoder_config_.codec()) {
+    case VideoCodec::kCodecH264:
+    case VideoCodec::kCodecVP8:
+      return true;
+    default:
+      VLOG(2) << "Remoting does not support video codec: "
+              << video_decoder_config_.codec();
+      return false;
+  }
+}
+
+bool RemotingRendererController::IsAudioCodecSupported() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(has_audio_);
+
+  switch (audio_decoder_config_.codec()) {
+    case AudioCodec::kCodecAAC:
+    case AudioCodec::kCodecMP3:
+    case AudioCodec::kCodecPCM:
+    case AudioCodec::kCodecVorbis:
+    case AudioCodec::kCodecFLAC:
+    case AudioCodec::kCodecAMR_NB:
+    case AudioCodec::kCodecAMR_WB:
+    case AudioCodec::kCodecPCM_MULAW:
+    case AudioCodec::kCodecGSM_MS:
+    case AudioCodec::kCodecPCM_S16BE:
+    case AudioCodec::kCodecPCM_S24BE:
+    case AudioCodec::kCodecOpus:
+    case AudioCodec::kCodecEAC3:
+    case AudioCodec::kCodecPCM_ALAW:
+    case AudioCodec::kCodecALAC:
+    case AudioCodec::kCodecAC3:
+      return true;
+    default:
+      VLOG(2) << "Remoting does not support audio codec: "
+              << audio_decoder_config_.codec();
+      return false;
+  }
+}
+
+bool RemotingRendererController::ShouldBeRemoting() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  if (!can_start_remoting_)
+    return false;
+  if (has_video_ && !IsVideoCodecSupported())

[miu, 2016/10/25 04:21:26]

Before this, I think you should add (this will simplify the code in
UpdateAndMaybeSwitch()):

  if (switch_renderer_cb_.is_null())
    return false;
  if (!has_audio_ && !has_video_)
    return false;

[xjz, 2016/10/26 22:00:26]

Done.

+    return false;
+  if (has_audio_ && !IsAudioCodecSupported())
+    return false;
+  if (is_encrypted_)
+    return remoting_source_->state() == RemotingSessionState::SESSION_STARTED;

[miu, 2016/10/25 04:21:26]

Consider adding a comment like: "Explicitly ignoring |is_fullscreen_| since all
EME content should trigger the start of remoting immediately, and not later
after switching into fullscreen. This is required by current technical
limitations."

[xjz, 2016/10/26 22:00:26]

Done.

+  return is_fullscreen_;
+}
+
+void RemotingRendererController::UpdateAndMaybeSwitch() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  if (switch_renderer_cb_.is_null())
+    return;
+
+  // Demuxer is not initialized yet.
+  if (!has_audio_ && !has_video_)
+    return;

[miu, 2016/10/25 04:21:26]

If you take my suggestions in ShouldBeRemoting() (above), then you don't need
either of these early-return statements. The code below will always start or
stop remoting, or do nothing, as appropriate.

[xjz, 2016/10/26 22:00:26]

Done.

+
+  bool should_be_remoting = ShouldBeRemoting();
+  if (should_be_remoting_ == should_be_remoting)
+    return;
+
+  // Switch between local renderer and remoting renderer.
+  should_be_remoting_ = should_be_remoting;
+
+  if (should_be_remoting_) {
+    // |switch_renderer_cb_.Run()| will be called after remoting is started
+    // successfully.
+    remoting_source_->StartRemoting(this);
+  } else {
+    switch_renderer_cb_.Run();
+    remoting_source_->StopRemoting(this);
+  }
+}
+
+bool RemotingRendererController::IsRemoting() const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  return should_be_remoting_;
+}
+
+bool RemotingRendererController::IsTerminated() const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  return remoting_source_->state() ==
+         RemotingSessionState::SESSION_PERMANENTLY_STOPPED;
+}
+
+}  // namespace media