media: Refactor SkCanvasVideoRenderer to use SkImages and not SkBitmaps.

media/blink/skcanvas_video_renderer.h

Index: media/blink/skcanvas_video_renderer.h
diff --git a/media/blink/skcanvas_video_renderer.h b/media/blink/skcanvas_video_renderer.h
index 9b75cd01e0fad04c242dd1463c73c6142e853845..aef1e04123a4dde533fdce8827239c893881d52b 100644
--- a/media/blink/skcanvas_video_renderer.h
+++ b/media/blink/skcanvas_video_renderer.h
@@ -9,21 +9,21 @@
 #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();
@@ -70,24 +70,24 @@ class MEDIA_EXPORT SkCanvasVideoRenderer {
                                                bool flip_y);
 
  private:
-  void ResetLastFrame();
-  void ResetAcceleratedLastFrame();
-
-  // 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_;
-
-  // 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_;
+  void ResetCache();
+
+  // 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;
+
+  // 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);
 };