media: Refactor SkCanvasVideoRenderer to use SkImages and not SkBitmaps.

media/blink/skcanvas_video_renderer.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 MEDIA_BLINK_SKCANVAS_VIDEO_RENDERER_H_
 #define MEDIA_BLINK_SKCANVAS_VIDEO_RENDERER_H_
 
 #include "base/memory/ref_counted.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "media/base/media_export.h"
+#include "media/base/video_frame.h"
 #include "media/base/video_rotation.h"
 #include "media/filters/context_3d.h"
-#include "third_party/skia/include/core/SkBitmap.h"
+#include "skia/ext/refptr.h"
+#include "third_party/skia/include/core/SkImage.h"
 #include "third_party/skia/include/core/SkXfermode.h"
 #include "ui/gfx/geometry/rect.h"
 
 class SkCanvas;
+class SkImage;
 
 namespace media {
-
-class VideoFrame;
 class VideoImageGenerator;
 
-// Handles rendering of VideoFrames to SkCanvases, doing any necessary YUV
-// conversion and caching of resulting RGB bitmaps.
+// Handles rendering of VideoFrames to SkCanvases.
 class MEDIA_EXPORT SkCanvasVideoRenderer {
  public:
   SkCanvasVideoRenderer();
   ~SkCanvasVideoRenderer();
 
   // Paints |video_frame| on |canvas|, scaling and rotating the result to fit
   // dimensions specified by |dest_rect|.
   // If the format of |video_frame| is VideoFrame::NATIVE_TEXTURE, |context_3d|
   // must be provided.
   //
@@ skipping 26 matching lines @@
   // The format of |video_frame| must be VideoFrame::NATIVE_TEXTURE.
   static void CopyVideoFrameTextureToGLTexture(gpu::gles2::GLES2Interface* gl,
                                                VideoFrame* video_frame,
                                                unsigned int texture,
                                                unsigned int internal_format,
                                                unsigned int type,
                                                bool premultiply_alpha,
                                                bool flip_y);
 
  private:
-  void ResetLastFrame();
-  void ResetAcceleratedLastFrame();
+  void ResetCache();
 
-  // An RGB bitmap and corresponding timestamp of the previously converted
-  // video frame data by software color space conversion.
-  SkBitmap last_frame_;
-  base::TimeDelta last_frame_timestamp_;
-  // If |last_frame_| is not used for a while, it's deleted to save memory.
-  base::DelayTimer<SkCanvasVideoRenderer> frame_deleting_timer_;
+  // Last image created from a generator.
+  skia::RefPtr<SkImage> last_image_;
+  // Timestamp of the videoframe used to generate |last_image_|.
+  base::TimeDelta last_timestamp_ = media::kNoTimestamp();

[dshwang, 2015/06/08 12:21:44]

is it allowed to initialize here in chromium coding style?

[Daniele Castagna, 2015/06/09 23:21:11]

It seems so, look for "Non-Static Class Member Initializers" here:
https://chromium-cpp.appspot.com/

[dshwang, 2015/06/10 07:17:39]

Acknowledged.

+  // If the last videoframe was software |video_generator_| is the generator
+  // used to generate last_image_. SkImage takes ownership of video_generator_,
+  // here we're assuming that video_generator_ is valid as long as the
+  // associated SkImage is not deleted.
+  // https://code.google.com/p/skia/issues/detail?id=3870
+  VideoImageGenerator* video_generator_ = nullptr;
 
-  // This is a hardware accelerated copy of the frame generated by
-  // |accelerated_generator_|.
-  // It's used when |canvas| parameter in Paint() is Ganesh canvas.
-  // Note: all GrContext in SkCanvas instances are same.
-  SkBitmap accelerated_last_frame_;
-  VideoImageGenerator* accelerated_generator_;
-  base::TimeDelta accelerated_last_frame_timestamp_;
-  base::DelayTimer<SkCanvasVideoRenderer> accelerated_frame_deleting_timer_;
+  // texture object backing |last_image_| if the last videoframe had a native
+  // texture.
+  unsigned last_texture_id_ = 0;
+  // GL interface used to create last_texture_id_, if set.
+  gpu::gles2::GLES2Interface* last_gl_ = nullptr;

[dshwang, 2015/06/08 12:21:44]

it's very unsafe. it can easily cause sef fault. If you want to keep texture,
keep GrTexture. GrTexture is deleted automatically when gl context is gone.

[Daniele Castagna, 2015/06/09 23:21:11]

Agreed.
The idea in this CL is to use SkImage, so I'm not sure using a GrTexture would
be aligned with Skia's goal.
Let me discuss with the Skia team how we can fix this. It'd be nice if
SkImage::NewFromTexture could take ownership of the backend texture and delete
it when done.

[dshwang, 2015/06/10 07:17:39]

GrTexture is used in here and there. I think it's fine to use GrTexture. If you
uses GrTexture, you don't need to keep GLES2Interface. Keeping GLES2Interface
pointer is so dangerous because there is no way to refresh when gpu process
down.
In addition, when GPU process crash, GrTexture is automatically deleted.

 
   DISALLOW_COPY_AND_ASSIGN(SkCanvasVideoRenderer);
 };
 
 }  // namespace media
 
 #endif  // MEDIA_BLINK_SKCANVAS_VIDEO_RENDERER_H_