Commit cd9523cc authored by Daniele Castagna's avatar Daniele Castagna Committed by Commit Bot

Reland: v4l2: Remove GLImage from OutputRecord

This CL was originally (crrev.com/c/911841) reverted since it broke ARC++
video playback where we initialize the decoder with bind_cb_ set to null.
The new patch removes the call to bind_cb_ unconditionally.
That's what we want, since we need the client mapping from texture id to GLImage
to stay valid for scanout to work. Additionally, unbinding the images doesn't
actually free the underlying memory, since the driver texture keeps
a reference. The buffers will be freed when either the client textures
are freed or a different GLImage is bound.

Original description:
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 kevin. ARC++ video on kevin
Bug: b/73751133
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: Ie5f70db57e2ae1dc906cb8197dc5996651be08db
Reviewed-on: https://chromium-review.googlesource.com/1022373
Commit-Queue: Daniele Castagna <dcastagna@chromium.org>
Reviewed-by: default avatarKristian H. Kristensen <hoegsberg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552446}
parent 24da5fae
......@@ -64,15 +64,6 @@
} while (0)
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
const uint32_t V4L2SliceVideoDecodeAccelerator::supported_input_fourccs_[] = {
......@@ -1521,13 +1512,6 @@ bool V4L2SliceVideoDecodeAccelerator::DestroyOutputs(bool dismiss) {
VLOGF(1) << "eglDestroySyncKHR failed.";
}
if (output_record.gl_image) {
child_task_runner_->PostTask(
FROM_HERE, base::Bind(&DropGLImage, std::move(output_record.gl_image),
bind_image_cb_, output_record.client_texture_id,
device_->GetTextureTarget()));
}
picture_buffers_to_dismiss.push_back(output_record.picture_id);
}
......@@ -1639,7 +1623,6 @@ void V4L2SliceVideoDecodeAccelerator::AssignPictureBuffersTask(
OutputRecord& output_record = output_buffer_map_[i];
DCHECK(!output_record.at_device);
DCHECK(!output_record.at_client);
DCHECK(!output_record.gl_image);
DCHECK_EQ(output_record.egl_sync, EGL_NO_SYNC_KHR);
DCHECK_EQ(output_record.picture_id, -1);
DCHECK(output_record.dmabuf_fds.empty());
......@@ -1723,15 +1706,14 @@ void V4L2SliceVideoDecodeAccelerator::CreateGLImageFor(
true);
decoder_thread_task_runner_->PostTask(
FROM_HERE,
base::Bind(&V4L2SliceVideoDecodeAccelerator::AssignGLImage,
base::Unretained(this), buffer_index, picture_buffer_id,
gl_image, base::Passed(&passed_dmabuf_fds)));
base::BindOnce(&V4L2SliceVideoDecodeAccelerator::AssignDmaBufs,
base::Unretained(this), buffer_index, picture_buffer_id,
base::Passed(&passed_dmabuf_fds)));
}
void V4L2SliceVideoDecodeAccelerator::AssignGLImage(
void V4L2SliceVideoDecodeAccelerator::AssignDmaBufs(
size_t buffer_index,
int32_t picture_buffer_id,
scoped_refptr<gl::GLImage> gl_image,
std::unique_ptr<std::vector<base::ScopedFD>> passed_dmabuf_fds) {
DVLOGF(3) << "index=" << buffer_index;
DCHECK(decoder_thread_task_runner_->BelongsToCurrentThread());
......@@ -1750,12 +1732,10 @@ void V4L2SliceVideoDecodeAccelerator::AssignGLImage(
}
OutputRecord& output_record = output_buffer_map_[buffer_index];
DCHECK(!output_record.gl_image);
DCHECK_EQ(output_record.egl_sync, EGL_NO_SYNC_KHR);
DCHECK(!output_record.at_client);
DCHECK(!output_record.at_device);
output_record.gl_image = gl_image;
if (output_mode_ == Config::OutputMode::IMPORT) {
DCHECK(output_record.dmabuf_fds.empty());
output_record.dmabuf_fds = std::move(*passed_dmabuf_fds);
......@@ -1839,7 +1819,6 @@ void V4L2SliceVideoDecodeAccelerator::ImportBufferForPictureTask(
DCHECK(!iter->at_device);
iter->at_client = false;
if (iter->texture_id != 0) {
iter->gl_image = nullptr;
child_task_runner_->PostTask(
FROM_HERE,
base::Bind(&V4L2SliceVideoDecodeAccelerator::CreateGLImageFor,
......
......@@ -90,7 +90,6 @@ class MEDIA_GPU_EXPORT V4L2SliceVideoDecodeAccelerator
int32_t picture_id;
GLuint client_texture_id;
GLuint texture_id;
scoped_refptr<gl::GLImage> gl_image;
EGLSyncKHR egl_sync;
std::vector<base::ScopedFD> dmabuf_fds;
bool cleared;
......@@ -269,13 +268,12 @@ class MEDIA_GPU_EXPORT V4L2SliceVideoDecodeAccelerator
const gfx::Size& size,
uint32_t fourcc);
// Take the GLImage |gl_image|, created for |picture_buffer_id|, and use it
// Take the dmabuf |passed_dmabuf_fds|, for |picture_buffer_id|, and use it
// for OutputRecord at |buffer_index|. The buffer is backed by
// |passed_dmabuf_fds|, and the OutputRecord takes ownership of them.
void AssignGLImage(
void AssignDmaBufs(
size_t buffer_index,
int32_t picture_buffer_id,
scoped_refptr<gl::GLImage> gl_image,
// 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
// 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