Media Remoting: Update remoting interstitial when status changes.

media/remoting/remoting_renderer_controller.cc

 // 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/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) {
   remoting_source_->AddClient(this);
 }
 
 RemotingRendererController::~RemotingRendererController() {
   DCHECK(thread_checker_.CalledOnValidThread());
   remoting_source_->RemoveClient(this);
 }
 
 void RemotingRendererController::OnStarted(bool success) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (success) {
     VLOG(1) << "Remoting started successively.";
     if (remote_rendering_started_) {
       DCHECK(!switch_renderer_cb_.is_null());
       switch_renderer_cb_.Run();
     } else {
       remoting_source_->StopRemoting(this);
     }
   } else {
     VLOG(1) << "Failed to start remoting.";
     remote_rendering_started_ = false;
   }
 }

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

Looks like you should call UpdateInterstitial() here (at the end of this
method).

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

As chatted face to face, there is no need to call it here, since it will be
called when either RemoteRendererImpl is created or conditions change during
remoting session.

 
 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();
 }
 
 void RemotingRendererController::OnEnteredFullscreen() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   is_fullscreen_ = true;
   UpdateAndMaybeSwitch();
 }
 
@@ skipping 40 matching lines @@
   DCHECK(thread_checker_.CalledOnValidThread());
 
   remoting_source_->StartDataPipe(std::move(audio_data_pipe),
                                   std::move(video_data_pipe), done_callback);
 }
 
 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();
 }
 
 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;
   }
 }
 
 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:
     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();
+              << pipeline_metadata_.audio_decoder_config.codec();
       return false;
   }
 }
 
 bool RemotingRendererController::ShouldBeRemoting() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (switch_renderer_cb_.is_null())
     return false;  // No way to switch to a RemotingRenderImpl.
 
@@ skipping 61 matching lines @@
     }
     // |switch_renderer_cb_.Run()| will be called after remoting is started
     // successfully.
     remoting_source_->StartRemoting(this);
   } else {
     // For encrypted content, it's only valid to switch to remoting renderer,
     // and never back to the local renderer. The RemotingCdmController will
     // 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