rendering_helper - Refactoring - remove the Client interface.

content/common/gpu/media/rendering_helper.cc

Index: content/common/gpu/media/rendering_helper.cc
diff --git a/content/common/gpu/media/rendering_helper.cc b/content/common/gpu/media/rendering_helper.cc
index 7ee940148d2d2f9e741fd38f0b3c1d798fad37df..5f8ae749e1eef31f9e7972f3bbd8c075cb711452 100644
--- a/content/common/gpu/media/rendering_helper.cc
+++ b/content/common/gpu/media/rendering_helper.cc
@@ -9,9 +9,11 @@
 #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/stl_util.h"
 #include "base/strings/stringize_macros.h"
 #include "base/synchronization/waitable_event.h"
 #include "ui/gl/gl_context.h"
@@ -60,6 +62,19 @@ RenderingHelperParams::RenderingHelperParams() {}
 
 RenderingHelperParams::~RenderingHelperParams() {}
 
+VideoFrame::VideoFrame(uint32 texture_target,
+                       uint32 texture_id,
+                       const base::Closure& no_longer_needed_cb)
+    : texture_target_(texture_target),
+      texture_id_(texture_id),
+      no_longer_needed_cb_(no_longer_needed_cb) {
+}
+
+VideoFrame::~VideoFrame() {
+  if (!no_longer_needed_cb_.is_null())

[Pawel Osciak, 2014/08/14 04:48:23]

I don't think we can allow this to be optional, better to check it's not null in
constructor?

[Owen Lin, 2014/08/14 09:20:41]

Done.

+    base::ResetAndReturn(&no_longer_needed_cb_).Run();
+}
+
 // static
 bool RenderingHelper::InitializeOneOff() {
   base::CommandLine* cmd_line = base::CommandLine::ForCurrentProcess();
@@ -84,7 +99,7 @@ RenderingHelper::~RenderingHelper() {
 
 void RenderingHelper::Initialize(const RenderingHelperParams& params,
                                  base::WaitableEvent* done) {
-  // Use cients_.size() != 0 as a proxy for the class having already been
+  // Use clients_.size() != 0 as a proxy for the class having already been
   // Initialize()'d, and UnInitialize() before continuing.
   if (clients_.size()) {
     base::WaitableEvent done(false, false);
@@ -153,9 +168,9 @@ void RenderingHelper::Initialize(const RenderingHelperParams& params,
       NULL, gl_surface_, gfx::PreferIntegratedGpu);
   gl_context_->MakeCurrent(gl_surface_);
 
-  clients_ = params.clients;
-  CHECK_GT(clients_.size(), 0U);
-  LayoutRenderingAreas();
+  CHECK_GT(params.window_sizes.size(), 0U);
+  clients_.resize(params.window_sizes.size());

[Pawel Osciak, 2014/08/14 04:48:22]

This should go to LayoutRenderingAreas() maybe?

[Owen Lin, 2014/08/14 09:20:41]

mmm... I would prefer keep it here. As the name suggesting, it only do the
layout. We may need other initialization for all the clients.

[Pawel Osciak, 2014/08/15 05:45:15]

Ok, my issue was that we use videos_.size and window_sizes.size() exchangeably 
in LayoutRenderingAreas. Would prefer to defensively ensure they are equal
somehow. Up to you how.

+  LayoutRenderingAreas(params.window_sizes);
 
   if (render_as_thumbnails_) {
     CHECK_EQ(clients_.size(), 1U);
@@ -370,6 +385,24 @@ void RenderingHelper::RenderThumbnail(uint32 texture_target,
   ++frame_count_;
 }
 
+void RenderingHelper::QueueVideoFrame(size_t window_id,
+                                      scoped_ptr<VideoFrame> video_frame) {
+  RenderingClient* client = &clients_[window_id];
+
+  // Pop the front if it has been rendered.
+  if (client->last_frame_rendered && !client->pending_frames.empty()) {
+    delete client->pending_frames.front();

[Pawel Osciak, 2014/08/14 04:48:23]

Please use container of scoped_refptrs and just pop.

[Owen Lin, 2014/08/14 09:20:40]

Done

+    client->pending_frames.pop_front();
+  }
+
+  client->pending_frames.push_back(video_frame.release());
+  client->last_frame_rendered = false;

[Pawel Osciak, 2014/08/14 04:48:23]

Technically this should go to the if clause?

[Owen Lin, 2014/08/14 09:20:41]

Yes, the code is modified. 

+}
+
+void RenderingHelper::ClearVideoFrames(size_t window_id) {
+  STLDeleteElements(&clients_[window_id].pending_frames);

[Pawel Osciak, 2014/08/14 04:48:23]

client->last_frame_rendered = false just in case?

[Owen Lin, 2014/08/14 09:20:40]

Good catch. Thanks.

+}
+
 void RenderingHelper::RenderTexture(uint32 texture_target, uint32 texture_id) {
   // The ExternalOES sampler is bound to GL_TEXTURE1 and the Texture2D sampler
   // is bound to GL_TEXTURE0.
@@ -464,18 +497,34 @@ void RenderingHelper::RenderContent() {
   if (render_as_thumbnails_) {
     // In render_as_thumbnails_ mode, we render the FBO content on the
     // screen instead of the decoded textures.
-    GLSetViewPort(render_areas_[0]);
+    GLSetViewPort(clients_[0].render_area);
     RenderTexture(GL_TEXTURE_2D, thumbnails_texture_id_);
-  } else {
-    for (size_t i = 0; i < clients_.size(); ++i) {
-      if (clients_[i]) {
-        GLSetViewPort(render_areas_[i]);
-        clients_[i]->RenderContent(this);
-      }
-    }
+    gl_surface_->SwapBuffers();
+    return;
+  }
+
+  for (size_t i = 0; i < clients_.size(); ++i) {
+    RenderingClient* client = &clients_[i];
+    if (client->pending_frames.empty())
+      continue;
+    VideoFrame* frame = client->pending_frames.front();
+    GLSetViewPort(client->render_area);
+    RenderTexture(frame->texture_target(), frame->texture_id());
   }
 
   gl_surface_->SwapBuffers();
+
+  // Releases the rendered frame after SwapBuffers() if it is not the last one.

[Pawel Osciak, 2014/08/14 04:48:22]

Instead of this, just check if client->pending_frames.size() > 1 in the first
loop and if yes, pop it from there into a scoped_ptr, then let it drop out of
scope after this method returns.

That way you can also take swapbuffers out of the render thumbnails loop and put
it at the end of the method:


RenderContent() {
  std::vector<scoped_refptr> frames_to_be_returned;
  if (render_as_thumbnails_) {
    // render
  } else {
    // render
    if (client->pending_frames.size() > 1) {
      frames_to_be_returned.push_back(client->pending_frames.front());
      client->pending_frames.pop_front();
      client->last_frame_rendered = false;
    }
  }
  
  SwapBuffers();
}

[Owen Lin, 2014/08/14 09:20:41]

Done. I was avoid using RefCounted  object since the VF actually has one owner.
But it did simplify the code a lot. 

On 2014/08/14 04:48:22, Pawel Osciak wrote:

+  for (size_t i = 0; i < clients_.size(); ++i) {
+    RenderingClient* client = &clients_[i];
+    if (client->pending_frames.size() > 1) {
+      delete client->pending_frames.front();
+      client->pending_frames.pop_front();
+      client->last_frame_rendered = false;
+      continue;
+    }
+    client->last_frame_rendered = true;
+  }
 }
 
 // Helper function for the LayoutRenderingAreas(). The |lengths| are the
@@ -492,7 +541,8 @@ static void ScaleAndCalculateOffsets(std::vector<int>* lengths,
   }
 }
 
-void RenderingHelper::LayoutRenderingAreas() {
+void RenderingHelper::LayoutRenderingAreas(
+    const std::vector<gfx::Size>& window_sizes) {
   // Find the number of colums and rows.
   // The smallest n * n or n * (n + 1) > number of clients.
   size_t cols = sqrt(clients_.size() - 1) + 1;
@@ -504,31 +554,30 @@ void RenderingHelper::LayoutRenderingAreas() {
   std::vector<int> offset_x(cols);
   std::vector<int> offset_y(rows);
 
-  for (size_t i = 0; i < clients_.size(); ++i) {
-    const gfx::Size& window_size = clients_[i]->GetWindowSize();
-    widths[i % cols] = std::max(widths[i % cols], window_size.width());
-    heights[i / cols] = std::max(heights[i / cols], window_size.height());
+  for (size_t i = 0; i < window_sizes.size(); ++i) {
+    const gfx::Size& size = window_sizes[i];
+    widths[i % cols] = std::max(widths[i % cols], size.width());
+    heights[i / cols] = std::max(heights[i / cols], size.height());
   }
 
   ScaleAndCalculateOffsets(&widths, &offset_x, screen_size_.width());
   ScaleAndCalculateOffsets(&heights, &offset_y, screen_size_.height());
 
   // Put each render_area_ in the center of each cell.
-  render_areas_.clear();
-  for (size_t i = 0; i < clients_.size(); ++i) {
-    const gfx::Size& window_size = clients_[i]->GetWindowSize();
+  for (size_t i = 0; i < window_sizes.size(); ++i) {
+    const gfx::Size& size = window_sizes[i];
     float scale =
-        std::min(static_cast<float>(widths[i % cols]) / window_size.width(),
-                 static_cast<float>(heights[i / cols]) / window_size.height());
+        std::min(static_cast<float>(widths[i % cols]) / size.width(),
+                 static_cast<float>(heights[i / cols]) / size.height());
 
     // Don't scale up the texture.
     scale = std::min(1.0f, scale);
 
-    size_t w = scale * window_size.width();
-    size_t h = scale * window_size.height();
+    size_t w = scale * size.width();
+    size_t h = scale * size.height();
     size_t x = offset_x[i % cols] + (widths[i % cols] - w) / 2;
     size_t y = offset_y[i / cols] + (heights[i / cols] - h) / 2;
-    render_areas_.push_back(gfx::Rect(x, y, w, h));
+    clients_[i].render_area = gfx::Rect(x, y, w, h);
   }
 }
 }  // namespace content