Commit 56e621aa authored by Hirokazu Honda's avatar Hirokazu Honda Committed by Commit Bot

Revert "v4l2: Remove GLImage from OutputRecord"

This reverts commit 588d8a94.

Reason for revert: Some CTS/GTS tests has failed due to the CL.

Original change's description:
> v4l2: Remove GLImage from OutputRecord
> 
> When GLImages were introduced to v4l2 in crrev.com/c/517883 to replace
> EGLImages, a scoped ref to a GLImage was added to OutputRecord in place of
> the EGLImage.
> 
> This was not necessary since when we bind the GLImage in GLES2 cmd decoder,
> the decoder increases the ref count and keeps the GLImage alive.
> 
> Additionally, copying the scoped_refptr<GLImage> in a different
> thread than it was created, when we associate GLImage to OutputRecords,
> causes a DCHECK to go off.
> 
> This CL removes GLImage scoped_refptr from
> V4L2SliceVideoDecodeAccelerator::OutputRecord.
> 
> 
> Test: Youtube video on scarlet. Tried to close and open new tabs with videos.
> Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
> Change-Id: Idd2625996b177be35b7a67eab3d9ac8b3f8206d7
> Reviewed-on: https://chromium-review.googlesource.com/911841
> Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
> Reviewed-by: Miguel Casas <mcasas@chromium.org>
> Commit-Queue: Daniele Castagna <dcastagna@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#536139}

TBR=dalecurtis@chromium.org,mcasas@chromium.org,dcastagna@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: Iafc4e43b6d99de251d18c45ec1efcb901d8a2e79
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/936882Reviewed-by: default avatarPawel Osciak <posciak@chromium.org>
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539083}
parent f7788eaf
...@@ -64,6 +64,15 @@ ...@@ -64,6 +64,15 @@
} while (0) } while (0)
namespace media { namespace media {
namespace {
void DropGLImage(scoped_refptr<gl::GLImage> gl_image,
BindGLImageCallback bind_image_cb,
GLuint client_texture_id,
GLuint texture_target) {
bind_image_cb.Run(client_texture_id, texture_target, nullptr, false);
}
} // namespace
// static // static
const uint32_t V4L2SliceVideoDecodeAccelerator::supported_input_fourccs_[] = { const uint32_t V4L2SliceVideoDecodeAccelerator::supported_input_fourccs_[] = {
...@@ -1513,11 +1522,12 @@ bool V4L2SliceVideoDecodeAccelerator::DestroyOutputs(bool dismiss) { ...@@ -1513,11 +1522,12 @@ bool V4L2SliceVideoDecodeAccelerator::DestroyOutputs(bool dismiss) {
VLOGF(1) << "eglDestroySyncKHR failed."; VLOGF(1) << "eglDestroySyncKHR failed.";
} }
child_task_runner_->PostTask( if (output_record.gl_image) {
FROM_HERE, base::BindOnce(IgnoreResult(bind_image_cb_), child_task_runner_->PostTask(
output_record.client_texture_id, FROM_HERE, base::Bind(&DropGLImage, std::move(output_record.gl_image),
device_->GetTextureTarget(), bind_image_cb_, output_record.client_texture_id,
scoped_refptr<gl::GLImage>(), false)); device_->GetTextureTarget()));
}
picture_buffers_to_dismiss.push_back(output_record.picture_id); picture_buffers_to_dismiss.push_back(output_record.picture_id);
} }
...@@ -1630,6 +1640,7 @@ void V4L2SliceVideoDecodeAccelerator::AssignPictureBuffersTask( ...@@ -1630,6 +1640,7 @@ void V4L2SliceVideoDecodeAccelerator::AssignPictureBuffersTask(
OutputRecord& output_record = output_buffer_map_[i]; OutputRecord& output_record = output_buffer_map_[i];
DCHECK(!output_record.at_device); DCHECK(!output_record.at_device);
DCHECK(!output_record.at_client); DCHECK(!output_record.at_client);
DCHECK(!output_record.gl_image);
DCHECK_EQ(output_record.egl_sync, EGL_NO_SYNC_KHR); DCHECK_EQ(output_record.egl_sync, EGL_NO_SYNC_KHR);
DCHECK_EQ(output_record.picture_id, -1); DCHECK_EQ(output_record.picture_id, -1);
DCHECK(output_record.dmabuf_fds.empty()); DCHECK(output_record.dmabuf_fds.empty());
...@@ -1713,14 +1724,15 @@ void V4L2SliceVideoDecodeAccelerator::CreateGLImageFor( ...@@ -1713,14 +1724,15 @@ void V4L2SliceVideoDecodeAccelerator::CreateGLImageFor(
true); true);
decoder_thread_task_runner_->PostTask( decoder_thread_task_runner_->PostTask(
FROM_HERE, FROM_HERE,
base::BindOnce(&V4L2SliceVideoDecodeAccelerator::AssignDmaBufs, base::Bind(&V4L2SliceVideoDecodeAccelerator::AssignGLImage,
base::Unretained(this), buffer_index, picture_buffer_id, base::Unretained(this), buffer_index, picture_buffer_id,
base::Passed(&passed_dmabuf_fds))); gl_image, base::Passed(&passed_dmabuf_fds)));
} }
void V4L2SliceVideoDecodeAccelerator::AssignDmaBufs( void V4L2SliceVideoDecodeAccelerator::AssignGLImage(
size_t buffer_index, size_t buffer_index,
int32_t picture_buffer_id, int32_t picture_buffer_id,
scoped_refptr<gl::GLImage> gl_image,
std::unique_ptr<std::vector<base::ScopedFD>> passed_dmabuf_fds) { std::unique_ptr<std::vector<base::ScopedFD>> passed_dmabuf_fds) {
DVLOGF(3) << "index=" << buffer_index; DVLOGF(3) << "index=" << buffer_index;
DCHECK(decoder_thread_task_runner_->BelongsToCurrentThread()); DCHECK(decoder_thread_task_runner_->BelongsToCurrentThread());
...@@ -1739,10 +1751,12 @@ void V4L2SliceVideoDecodeAccelerator::AssignDmaBufs( ...@@ -1739,10 +1751,12 @@ void V4L2SliceVideoDecodeAccelerator::AssignDmaBufs(
} }
OutputRecord& output_record = output_buffer_map_[buffer_index]; OutputRecord& output_record = output_buffer_map_[buffer_index];
DCHECK(!output_record.gl_image);
DCHECK_EQ(output_record.egl_sync, EGL_NO_SYNC_KHR); DCHECK_EQ(output_record.egl_sync, EGL_NO_SYNC_KHR);
DCHECK(!output_record.at_client); DCHECK(!output_record.at_client);
DCHECK(!output_record.at_device); DCHECK(!output_record.at_device);
output_record.gl_image = gl_image;
if (output_mode_ == Config::OutputMode::IMPORT) { if (output_mode_ == Config::OutputMode::IMPORT) {
DCHECK(output_record.dmabuf_fds.empty()); DCHECK(output_record.dmabuf_fds.empty());
output_record.dmabuf_fds = std::move(*passed_dmabuf_fds); output_record.dmabuf_fds = std::move(*passed_dmabuf_fds);
...@@ -1826,6 +1840,7 @@ void V4L2SliceVideoDecodeAccelerator::ImportBufferForPictureTask( ...@@ -1826,6 +1840,7 @@ void V4L2SliceVideoDecodeAccelerator::ImportBufferForPictureTask(
DCHECK(!iter->at_device); DCHECK(!iter->at_device);
iter->at_client = false; iter->at_client = false;
if (iter->texture_id != 0) { if (iter->texture_id != 0) {
iter->gl_image = nullptr;
child_task_runner_->PostTask( child_task_runner_->PostTask(
FROM_HERE, FROM_HERE,
base::Bind(&V4L2SliceVideoDecodeAccelerator::CreateGLImageFor, base::Bind(&V4L2SliceVideoDecodeAccelerator::CreateGLImageFor,
......
...@@ -90,6 +90,7 @@ class MEDIA_GPU_EXPORT V4L2SliceVideoDecodeAccelerator ...@@ -90,6 +90,7 @@ class MEDIA_GPU_EXPORT V4L2SliceVideoDecodeAccelerator
int32_t picture_id; int32_t picture_id;
GLuint client_texture_id; GLuint client_texture_id;
GLuint texture_id; GLuint texture_id;
scoped_refptr<gl::GLImage> gl_image;
EGLSyncKHR egl_sync; EGLSyncKHR egl_sync;
std::vector<base::ScopedFD> dmabuf_fds; std::vector<base::ScopedFD> dmabuf_fds;
bool cleared; bool cleared;
...@@ -267,12 +268,13 @@ class MEDIA_GPU_EXPORT V4L2SliceVideoDecodeAccelerator ...@@ -267,12 +268,13 @@ class MEDIA_GPU_EXPORT V4L2SliceVideoDecodeAccelerator
const gfx::Size& size, const gfx::Size& size,
uint32_t fourcc); uint32_t fourcc);
// Take the dmabuf |passed_dmabuf_fds|, for |picture_buffer_id|, and use it // Take the GLImage |gl_image|, created for |picture_buffer_id|, and use it
// for OutputRecord at |buffer_index|. The buffer is backed by // for OutputRecord at |buffer_index|. The buffer is backed by
// |passed_dmabuf_fds|, and the OutputRecord takes ownership of them. // |passed_dmabuf_fds|, and the OutputRecord takes ownership of them.
void AssignDmaBufs( void AssignGLImage(
size_t buffer_index, size_t buffer_index,
int32_t picture_buffer_id, int32_t picture_buffer_id,
scoped_refptr<gl::GLImage> gl_image,
// TODO(posciak): (https://crbug.com/561749) we should normally be able to // TODO(posciak): (https://crbug.com/561749) we should normally be able to
// pass the vector by itself via std::move, but it's not possible to do // pass the vector by itself via std::move, but it's not possible to do
// this if this method is used as a callback. // this if this method is used as a callback.
......
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