Commit a0bb0465 authored by Jeffrey Kardatzke's avatar Jeffrey Kardatzke Committed by Commit Bot

Revert "media/gpu/chromeos/MailboxVFConverter: Set visible size to SharedImage"

This reverts commit 7d82db05.

Reason for revert: This crashes. The frame pointer referenced on line 256 was std::move'd on line 240 so it is invalid.

Original change's description:
> media/gpu/chromeos/MailboxVFConverter: Set visible size to SharedImage
> 
> MailboxVideoFrameConverter sets coded size to SharedImage. It
> causes that a green line is shown at the bottom and right edge
> when playing a video.
> 
> This fixes the issue by setting visible size to SharedImage so
> that GPU doesn't access non visible area.
> Note that MailboxVideoFrameConverter needs to recreate
> SharedImage if the visible rectangle of the current video frame
> is changed, which should be rare though.
> 
> Bug: 1043582
> Test: Play a 1080p video on soraka with --enable-features=ChromeosVideoDecoder
> Change-Id: I76878a27fa92d755380d9b92b94d20e765056fac
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2011586
> Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
> Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#734795}

TBR=mcasas@chromium.org,hiroh@chromium.org,acourbot@chromium.org

Change-Id: Ieea5d53c3deb5aae963da4f6cdb82cc9648f8f44
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1043582
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2020409Reviewed-by: default avatarJeffrey Kardatzke <jkardatzke@google.com>
Commit-Queue: Jeffrey Kardatzke <jkardatzke@google.com>
Cr-Commit-Position: refs/heads/master@{#735166}
parent b3593d07
...@@ -39,11 +39,9 @@ class MailboxVideoFrameConverter::ScopedSharedImage { ...@@ -39,11 +39,9 @@ class MailboxVideoFrameConverter::ScopedSharedImage {
gpu::SharedImageStub::SharedImageDestructionCallback; gpu::SharedImageStub::SharedImageDestructionCallback;
ScopedSharedImage(const gpu::Mailbox& mailbox, ScopedSharedImage(const gpu::Mailbox& mailbox,
const gfx::Size& size,
scoped_refptr<base::SingleThreadTaskRunner> gpu_task_runner, scoped_refptr<base::SingleThreadTaskRunner> gpu_task_runner,
DestroySharedImageCB destroy_shared_image_cb) DestroySharedImageCB destroy_shared_image_cb)
: mailbox_(mailbox), : mailbox_(mailbox),
size_(size),
destroy_shared_image_cb_(std::move(destroy_shared_image_cb)), destroy_shared_image_cb_(std::move(destroy_shared_image_cb)),
destruction_task_runner_(std::move(gpu_task_runner)) {} destruction_task_runner_(std::move(gpu_task_runner)) {}
~ScopedSharedImage() { ~ScopedSharedImage() {
...@@ -57,11 +55,9 @@ class MailboxVideoFrameConverter::ScopedSharedImage { ...@@ -57,11 +55,9 @@ class MailboxVideoFrameConverter::ScopedSharedImage {
} }
const gpu::Mailbox& mailbox() const { return mailbox_; } const gpu::Mailbox& mailbox() const { return mailbox_; }
const gfx::Size& size() const { return size_; }
private: private:
const gpu::Mailbox mailbox_; const gpu::Mailbox mailbox_;
const gfx::Size size_;
DestroySharedImageCB destroy_shared_image_cb_; DestroySharedImageCB destroy_shared_image_cb_;
const scoped_refptr<base::SequencedTaskRunner> destruction_task_runner_; const scoped_refptr<base::SequencedTaskRunner> destruction_task_runner_;
...@@ -154,14 +150,8 @@ void MailboxVideoFrameConverter::ConvertFrame(scoped_refptr<VideoFrame> frame) { ...@@ -154,14 +150,8 @@ void MailboxVideoFrameConverter::ConvertFrame(scoped_refptr<VideoFrame> frame) {
gpu::Mailbox mailbox; gpu::Mailbox mailbox;
const UniqueID origin_frame_id = origin_frame->unique_id(); const UniqueID origin_frame_id = origin_frame->unique_id();
if (shared_images_.find(origin_frame_id) != shared_images_.end()) { if (shared_images_.find(origin_frame_id) != shared_images_.end())
// If visible_rect()'s size is changed, recreate SharedImage with the new mailbox = shared_images_[origin_frame_id]->mailbox();
// visible_rect() size.
if (shared_images_[origin_frame_id]->size() == frame->visible_rect().size())
mailbox = shared_images_[origin_frame_id]->mailbox();
else
shared_images_[origin_frame_id].reset();
}
input_frame_queue_.emplace(frame, origin_frame_id); input_frame_queue_.emplace(frame, origin_frame_id);
...@@ -252,8 +242,7 @@ void MailboxVideoFrameConverter::ConvertFrameOnGPUThread( ...@@ -252,8 +242,7 @@ void MailboxVideoFrameConverter::ConvertFrameOnGPUThread(
} }
std::unique_ptr<ScopedSharedImage> scoped_shared_image; std::unique_ptr<ScopedSharedImage> scoped_shared_image;
scoped_shared_image = GenerateSharedImageOnGPUThread( scoped_shared_image = GenerateSharedImageOnGPUThread(origin_frame);
origin_frame, frame->visible_rect().size());
if (!scoped_shared_image) if (!scoped_shared_image)
return; return;
...@@ -273,8 +262,7 @@ void MailboxVideoFrameConverter::ConvertFrameOnGPUThread( ...@@ -273,8 +262,7 @@ void MailboxVideoFrameConverter::ConvertFrameOnGPUThread(
std::unique_ptr<MailboxVideoFrameConverter::ScopedSharedImage> std::unique_ptr<MailboxVideoFrameConverter::ScopedSharedImage>
MailboxVideoFrameConverter::GenerateSharedImageOnGPUThread( MailboxVideoFrameConverter::GenerateSharedImageOnGPUThread(
VideoFrame* video_frame, VideoFrame* video_frame) {
const gfx::Size& visible_size) {
DCHECK(gpu_task_runner_->BelongsToCurrentThread()); DCHECK(gpu_task_runner_->BelongsToCurrentThread());
DVLOGF(4) << "frame: " << video_frame->unique_id(); DVLOGF(4) << "frame: " << video_frame->unique_id();
...@@ -312,8 +300,8 @@ MailboxVideoFrameConverter::GenerateSharedImageOnGPUThread( ...@@ -312,8 +300,8 @@ MailboxVideoFrameConverter::GenerateSharedImageOnGPUThread(
const bool success = shared_image_stub->CreateSharedImage( const bool success = shared_image_stub->CreateSharedImage(
mailbox, shared_image_stub->channel()->client_id(), mailbox, shared_image_stub->channel()->client_id(),
std::move(gpu_memory_buffer_handle), *buffer_format, std::move(gpu_memory_buffer_handle), *buffer_format,
gpu::kNullSurfaceHandle, visible_size, video_frame->ColorSpace(), gpu::kNullSurfaceHandle, video_frame->coded_size(),
shared_image_usage); video_frame->ColorSpace(), shared_image_usage);
if (!success) { if (!success) {
OnError(FROM_HERE, "Failed to create shared image."); OnError(FROM_HERE, "Failed to create shared image.");
return nullptr; return nullptr;
...@@ -321,7 +309,7 @@ MailboxVideoFrameConverter::GenerateSharedImageOnGPUThread( ...@@ -321,7 +309,7 @@ MailboxVideoFrameConverter::GenerateSharedImageOnGPUThread(
// There's no need to UpdateSharedImage() after CreateSharedImage(). // There's no need to UpdateSharedImage() after CreateSharedImage().
return std::make_unique<ScopedSharedImage>( return std::make_unique<ScopedSharedImage>(
mailbox, visible_size, gpu_task_runner_, mailbox, gpu_task_runner_,
shared_image_stub->GetSharedImageDestructionCallback(mailbox)); shared_image_stub->GetSharedImageDestructionCallback(mailbox));
} }
......
...@@ -99,8 +99,7 @@ class MEDIA_GPU_EXPORT MailboxVideoFrameConverter : public VideoFrameConverter { ...@@ -99,8 +99,7 @@ class MEDIA_GPU_EXPORT MailboxVideoFrameConverter : public VideoFrameConverter {
// alive for the duration of this method. This method runs on // alive for the duration of this method. This method runs on
// |gpu_task_runner_|. // |gpu_task_runner_|.
std::unique_ptr<ScopedSharedImage> GenerateSharedImageOnGPUThread( std::unique_ptr<ScopedSharedImage> GenerateSharedImageOnGPUThread(
VideoFrame* video_frame, VideoFrame* video_frame);
const gfx::Size& visible_size);
// Registers the mapping between a DMA-buf VideoFrame and the SharedImage. // Registers the mapping between a DMA-buf VideoFrame and the SharedImage.
// |origin_frame| must be kept alive for the duration of this method. // |origin_frame| must be kept alive for the duration of this method.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment