Media Remoting: Update remoting interstitial when status changes.

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
index 5f0d37eacdbc7a5939d49d5b0941e7077d07da8f..00029bc36f439fa32f26ce93847a5a845c934ce6 100644
--- a/media/remoting/remoting_renderer_controller.cc
+++ b/media/remoting/remoting_renderer_controller.cc
@@ -7,10 +7,21 @@
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/threading/thread_checker.h"
+#include "media/base/video_util.h"
 #include "media/remoting/remoting_cdm_context.h"
 
 namespace media {
 
+namespace {
+
+gfx::Size GetRotatedVideoSize(VideoRotation rotation, gfx::Size natural_size) {
+  if (rotation == VIDEO_ROTATION_90 || rotation == VIDEO_ROTATION_270)
+    return gfx::Size(natural_size.height(), natural_size.width());
+  return natural_size;
+}
+
+}  // namespace
+
 RemotingRendererController::RemotingRendererController(
     scoped_refptr<RemotingSourceImpl> remoting_source)
     : remoting_source_(remoting_source), weak_factory_(this) {
@@ -43,6 +54,10 @@ void RemotingRendererController::OnSessionStateChanged() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   VLOG(1) << "OnSessionStateChanged: " << remoting_source_->state();
+  if (remoting_source_->state() ==

[miu, 2016/12/20 00:16:17]

How about just calling UpdateInterstitial() unconditionally (i.e., for all state
changes), and let the code in UpdateInterstitial() decide whether anything
actually changed before running the callback?

[xjz, 2016/12/20 19:32:29]

Done.

+      RemotingSessionState::SESSION_PERMANENTLY_STOPPED)
+    UpdateInterstitial();
+
   UpdateAndMaybeSwitch();
 }
 
@@ -103,17 +118,22 @@ void RemotingRendererController::OnMetadataChanged(
     const PipelineMetadata& metadata) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
+  const gfx::Size old_size = pipeline_metadata_.natural_size;
   pipeline_metadata_ = metadata;
 
   is_encrypted_ = false;
   if (has_video()) {
-    video_decoder_config_ = metadata.video_decoder_config;
-    is_encrypted_ |= video_decoder_config_.is_encrypted();
+    is_encrypted_ |= metadata.video_decoder_config.is_encrypted();
+    pipeline_metadata_.natural_size = GetRotatedVideoSize(
+        pipeline_metadata_.video_rotation, pipeline_metadata_.natural_size);
   }
   if (has_audio()) {
-    audio_decoder_config_ = metadata.audio_decoder_config;
-    is_encrypted_ |= audio_decoder_config_.is_encrypted();
+    is_encrypted_ |= metadata.audio_decoder_config.is_encrypted();
   }
+
+  if (pipeline_metadata_.natural_size != old_size)
+    UpdateInterstitial();
+
   UpdateAndMaybeSwitch();
 }
 
@@ -121,13 +141,13 @@ bool RemotingRendererController::IsVideoCodecSupported() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(has_video());
 
-  switch (video_decoder_config_.codec()) {
+  switch (pipeline_metadata_.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();
+              << pipeline_metadata_.video_decoder_config.codec();
       return false;
   }
 }
@@ -136,7 +156,7 @@ bool RemotingRendererController::IsAudioCodecSupported() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(has_audio());
 
-  switch (audio_decoder_config_.codec()) {
+  switch (pipeline_metadata_.audio_decoder_config.codec()) {
     case AudioCodec::kCodecAAC:
     case AudioCodec::kCodecMP3:
     case AudioCodec::kCodecPCM:
@@ -156,7 +176,7 @@ bool RemotingRendererController::IsAudioCodecSupported() {
       return true;
     default:
       VLOG(2) << "Remoting does not support audio codec: "
-              << audio_decoder_config_.codec();
+              << pipeline_metadata_.audio_decoder_config.codec();
       return false;
   }
 }
@@ -238,9 +258,38 @@ void RemotingRendererController::UpdateAndMaybeSwitch() {
     // force-stop the session when remoting has ended; so no need to call
     // StopRemoting() from here.
     DCHECK(!is_encrypted_);
+    if (!show_interstitial_cb_.is_null()) {
+      show_interstitial_cb_.Run(SkBitmap(), pipeline_metadata_.natural_size,

[miu, 2016/12/20 00:16:17]

Hmm...It looks like you don't need this here (if you follow suggestions above
about just always calling UpdateInterstitial() on state changes).

If you're concerned about the very last frame painted before the
RemoteRendererImpl is destroyed, the RemoteRendererImpl should probably paint a
black frame from it's destructor.

[xjz, 2016/12/20 19:32:29]

Done. Now let RemoteRendererImpl destructor handles painting the last frame.

+                                RemotingInterstitialType::NONE);
+      show_interstitial_cb_.Reset();

[miu, 2016/12/20 00:16:17]

Why do we reset the callback? What if we start remoting again, and then there's
no callback? Should be safe to just leave it.

[xjz, 2016/12/20 19:32:29]

When we start remoting again, a new RemoteRendererImpl will be created and a new
callback will be set. As suggested, now let RemoteRendererImpl reset the
callback.

+    }
     switch_renderer_cb_.Run();
     remoting_source_->StopRemoting(this);
   }
 }
 
+void RemotingRendererController::SetShowInterstitialCallback(
+    const ShowInterstitialCallback& cb) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(!cb.is_null());

[miu, 2016/12/20 00:16:17]

As mentioned above, let RemotingRendererImpl set a null callback to "unset" it.
This allows the "view" to explicitly disconnect from the "controller."

[xjz, 2016/12/20 19:32:29]

Done.

+  DCHECK(show_interstitial_cb_.is_null());
+  show_interstitial_cb_ = cb;
+  UpdateInterstitial();
+}
+
+void RemotingRendererController::UpdateInterstitial() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (show_interstitial_cb_.is_null() ||
+      pipeline_metadata_.natural_size.IsEmpty())
+    return;
+
+  DCHECK(remote_rendering_started_);
+  // TODO(xjz): Download poster image when available.
+  show_interstitial_cb_.Run(
+      SkBitmap(), pipeline_metadata_.natural_size,
+      remoting_source_->state() == RemotingSessionState::SESSION_STARTED
+          ? RemotingInterstitialType::SUCCESS
+          : RemotingInterstitialType::FAIL);
+}
+
 }  // namespace media