rendering_helper - Refactoring - remove the Client interface.

content/common/gpu/media/rendering_helper.h

 // Copyright (c) 2012 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.
 
 #ifndef CONTENT_COMMON_GPU_MEDIA_RENDERING_HELPER_H_
 #define CONTENT_COMMON_GPU_MEDIA_RENDERING_HELPER_H_
 
+#include <deque>
 #include <map>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/gl/gl_bindings.h"
 #include "ui/gl/gl_context.h"
 #include "ui/gl/gl_surface.h"
 
 namespace base {
 class MessageLoop;
 class WaitableEvent;
 }
 
 namespace content {
 
-struct RenderingHelperParams;
+class VideoFrame {

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

Why not use media::VideoFrame?

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

The main reason is I don't know how to use it in our case. It uses mailbox not
texture_id.

Besides, we don't need many things there like coded size and visible size.

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

Ok, texture_id is an issue (the other is not really ;)).
Let's just please rename it to VideoFrameTexture or something similar so that
nobody confuses it with media::VideoFrame.

+ public:
+  uint32 texture_id() const { return texture_id_; }
+  uint32 texture_target() const { return texture_target_; }
+
+  VideoFrame(uint32 texture_target,
+             uint32 texture_id,
+             const base::Closure& no_longer_needed_cb);
+  ~VideoFrame();
+
+ private:
+  uint32 texture_target_;
+  uint32 texture_id_;
+  base::Closure no_longer_needed_cb_;
+};
+
+struct RenderingHelperParams {
+  RenderingHelperParams();
+  ~RenderingHelperParams();
+
+  // The rendering FPS.
+  int rendering_fps;
+
+  // The designed size of each window.

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

s/designed//

Please explain this is used for the regular playback mode with each stream
playing in its own window on the screen, and the members below that they are
used for the thumbnail mode where all frames are rendered in sequence onto one
FBO for comparison/verification purposes in thumbnail test.

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

Sorry, it should be "desired". We may resize the window in order to show all
streams on the screen.

The window_sizes is also used for the thumbnail mode. So, I rephrase the comment
a little bit.


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

+  std::vector<gfx::Size> window_sizes;
+
+  // Whether the frames are rendered as scaled thumbnails within a
+  // larger FBO that is in turn rendered to the window.
+  bool render_as_thumbnails;
+  // The size of the FBO containing all visible thumbnails.
+  gfx::Size thumbnails_page_size;
+  // The size of each thumbnail within the FBO.
+  gfx::Size thumbnail_size;
+};
 
 // Creates and draws textures used by the video decoder.
 // This class is not thread safe and thus all the methods of this class
 // (except for ctor/dtor) ensure they're being run on a single thread.
 class RenderingHelper {
  public:
-  // Interface for the content provider of the RenderingHelper.
-  class Client {
-   public:
-    // Callback to tell client to render the content.
-    virtual void RenderContent(RenderingHelper* helper) = 0;
-
-    // Callback to get the desired window size of the client.
-    virtual const gfx::Size& GetWindowSize() = 0;
-
-   protected:
-    virtual ~Client() {}
-  };
 
   RenderingHelper();
+

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

Not needed.

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

Done.

   ~RenderingHelper();
 
   static bool InitializeOneOff();
 
   // Create the render context and windows by the specified dimensions.
   void Initialize(const RenderingHelperParams& params,
                   base::WaitableEvent* done);
 
   // Undo the effects of Initialize() and signal |*done|.
   void UnInitialize(base::WaitableEvent* done);
 
   // Return a newly-created GLES2 texture id of the specified size, and
   // signal |*done|.
   void CreateTexture(uint32 texture_target,
                      uint32* texture_id,
                      const gfx::Size& size,
                      base::WaitableEvent* done);
 
   // Render thumbnail in the |texture_id| to the FBO buffer using target
   // |texture_target|.
   void RenderThumbnail(uint32 texture_target, uint32 texture_id);
 
-  // Render |texture_id| to the current view port of the screen using target
-  // |texture_target|.
-  void RenderTexture(uint32 texture_target, uint32 texture_id);
+  // Queues the VideoFrame for rendering.
+  void QueueVideoFrame(size_t window_id, scoped_ptr<VideoFrame> video_frame);
+
+  void ClearVideoFrames(size_t window_id);

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

Please document and rename to DropPendingFrames().

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

Done.

 
   // Delete |texture_id|.
   void DeleteTexture(uint32 texture_id);
 
   // Get the platform specific handle to the OpenGL display.
   void* GetGLDisplay();
 
   // Get the platform specific handle to the OpenGL context.
   void* GetGLContext();
 
   // Get rendered thumbnails as RGB.
   // Sets alpha_solid to true if the alpha channel is entirely 0xff.
   void GetThumbnailsAsRGB(std::vector<unsigned char>* rgb,
                           bool* alpha_solid,
                           base::WaitableEvent* done);
 
  private:
+  struct RenderingClient {

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

I think we should not call it a Client, because this is not a Client, just a
struct with data for clients. Otherwise it would imply that we owned the clients
by having a vector of them below.

How about RenderedVideo ?

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

Done.

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

+    gfx::Rect render_area;

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

Please documentation for all members.

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

Done.

+    bool last_frame_rendered;

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

Isn't this equivalent to pending_frames.empty() ?

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

Why? do you mean pending_frames.size() == 1? We will keep the last frame even it
has been rendered.

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

Sorry, this was something that I forgot to remove before posting my comments for
previous PS. Please ignore.

+    base::Closure wait_rendering_cb;
+    std::deque<VideoFrame*> pending_frames;

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

Please keep scoped_refptrs here.

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

Done.

+
+    inline RenderingClient() : last_frame_rendered(false) {}

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

s/inline//

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

Done.

+  };
+
   void Clear();
 
   void RenderContent();
 
-  void LayoutRenderingAreas();
+  void LayoutRenderingAreas(const std::vector<gfx::Size>& window_sizes);
 
+  // Render |texture_id| to the current view port of the screen using target
+  // |texture_target|.
+

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

Remove empty line please.

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

Done.

+  void RenderTexture(uint32 texture_target, uint32 texture_id);
   // Timer to trigger the RenderContent() repeatly.
   scoped_ptr<base::RepeatingTimer<RenderingHelper> > render_timer_;
   base::MessageLoop* message_loop_;
 
   scoped_refptr<gfx::GLContext> gl_context_;
   scoped_refptr<gfx::GLSurface> gl_surface_;
 
   gfx::AcceleratedWidget window_;
 
   gfx::Size screen_size_;
 
-  // The rendering area of each window on the screen.
-  std::vector<gfx::Rect> render_areas_;
-
-  std::vector<base::WeakPtr<Client> > clients_;
+  std::vector<RenderingClient> clients_;
 
   bool render_as_thumbnails_;
   int frame_count_;
   GLuint thumbnails_fbo_id_;
   GLuint thumbnails_texture_id_;
   gfx::Size thumbnails_fbo_size_;
   gfx::Size thumbnail_size_;
   GLuint program_;
   base::TimeDelta frame_duration_;
 
   DISALLOW_COPY_AND_ASSIGN(RenderingHelper);
 };
 
-struct RenderingHelperParams {
-  RenderingHelperParams();
-  ~RenderingHelperParams();
-
-  // The rendering FPS.
-  int rendering_fps;
-
-  // The clients who provide the content for rendering.
-  std::vector<base::WeakPtr<RenderingHelper::Client> > clients;
-
-  // Whether the frames are rendered as scaled thumbnails within a
-  // larger FBO that is in turn rendered to the window.
-  bool render_as_thumbnails;
-  // The size of the FBO containing all visible thumbnails.
-  gfx::Size thumbnails_page_size;
-  // The size of each thumbnail within the FBO.
-  gfx::Size thumbnail_size;
-};
 }  // namespace content
 
 #endif  // CONTENT_COMMON_GPU_MEDIA_RENDERING_HELPER_H_