gpu: media: RenderingHelper: wait for the display & window to be ready

content/common/gpu/media/rendering_helper.cc

 // Copyright 2013 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 "content/common/gpu/media/rendering_helper.h"
 
 #include <algorithm>
 #include <numeric>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/mac/scoped_nsautorelease_pool.h"
 #include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
 #include "base/strings/stringize_macros.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/time/time.h"
 #include "ui/gl/gl_context.h"
 #include "ui/gl/gl_implementation.h"
 #include "ui/gl/gl_surface.h"
 
 #if defined(OS_WIN)
 #include <windows.h>
 #endif
 
 #if defined(USE_X11)
 #include "ui/gfx/x/x11_types.h"
 #endif
 
 #if defined(ARCH_CPU_X86_FAMILY) && defined(USE_X11)
 #include "ui/gl/gl_surface_glx.h"
 #define GL_VARIANT_GLX 1
 #else
 #include "ui/gl/gl_surface_egl.h"
 #define GL_VARIANT_EGL 1
 #endif
 
 #if defined(USE_OZONE)
 #if defined(OS_CHROMEOS)
 #include "ui/display/chromeos/display_configurator.h"
+#include "ui/display/types/display_constants.h"
 #endif  // defined(OS_CHROMEOS)
 #include "ui/ozone/public/ozone_platform.h"
 #include "ui/ozone/public/ui_thread_gpu.h"
 #include "ui/platform_window/platform_window.h"
 #include "ui/platform_window/platform_window_delegate.h"
 #endif  // defined(USE_OZONE)
 
 // Helper for Shader creation.
 static void CreateShader(GLuint program,
                          GLenum type,
@@ skipping 11 matching lines @@
   }
   glAttachShader(program, shader);
   glDeleteShader(shader);
   CHECK_EQ(static_cast<int>(glGetError()), GL_NO_ERROR);
 }
 
 namespace content {
 
 #if defined(USE_OZONE)
 
+class DisplayConfiguratorObserver : public ui::DisplayConfigurator::Observer {
+ public:
+  DisplayConfiguratorObserver(base::RunLoop* loop) : loop_(loop) {}
+  ~DisplayConfiguratorObserver() override {}
+
+ private:
+  // ui::DisplayConfigurator::Observer overrides:
+  void OnDisplayModeChanged(
+      const ui::DisplayConfigurator::DisplayStateList& outputs) override {
+    if (!loop_)
+      return;
+    loop_->Quit();
+    loop_ = nullptr;
+  }
+  void OnDisplayModeChangeFailed(
+      ui::MultipleDisplayState failed_new_state) override {
+    LOG(FATAL) << "Could not configure display";
+  }
+
+  base::RunLoop* loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(DisplayConfiguratorObserver);
+};
+
 class RenderingHelper::StubOzoneDelegate : public ui::PlatformWindowDelegate {
  public:
-  StubOzoneDelegate() : accelerated_widget_(gfx::kNullAcceleratedWidget) {
+  StubOzoneDelegate(base::RunLoop* loop)
+      : accelerated_widget_(gfx::kNullAcceleratedWidget), loop_(loop) {
     ui_thread_gpu_.Initialize();
     platform_window_ = ui::OzonePlatform::GetInstance()->CreatePlatformWindow(
         this, gfx::Rect());
   }
   virtual ~StubOzoneDelegate() {}
 
-  void OnBoundsChanged(const gfx::Rect& new_bounds) override {}
+  void OnBoundsChanged(const gfx::Rect& new_bounds) override {
+    if (!loop_)
+      return;
+    loop_->Quit();
+    loop_ = nullptr;
+  }
 
   void OnDamageRect(const gfx::Rect& damaged_region) override {}
 
   void DispatchEvent(ui::Event* event) override {}
 
   void OnCloseRequest() override {}
   void OnClosed() override {}
 
   void OnWindowStateChanged(ui::PlatformWindowState new_state) override {}
 
@@ skipping 10 matching lines @@
   }
 
   gfx::Size GetSize() { return platform_window_->GetBounds().size(); }
 
   ui::PlatformWindow* platform_window() const { return platform_window_.get(); }
 
  private:
   ui::UiThreadGpu ui_thread_gpu_;
   scoped_ptr<ui::PlatformWindow> platform_window_;
   gfx::AcceleratedWidget accelerated_widget_;
+  base::RunLoop* loop_;
 
   DISALLOW_COPY_AND_ASSIGN(StubOzoneDelegate);
 };
 
 #endif  // defined(USE_OZONE)
 
 RenderingHelperParams::RenderingHelperParams()
     : rendering_fps(0), warm_up_iterations(0), render_as_thumbnails(false) {
 }
 
@@ skipping 37 matching lines @@
 RenderingHelper::RenderingHelper() {
   window_ = gfx::kNullAcceleratedWidget;
   Clear();
 }
 
 RenderingHelper::~RenderingHelper() {
   CHECK_EQ(videos_.size(), 0U) << "Must call UnInitialize before dtor.";
   Clear();
 }
 
-void RenderingHelper::Initialize(const RenderingHelperParams& params,
-                                 base::WaitableEvent* done) {
-  // Use videos_.size() != 0 as a proxy for the class having already been
-  // Initialize()'d, and UnInitialize() before continuing.
-  if (videos_.size()) {
-    base::WaitableEvent done(false, false);
-    UnInitialize(&done);
-    done.Wait();
-  }
-
-  render_task_.Reset(
-      base::Bind(&RenderingHelper::RenderContent, base::Unretained(this)));
-
-  frame_duration_ = params.rendering_fps > 0
-                        ? base::TimeDelta::FromSeconds(1) / params.rendering_fps
-                        : base::TimeDelta();
-
-  render_as_thumbnails_ = params.render_as_thumbnails;
-  message_loop_ = base::MessageLoop::current();
-
+void RenderingHelper::Setup() {
 #if defined(OS_WIN)
   window_ = CreateWindowEx(0,
                            L"Static",
                            L"VideoDecodeAcceleratorTest",
                            WS_OVERLAPPEDWINDOW | WS_VISIBLE,
                            0,
                            0,
                            GetSystemMetrics(SM_CXSCREEN),
                            GetSystemMetrics(SM_CYSCREEN),
                            NULL,
@@ skipping 22 matching lines @@
                           0 /* border width */,
                           depth,
                           CopyFromParent /* class */,
                           CopyFromParent /* visual */,
                           (CWBackPixel | CWOverrideRedirect),
                           &window_attributes);
   XStoreName(display, window_, "VideoDecodeAcceleratorTest");
   XSelectInput(display, window_, ExposureMask);
   XMapWindow(display, window_);
 #elif defined(USE_OZONE)
-  platform_window_delegate_.reset(new RenderingHelper::StubOzoneDelegate());
+  base::MessageLoop::ScopedNestableTaskAllower nest_loop(
+      base::MessageLoop::current());
+  base::RunLoop wait_window_resize;
+
+  platform_window_delegate_.reset(
+      new RenderingHelper::StubOzoneDelegate(&wait_window_resize));
   window_ = platform_window_delegate_->accelerated_widget();
 #if defined(OS_CHROMEOS)
+  // We hold onto the main loop here to wait for the DisplayController
+  // to give us the size of the display so we can create a window of
+  // the same size.
+  base::RunLoop wait_display_setup;
+  DisplayConfiguratorObserver observer(&wait_display_setup);
   display_configurator_.reset(new ui::DisplayConfigurator());
+  display_configurator_->AddObserver(&observer);
   display_configurator_->Init(true);
   display_configurator_->ForceInitialConfigure(0);
+  // Make sure all the display configuration is applied.
+  wait_display_setup.Run();
+  display_configurator_->RemoveObserver(&observer);
   platform_window_delegate_->platform_window()->SetBounds(
       gfx::Rect(display_configurator_->framebuffer_size()));
 #else
   platform_window_delegate_->platform_window()->SetBounds(gfx::Rect(800, 600));
 #endif
+  wait_window_resize.Run();

[dnicoara, 2015/01/22 19:23:17]

I'm not fine with adding more complexity to platform implementations. This can
be replaced with base::RunLoop().RunUntilIdle() to flush all the pending updates
without adding extra complexity to the platform code.

[Owen Lin, 2015/01/23 08:57:38]

I think you're talking the other CL (859943004). But correctness should be
considered before simplicity. 

Anyway, to prevent the dilemma, how about just signaling done in the end of the
function. So we don't need to wait for VSYNC info here. The current
rendering_helper has been modified to live without vsync provider. We will try
to get the VSync info when rendering a frame later.

The drawback is we will never know whether we get the VSYNC or not. I hope we
can get a performance alert in that case.

[dnicoara, 2015/01/23 15:10:13]

The current approach is correct. Platforms have 2 choices:
 1) If they use their own message loop (or connection) they need to provide
their own flushing primitives and those would need to be explicitly called from
functions such as SetBounds().
 2) If they use the built-in message loop, then they have to rely on running the
message loop to achieve the same effect. Think of base::RunLoop().RunUntilIdle()
as a flush.

Note, "wait_window_resize.Run()" essentially does the same thing.

Let me know if I should expand, I'm happy to provide more details.

[Owen Lin, 2015/01/26 07:25:28]

Thanks for the explanation. I am kind of understanding it. It makes sure all
these setup commands have been flushed to the hardware (or something) before the
vsync command.

I think the basic issue is we don't understand what we need to make
VSyncProvider works.

 #else
 #error unknown platform
 #endif
   CHECK(window_ != gfx::kNullAcceleratedWidget);
+}
+
+void RenderingHelper::TearDown() {
+#if defined(OS_WIN)
+  if (window_)
+    DestroyWindow(window_);
+#elif defined(USE_X11)
+  // Destroy resources acquired in Initialize, in reverse-acquisition order.
+  if (window_) {
+    CHECK(XUnmapWindow(gfx::GetXDisplay(), window_));
+    CHECK(XDestroyWindow(gfx::GetXDisplay(), window_));
+  }
+#elif defined(USE_OZONE)
+  platform_window_delegate_.reset();
+#if defined(OS_CHROMEOS)
+  display_configurator_->PrepareForExit();
+  display_configurator_.reset();
+#endif
+#endif
+  window_ = gfx::kNullAcceleratedWidget;
+}
+
+void RenderingHelper::Initialize(const RenderingHelperParams& params,
+                                 base::WaitableEvent* done) {
+  // Use videos_.size() != 0 as a proxy for the class having already been
+  // Initialize()'d, and UnInitialize() before continuing.
+  if (videos_.size()) {
+    base::WaitableEvent done(false, false);
+    UnInitialize(&done);
+    done.Wait();
+  }
+
+  render_task_.Reset(
+      base::Bind(&RenderingHelper::RenderContent, base::Unretained(this)));
+
+  frame_duration_ = params.rendering_fps > 0
+                        ? base::TimeDelta::FromSeconds(1) / params.rendering_fps
+                        : base::TimeDelta();
+
+  render_as_thumbnails_ = params.render_as_thumbnails;
+  message_loop_ = base::MessageLoop::current();
 
   gl_surface_ = gfx::GLSurface::CreateViewGLSurface(window_);

[dnicoara, 2015/01/22 19:23:17]

While reading more of this code I realized this is also broken.

GLSurface::InitializeOneOff() is called from the main thread while this is
called from the renderer_helper thread. This violates threading assumptions in
Ozone (and X11).

Though, given that this review is fixing a M41 beta blocker bug, perhaps just
file a bug against this to have it fixed later.

   screen_size_ = gl_surface_->GetSize();
 
   gl_context_ = gfx::GLContext::CreateGLContext(
       NULL, gl_surface_.get(), gfx::PreferIntegratedGpu);
   CHECK(gl_context_->MakeCurrent(gl_surface_.get()));
 
   CHECK_GT(params.window_sizes.size(), 0U);
   videos_.resize(params.window_sizes.size());
   LayoutRenderingAreas(params.window_sizes);
 
@@ skipping 163 matching lines @@
   for (int i = 0; i < warm_up_iterations; ++i) {
     RenderTexture(GL_TEXTURE_2D, texture_id);
     gl_surface_->SwapBuffers();
   }
   glDeleteTextures(1, &texture_id);
 }
 
 void RenderingHelper::UnInitialize(base::WaitableEvent* done) {
   CHECK_EQ(base::MessageLoop::current(), message_loop_);
 
-#if defined(USE_OZONE) && defined(OS_CHROMEOS)
-  display_configurator_->PrepareForExit();
-  display_configurator_.reset();
-#endif
-
   render_task_.Cancel();
 
   if (render_as_thumbnails_) {
     glDeleteTextures(1, &thumbnails_texture_id_);
     glDeleteFramebuffersEXT(1, &thumbnails_fbo_id_);
   }
 
   gl_context_->ReleaseCurrent(gl_surface_.get());
   gl_context_ = NULL;
   gl_surface_ = NULL;
@@ skipping 124 matching lines @@
 void RenderingHelper::Clear() {
   videos_.clear();
   message_loop_ = NULL;
   gl_context_ = NULL;
   gl_surface_ = NULL;
 
   render_as_thumbnails_ = false;
   frame_count_ = 0;
   thumbnails_fbo_id_ = 0;
   thumbnails_texture_id_ = 0;
-
-#if defined(OS_WIN)
-  if (window_)
-    DestroyWindow(window_);
-#elif defined(USE_X11)
-  // Destroy resources acquired in Initialize, in reverse-acquisition order.
-  if (window_) {
-    CHECK(XUnmapWindow(gfx::GetXDisplay(), window_));
-    CHECK(XDestroyWindow(gfx::GetXDisplay(), window_));
-  }
-#elif defined(USE_OZONE)
-  platform_window_delegate_.reset();
-#endif
-  window_ = gfx::kNullAcceleratedWidget;
 }
 
 void RenderingHelper::GetThumbnailsAsRGB(std::vector<unsigned char>* rgb,
                                          bool* alpha_solid,
                                          base::WaitableEvent* done) {
   CHECK(render_as_thumbnails_);
 
   const size_t num_pixels = thumbnails_fbo_size_.GetArea();
   std::vector<unsigned char> rgba;
   rgba.resize(num_pixels * 4);
@@ skipping 177 matching lines @@
   // When the rendering falls behind, drops frames.
   while (scheduled_render_time_ < target) {
     scheduled_render_time_ += frame_duration_;
     DropOneFrameForAllVideos();
   }
 
   message_loop_->PostDelayedTask(
       FROM_HERE, render_task_.callback(), target - now);
 }
 }  // namespace content