Chromium Code Reviews| Index: media/blink/skcanvas_video_renderer.cc |
| diff --git a/media/blink/skcanvas_video_renderer.cc b/media/blink/skcanvas_video_renderer.cc |
| index a61ea3688f0c27e8c9ba92e7c218635e7682c9b2..0631a23c6a29e94220e2b3f05e45851ee8f93937 100644 |
| --- a/media/blink/skcanvas_video_renderer.cc |
| +++ b/media/blink/skcanvas_video_renderer.cc |
| @@ -18,9 +18,6 @@ |
| #include "third_party/skia/include/gpu/GrPaint.h" |
| #include "third_party/skia/include/gpu/GrTexture.h" |
| #include "third_party/skia/include/gpu/GrTextureProvider.h" |
| -#include "third_party/skia/include/gpu/SkGr.h" |
| -#include "third_party/skia/include/gpu/SkGrPixelRef.h" |
| -#include "ui/gfx/skbitmap_operations.h" |
| // Skia internal format depends on a platform. On Android it is ABGR, on others |
| // it is ARGB. |
| @@ -40,10 +37,8 @@ namespace media { |
| namespace { |
| -// This class keeps two temporary resources; software bitmap, hardware bitmap. |
| -// If both bitmap are created and then only software bitmap is updated every |
| -// frame, hardware bitmap outlives until the media player dies. So we delete |
| -// a temporary resource if it is not used for 3 sec. |
| +// This class keeps the last image drawn. |
| +// We delete the temporary resource if it is not used for 3 seconds. |
| const int kTemporaryResourceDeletionDelay = 3; // Seconds; |
| bool CheckColorSpace(const scoped_refptr<VideoFrame>& video_frame, |
| @@ -54,40 +49,6 @@ bool CheckColorSpace(const scoped_refptr<VideoFrame>& video_frame, |
| result == color_space; |
| } |
| -bool IsSkBitmapProperlySizedTexture(const SkBitmap* bitmap, |
| - const gfx::Size& size) { |
| - return bitmap->getTexture() && bitmap->width() == size.width() && |
| - bitmap->height() == size.height(); |
| -} |
| - |
| -bool AllocateSkBitmapTexture(GrContext* gr, |
| - SkBitmap* bitmap, |
| - const gfx::Size& size) { |
| - DCHECK(gr); |
| - GrTextureDesc desc; |
| - // Use kRGBA_8888_GrPixelConfig, not kSkia8888_GrPixelConfig, to avoid |
| - // RGBA to BGRA conversion. |
| - desc.fConfig = kRGBA_8888_GrPixelConfig; |
| - desc.fFlags = kRenderTarget_GrSurfaceFlag; |
| - desc.fSampleCnt = 0; |
| - desc.fOrigin = kTopLeft_GrSurfaceOrigin; |
| - desc.fWidth = size.width(); |
| - desc.fHeight = size.height(); |
| - skia::RefPtr<GrTexture> texture = skia::AdoptRef( |
| - gr->textureProvider()->refScratchTexture( |
| - desc, GrTextureProvider::kExact_ScratchTexMatch)); |
| - if (!texture.get()) |
| - return false; |
| - |
| - SkImageInfo info = SkImageInfo::MakeN32Premul(desc.fWidth, desc.fHeight); |
| - SkGrPixelRef* pixel_ref = SkNEW_ARGS(SkGrPixelRef, (info, texture.get())); |
| - if (!pixel_ref) |
| - return false; |
| - bitmap->setInfo(info); |
| - bitmap->setPixelRef(pixel_ref)->unref(); |
| - return true; |
| -} |
| - |
| class SyncPointClientImpl : public VideoFrame::SyncPointClient { |
| public: |
| explicit SyncPointClientImpl(gpu::gles2::GLES2Interface* gl) : gl_(gl) {} |
| @@ -103,8 +64,8 @@ class SyncPointClientImpl : public VideoFrame::SyncPointClient { |
| DISALLOW_IMPLICIT_CONSTRUCTORS(SyncPointClientImpl); |
| }; |
| -scoped_ptr<SkImage> CreateSkImageFromVideoFrameYUVTextures( |
| - VideoFrame* video_frame, |
| +skia::RefPtr<SkImage> NewSkImageFromVideoFrameYUVTextures( |
| + const scoped_refptr<VideoFrame>& video_frame, |
| const Context3D& context_3d) { |
| // Support only TEXTURE_YUV_420. |
| DCHECK(video_frame->HasTextures()); |
| @@ -151,34 +112,48 @@ scoped_ptr<SkImage> CreateSkImageFromVideoFrameYUVTextures( |
| gl->DeleteTextures(3, source_textures); |
| SyncPointClientImpl client(gl); |
| video_frame->UpdateReleaseSyncPoint(&client); |
| - return make_scoped_ptr(img); |
| + return skia::AdoptRef(img); |
| } |
| -bool CopyVideoFrameSingleTextureToSkBitmap(VideoFrame* video_frame, |
| - SkBitmap* bitmap, |
| - const Context3D& context_3d) { |
| - // Check if we could reuse existing texture based bitmap. |
| - // Otherwise, release existing texture based bitmap and allocate |
| - // a new one based on video size. |
| - if (!IsSkBitmapProperlySizedTexture(bitmap, |
| - video_frame->visible_rect().size())) { |
| - if (!AllocateSkBitmapTexture(context_3d.gr_context, bitmap, |
| - video_frame->visible_rect().size())) { |
| - return false; |
| - } |
| - } |
| +// Creates a SkImage from a |video_frame| backed by native resources. |
| +// The SkImage will take ownership of the underlying resource. |
| +skia::RefPtr<SkImage> NewSkImageFromVideoFrameNative( |
| + const scoped_refptr<VideoFrame>& video_frame, |
| + const Context3D& context_3d) { |
| + DCHECK_EQ(VideoFrame::ARGB, video_frame->format()); |
| + |
| + const gpu::MailboxHolder& mailbox_holder = video_frame->mailbox_holder(0); |
| + DCHECK(mailbox_holder.texture_target == GL_TEXTURE_2D || |
| + mailbox_holder.texture_target == GL_TEXTURE_RECTANGLE_ARB || |
| + mailbox_holder.texture_target == GL_TEXTURE_EXTERNAL_OES) |
| + << mailbox_holder.texture_target; |
| - unsigned texture_id = |
| - static_cast<unsigned>((bitmap->getTexture())->getTextureHandle()); |
| - // If CopyVideoFrameSingleTextureToGLTexture() changes the state of the |
| - // |texture_id|, it's needed to invalidate the state cached in skia, |
| - // but currently the state isn't changed. |
| - |
| - SkCanvasVideoRenderer::CopyVideoFrameSingleTextureToGLTexture( |
| - context_3d.gl, video_frame, texture_id, GL_RGBA, GL_UNSIGNED_BYTE, true, |
| - false); |
| - bitmap->notifyPixelsChanged(); |
| - return true; |
| + gpu::gles2::GLES2Interface* gl = context_3d.gl; |
| + unsigned source_texture = 0; |
| + if (mailbox_holder.texture_target != GL_TEXTURE_2D) { |
| + // TODO(dcastagna): At the moment Skia doesn't support targets different |
| + // than GL_TEXTURE_2D. Avoid this copy once |
| + // https://code.google.com/p/skia/issues/detail?id=3868 is addressed. |
| + gl->GenTextures(1, &source_texture); |
| + DCHECK(source_texture); |
| + gl->BindTexture(GL_TEXTURE_2D, source_texture); |
| + SkCanvasVideoRenderer::CopyVideoFrameSingleTextureToGLTexture( |
| + gl, video_frame.get(), source_texture, GL_RGBA, GL_UNSIGNED_BYTE, true, |
| + false); |
| + } else { |
| + gl->WaitSyncPointCHROMIUM(mailbox_holder.sync_point); |
| + source_texture = gl->CreateAndConsumeTextureCHROMIUM( |
| + mailbox_holder.texture_target, mailbox_holder.mailbox.name); |
|
dshwang
2015/07/15 15:10:52
This code bind the texture of the mailbox and then
Daniele Castagna
2015/07/17 15:42:41
The texture will be deleted after both SkImage and
dshwang
2015/07/20 10:47:47
let me imagine one worse senario.
There are two ac
Daniele Castagna
2015/07/30 19:55:00
GVD waits for release_sync_point_ in VideoFrame be
dshwang
2015/08/03 17:51:52
That's problem. While GVD waits for |release_sync_
Daniele Castagna
2015/08/03 20:52:21
This wouldn't be a problem since last_image_ is no
|
| + } |
| + GrBackendTextureDesc desc; |
| + desc.fFlags = kRenderTarget_GrBackendTextureFlag; |
| + desc.fOrigin = kTopLeft_GrSurfaceOrigin; |
| + desc.fWidth = video_frame->coded_size().width(); |
| + desc.fHeight = video_frame->coded_size().height(); |
| + desc.fConfig = kRGBA_8888_GrPixelConfig; |
| + desc.fTextureHandle = source_texture; |
| + return skia::AdoptRef( |
| + SkImage::NewFromAdoptedTexture(context_3d.gr_context, desc)); |
| } |
| } // anonymous namespace |
| @@ -186,16 +161,14 @@ bool CopyVideoFrameSingleTextureToSkBitmap(VideoFrame* video_frame, |
| // Generates an RGB image from a VideoFrame. Convert YUV to RGB plain on GPU. |
| class VideoImageGenerator : public SkImageGenerator { |
| public: |
| - VideoImageGenerator(const scoped_refptr<VideoFrame>& frame) |
| + VideoImageGenerator(VideoFrame* frame) |
| : SkImageGenerator( |
| SkImageInfo::MakeN32Premul(frame->visible_rect().width(), |
| - frame->visible_rect().height())) |
| - , frame_(frame) { |
| - DCHECK(frame_.get()); |
| - } |
| + frame->visible_rect().height())), |
| + frame_(frame) {} |
| ~VideoImageGenerator() override {} |
| - void set_frame(const scoped_refptr<VideoFrame>& frame) { frame_ = frame; } |
| + void set_frame(VideoFrame* frame) { frame_ = frame; } |
| protected: |
| Result onGetPixels(const SkImageInfo& info, |
| @@ -204,7 +177,7 @@ class VideoImageGenerator : public SkImageGenerator { |
| const Options&, |
| SkPMColor ctable[], |
| int* ctable_count) override { |
| - if (!frame_.get()) |
| + if (!frame_) |
| return kInvalidInput; |
| // If skia couldn't do the YUV conversion on GPU, we will on CPU. |
| SkCanvasVideoRenderer::ConvertVideoFrameToRGBPixels( |
| @@ -216,7 +189,7 @@ class VideoImageGenerator : public SkImageGenerator { |
| void* planes[3], |
| size_t row_bytes[3], |
| SkYUVColorSpace* color_space) override { |
| - if (!frame_.get() || !VideoFrame::IsYuvPlanar(frame_->format()) || |
| + if (!frame_ || !media::VideoFrame::IsYuvPlanar(frame_->format()) || |
| // TODO(rileya): Skia currently doesn't support Rec709 YUV conversion, |
| // or YUVA conversion. Remove this case once it does. As-is we will |
| // fall back on the pure-software path in this case. |
| @@ -280,29 +253,22 @@ class VideoImageGenerator : public SkImageGenerator { |
| } |
| private: |
| - scoped_refptr<VideoFrame> frame_; |
| + VideoFrame* frame_; |
| DISALLOW_IMPLICIT_CONSTRUCTORS(VideoImageGenerator); |
| }; |
| SkCanvasVideoRenderer::SkCanvasVideoRenderer() |
| - : last_frame_timestamp_(media::kNoTimestamp()), |
| - frame_deleting_timer_( |
| - FROM_HERE, |
| - base::TimeDelta::FromSeconds(kTemporaryResourceDeletionDelay), |
| - this, |
| - &SkCanvasVideoRenderer::ResetLastFrame), |
| - accelerated_generator_(nullptr), |
| - accelerated_last_frame_timestamp_(media::kNoTimestamp()), |
| - accelerated_frame_deleting_timer_( |
| + : last_image_deleting_timer_( |
| FROM_HERE, |
| base::TimeDelta::FromSeconds(kTemporaryResourceDeletionDelay), |
| this, |
| - &SkCanvasVideoRenderer::ResetAcceleratedLastFrame) { |
| - last_frame_.setIsVolatile(true); |
| + &SkCanvasVideoRenderer::ResetCache) { |
| } |
| -SkCanvasVideoRenderer::~SkCanvasVideoRenderer() {} |
| +SkCanvasVideoRenderer::~SkCanvasVideoRenderer() { |
| + ResetCache(); |
| +} |
| void SkCanvasVideoRenderer::Paint(const scoped_refptr<VideoFrame>& video_frame, |
| SkCanvas* canvas, |
| @@ -331,91 +297,37 @@ void SkCanvasVideoRenderer::Paint(const scoped_refptr<VideoFrame>& video_frame, |
| return; |
| } |
| - SkBitmap* target_frame = nullptr; |
| + gpu::gles2::GLES2Interface* gl = context_3d.gl; |
| - if (video_frame->HasTextures()) { |
| - // Draw HW Video on both SW and HW Canvas. |
| - // In SW Canvas case, rely on skia drawing Ganesh SkBitmap on SW SkCanvas. |
| - if (accelerated_last_frame_.isNull() || |
| - video_frame->timestamp() != accelerated_last_frame_timestamp_) { |
| - DCHECK(context_3d.gl); |
| + // This might be wrong, two different videos could be drawn to the same |
| + // canvas and the two different videoframes could have the same timestamp. |
| + if (last_image_ && video_frame->timestamp() == last_timestamp_) { |
| + // |last_image_| can be reused. |
| + // |video_generator_| will be set only if the last cache videoframe was |
| + // a software videoframe. |
| + if (video_generator_) |
| + video_generator_->set_frame(video_frame.get()); |
| + } else { |
| + ResetCache(); |
|
dshwang
2015/07/20 10:47:47
allocating texture or system memory is expensive.
Daniele Castagna
2015/07/30 19:54:59
Expensive compared to what?
We used to reallocate
dshwang
2015/08/03 17:51:52
no, previously reuse cached SkBitmap without reall
Daniele Castagna
2015/08/03 20:52:21
Please refer to Brian's answer.
|
| + // Generate a new image. |
| + if (video_frame->HasTextures()) { |
| DCHECK(context_3d.gr_context); |
| - if (accelerated_generator_) { |
| - // Reset SkBitmap used in SWVideo-to-HWCanvas path. |
| - accelerated_last_frame_.reset(); |
| - accelerated_generator_ = nullptr; |
| - } |
| - |
| + DCHECK(gl); |
|
dshwang
2015/07/15 15:10:52
if |video_generator_| exists, delete |video_genera
Daniele Castagna
2015/07/17 15:42:41
ResetCache() above takes care of that.
dshwang
2015/07/20 10:47:47
I have question about ResetCache(). see above comm
|
| if (media::VideoFrame::NumPlanes(video_frame->format()) == 1) { |
| - accelerated_last_image_.reset(); |
| - if (!CopyVideoFrameSingleTextureToSkBitmap( |
| - video_frame.get(), &accelerated_last_frame_, context_3d)) { |
| - NOTREACHED(); |
| - return; |
| - } |
| - DCHECK(video_frame->visible_rect().width() == |
| - accelerated_last_frame_.width() && |
| - video_frame->visible_rect().height() == |
| - accelerated_last_frame_.height()); |
| + last_image_ = NewSkImageFromVideoFrameNative(video_frame, context_3d); |
| } else { |
| - accelerated_last_image_ = CreateSkImageFromVideoFrameYUVTextures( |
| - video_frame.get(), context_3d); |
| - DCHECK(accelerated_last_image_); |
| + last_image_ = |
| + NewSkImageFromVideoFrameYUVTextures(video_frame, context_3d); |
| } |
| - accelerated_last_frame_timestamp_ = video_frame->timestamp(); |
| + SyncPointClientImpl client(gl); |
| + video_frame->UpdateReleaseSyncPoint(&client); |
|
dshwang
2015/07/15 15:10:52
it's duplicated to same code in NewSkImageFromVide
Daniele Castagna
2015/07/17 15:42:41
Moved UpdateReleaseSyncPoint after we draw. I left
dshwang
2015/07/20 10:47:47
Acknowledged.
|
| + } else { |
| + video_generator_ = new VideoImageGenerator(video_frame.get()); |
| + last_image_ = skia::AdoptRef(SkImage::NewFromGenerator(video_generator_)); |
|
dshwang
2015/07/15 15:10:52
now |last_image_| deal with both software SkCanvas
reed1
2015/07/15 15:37:32
It is. If there is a mismatch (e.g. raster-image t
dshwang
2015/07/15 16:21:10
thx for answering. in this case, gpu or raster ima
Daniele Castagna
2015/07/15 16:27:46
Is that a use case we care about? What's a real ex
dshwang
2015/07/15 16:37:13
here's real example,
./blink/tools/run_layout_test
dshwang
2015/07/15 17:56:24
It sounds good to this change. I agree on |last_im
|
| } |
| - target_frame = &accelerated_last_frame_; |
| - accelerated_frame_deleting_timer_.Reset(); |
| - } else if (canvas->getGrContext()) { |
| - if (accelerated_last_frame_.isNull() || |
| - video_frame->timestamp() != accelerated_last_frame_timestamp_) { |
| - // Draw SW Video on HW Canvas. |
| - if (!accelerated_generator_ && !accelerated_last_frame_.isNull()) { |
| - // Reset SkBitmap used in HWVideo-to-HWCanvas path. |
| - accelerated_last_frame_.reset(); |
| - } |
| - accelerated_generator_ = new VideoImageGenerator(video_frame); |
| - |
| - // Note: This takes ownership of |accelerated_generator_|. |
| - if (!SkInstallDiscardablePixelRef(accelerated_generator_, |
| - &accelerated_last_frame_)) { |
| - NOTREACHED(); |
| - return; |
| - } |
| - DCHECK(video_frame->visible_rect().width() == |
| - accelerated_last_frame_.width() && |
| - video_frame->visible_rect().height() == |
| - accelerated_last_frame_.height()); |
| - |
| - accelerated_last_frame_timestamp_ = video_frame->timestamp(); |
| - } else if (accelerated_generator_) { |
| - accelerated_generator_->set_frame(video_frame); |
| - } |
| - target_frame = &accelerated_last_frame_; |
| - accelerated_frame_deleting_timer_.Reset(); |
| - } else { |
| - // Draw SW Video on SW Canvas. |
| - DCHECK(video_frame->IsMappable()); |
| - if (last_frame_.isNull() || |
| - video_frame->timestamp() != last_frame_timestamp_) { |
| - // Check if |bitmap| needs to be (re)allocated. |
| - if (last_frame_.isNull() || |
| - last_frame_.width() != video_frame->visible_rect().width() || |
| - last_frame_.height() != video_frame->visible_rect().height()) { |
| - last_frame_.allocN32Pixels(video_frame->visible_rect().width(), |
| - video_frame->visible_rect().height()); |
| - last_frame_.setIsVolatile(true); |
| - } |
| - last_frame_.lockPixels(); |
| - ConvertVideoFrameToRGBPixels( |
| - video_frame, last_frame_.getPixels(), last_frame_.rowBytes()); |
| - last_frame_.notifyPixelsChanged(); |
| - last_frame_.unlockPixels(); |
| - last_frame_timestamp_ = video_frame->timestamp(); |
| - } |
| - target_frame = &last_frame_; |
| - frame_deleting_timer_.Reset(); |
| + last_timestamp_ = video_frame->timestamp(); |
| } |
| + last_image_deleting_timer_.Reset(); |
| paint.setXfermodeMode(mode); |
| paint.setFilterQuality(kLow_SkFilterQuality); |
| @@ -452,23 +364,26 @@ void SkCanvasVideoRenderer::Paint(const scoped_refptr<VideoFrame>& video_frame, |
| gfx::SizeF(rotated_dest_size.height(), rotated_dest_size.width()); |
| } |
| canvas->scale( |
| - SkFloatToScalar(rotated_dest_size.width() / target_frame->width()), |
| - SkFloatToScalar(rotated_dest_size.height() / target_frame->height())); |
| - canvas->translate(-SkFloatToScalar(target_frame->width() * 0.5f), |
| - -SkFloatToScalar(target_frame->height() * 0.5f)); |
| - } |
| - if (accelerated_last_image_) { |
| - canvas->drawImage(accelerated_last_image_.get(), 0, 0, &paint); |
| - } else { |
| - canvas->drawBitmap(*target_frame, 0, 0, &paint); |
| + SkFloatToScalar(rotated_dest_size.width() / last_image_->width()), |
| + SkFloatToScalar(rotated_dest_size.height() / last_image_->height())); |
| + canvas->translate(-SkFloatToScalar(last_image_->width() * 0.5f), |
| + -SkFloatToScalar(last_image_->height() * 0.5f)); |
| } |
| + canvas->drawImage(last_image_.get(), 0, 0, &paint); |
| + |
| if (need_transform) |
| canvas->restore(); |
| + // Make sure to flush so we can remove the videoframe from the generator. |
| canvas->flush(); |
| - // SkCanvas::flush() causes the generator to generate SkImage, so delete |
| - // |video_frame| not to be outlived. |
| - if (canvas->getGrContext() && accelerated_generator_) |
| - accelerated_generator_->set_frame(nullptr); |
| + if (gl) |
| + gl->Flush(); |
|
dshwang
2015/07/15 15:10:52
Why is this glFlush needed?
Daniele Castagna
2015/07/17 15:42:41
Gone.
I initially put it there to behave in the sa
|
| + |
| + // TODO(dcastagna): here we're assuming |video_generator_| will still be valid |
| + // after SkImage::NewFromGenerator took the ownership. |
| + // Fix this once https://code.google.com/p/skia/issues/detail?id=3870 is |
| + // addressed. |
| + if (video_generator_) |
| + video_generator_->set_frame(nullptr); |
| } |
| void SkCanvasVideoRenderer::Copy(const scoped_refptr<VideoFrame>& video_frame, |
| @@ -629,7 +544,8 @@ void SkCanvasVideoRenderer::CopyVideoFrameSingleTextureToGLTexture( |
| const gpu::MailboxHolder& mailbox_holder = video_frame->mailbox_holder(0); |
| DCHECK(mailbox_holder.texture_target == GL_TEXTURE_2D || |
| mailbox_holder.texture_target == GL_TEXTURE_RECTANGLE_ARB || |
| - mailbox_holder.texture_target == GL_TEXTURE_EXTERNAL_OES); |
| + mailbox_holder.texture_target == GL_TEXTURE_EXTERNAL_OES) |
| + << mailbox_holder.texture_target; |
| gl->WaitSyncPointCHROMIUM(mailbox_holder.sync_point); |
| uint32 source_texture = gl->CreateAndConsumeTextureCHROMIUM( |
| @@ -652,16 +568,11 @@ void SkCanvasVideoRenderer::CopyVideoFrameSingleTextureToGLTexture( |
| video_frame->UpdateReleaseSyncPoint(&client); |
| } |
| -void SkCanvasVideoRenderer::ResetLastFrame() { |
| - last_frame_.reset(); |
| - last_frame_timestamp_ = media::kNoTimestamp(); |
| -} |
| - |
| -void SkCanvasVideoRenderer::ResetAcceleratedLastFrame() { |
| - accelerated_last_image_.reset(); |
| - accelerated_last_frame_.reset(); |
| - accelerated_generator_ = nullptr; |
| - accelerated_last_frame_timestamp_ = media::kNoTimestamp(); |
| +void SkCanvasVideoRenderer::ResetCache() { |
| + // Clear cached values. |
| + video_generator_ = nullptr; |
| + last_image_ = nullptr; |
| + last_timestamp_ = kNoTimestamp(); |
| } |
| } // namespace media |