Media Remoting: Draw remoting interstitial on poster image.

media/remoting/remoting_interstitial_ui.cc

Index: media/remoting/remoting_interstitial_ui.cc
diff --git a/media/remoting/remoting_interstitial_ui.cc b/media/remoting/remoting_interstitial_ui.cc
index 9e3f01d23ea1fe55d4f598ef01ebaa02fba3b65a..7df5a0841371857b9457737fe8e09ae747603a80 100644
--- a/media/remoting/remoting_interstitial_ui.cc
+++ b/media/remoting/remoting_interstitial_ui.cc
@@ -5,9 +5,12 @@
 #include "media/remoting/remoting_interstitial_ui.h"
 
 #include "media/base/media_resources.h"
+#include "media/base/pipeline_metadata.h"
 #include "media/base/video_frame.h"
 #include "media/base/video_renderer_sink.h"
 #include "media/base/video_util.h"
+#include "media/remoting/remoting_renderer_controller.h"
+#include "skia/ext/image_operations.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkTypeface.h"
 #include "third_party/skia/include/effects/SkBlurImageFilter.h"
@@ -27,17 +30,23 @@ gfx::Size GetRotatedVideoSize(VideoRotation rotation, gfx::Size natural_size) {
   return natural_size;
 }
 
+SkBitmap ResizeImage(const SkBitmap& image, int width, int height) {
+  DCHECK(width && height);
+  if (image.width() == width && image.height() == height)
+    return image;
+  return skia::ImageOperations::Resize(

[miu, 2016/11/29 22:56:08]

This will stretch (not preserving aspect ratio). Is that okay? Or, should we be
adding some cropping logic too? To answer this, you should find out what happens
for normal videos by writing some simple HTML and running it in a browser: If
the poster image does not match the size or aspect ratio of the video, how is it
displayed? Or, even better, maybe this is discussed in the W3C spec?

[xjz, 2016/12/02 19:23:10]

Didn't find any specific in the W3C spec. But with some testing in browser,
seems that poster image is kept its aspect ratio and is displayed in center if
its aspect ratio is not same with that of video. Made the change accordingly.

+      image, skia::ImageOperations::RESIZE_BEST, width, height);
+}
+
 }  // namespace
 
 RemotingInterstitialUI::RemotingInterstitialUI(
     VideoRendererSink* video_renderer_sink,
-    const PipelineMetadata& pipeline_metadata)
+    const base::WeakPtr<RemotingRendererController>&
+        remoting_renderer_controller)
     : video_renderer_sink_(video_renderer_sink),
-      pipeline_metadata_(pipeline_metadata) {
-  DCHECK(pipeline_metadata_.has_video);
-  pipeline_metadata_.natural_size = GetRotatedVideoSize(
-      pipeline_metadata_.video_rotation, pipeline_metadata_.natural_size);
-}
+      remoting_renderer_controller_(remoting_renderer_controller),
+      weak_factory_(this) {}
 
 RemotingInterstitialUI::~RemotingInterstitialUI() {}
 
@@ -46,7 +55,6 @@ scoped_refptr<VideoFrame> RemotingInterstitialUI::GetInterstitial(
     bool is_remoting_successful) {
   const gfx::Size canvas_size =
       gfx::Size(background_image.width(), background_image.height());
-  DCHECK(canvas_size == pipeline_metadata_.natural_size);
 
   color_utils::HSL shift = {-1, 0, 0.2};  // Make monochromatic.
   SkBitmap modified_bitmap =
@@ -124,12 +132,43 @@ void RemotingInterstitialUI::ShowInterstitial(bool is_remoting_successful) {
   if (!video_renderer_sink_)
     return;
 
-  // TODO(xjz): Provide poster image, if available, rather than a blank, black
-  // image.
+  // Draw interstial on a blank, black background image before poster image is
+  // downloaded.
+  DisplayInterstitial(SkBitmap(), is_remoting_successful);

[miu, 2016/11/29 22:56:08]

Instead of a blank bitmap, it's possible the poster image was already downloaded
via a prior call to ShowInterstitial(). So, perhaps you should cache it? You
would only provide a black background and re-download the poster image when the
poster URL has changed. See comments in controller .cc file...

[xjz, 2016/12/02 19:23:10]

Done.

+
+  GURL poster = remoting_renderer_controller_->poster();
+  RemotingRendererController::DownloadPosterCallback cb =
+      remoting_renderer_controller_->download_poster_cb();
+  if (poster.is_empty() || cb.is_null())
+    return;
+
+  cb.Run(poster,
+         base::Bind(&RemotingInterstitialUI::PosterDownloaded,
+                    weak_factory_.GetWeakPtr(), is_remoting_successful));

[miu, 2016/11/29 22:56:08]

What if |is_remoting_successful| changes before this callback is run? (Meaning,
what if ShowInterstitial() is called again in the meantime?)

I don't think you should pass |is_remoting_successful| to DisplayInterstitial().
Instead, DisplayInterstitial() should check for that when it is called later.

[xjz, 2016/12/02 19:23:10]

Done.

+}
+
+void RemotingInterstitialUI::PosterDownloaded(bool is_remoting_successful,
+                                              const SkBitmap& poster_image) {
+  if (poster_image.drawsNothing())
+    return;
+
+  DisplayInterstitial(poster_image, is_remoting_successful);
+}
+
+void RemotingInterstitialUI::DisplayInterstitial(const SkBitmap& poster_image,
+                                                 bool is_remoting_successful) {
+  const PipelineMetadata pipeline_metadata =
+      remoting_renderer_controller_->pipeline_metadata();
+  DCHECK(pipeline_metadata.has_video);
+  const gfx::Size size = GetRotatedVideoSize(pipeline_metadata.video_rotation,
+                                             pipeline_metadata.natural_size);
   SkBitmap background_image;
-  const gfx::Size size = pipeline_metadata_.natural_size;
-  background_image.allocN32Pixels(size.width(), size.height());
-  background_image.eraseColor(SK_ColorBLACK);
+  if (poster_image.drawsNothing()) {
+    background_image.allocN32Pixels(size.width(), size.height());
+    background_image.eraseColor(SK_ColorBLACK);
+  } else {
+    background_image = ResizeImage(poster_image, size.width(), size.height());
+  }
 
   const scoped_refptr<VideoFrame> interstitial =
       GetInterstitial(background_image, is_remoting_successful);