Commit 671cdfa0 authored by Bret Sepulveda's avatar Bret Sepulveda Committed by Commit Bot

Revert "[VideoCapture] Handle GPU context lost for the zero-copy path"

This reverts commit 27c54e91.

Reason for revert: Sorry, looks like VideoCaptureImplTest.BufferReceived_GpuMemoryBufferHandle on Linux MSan Tests is still failing, see latest build: https://ci.chromium.org/p/chromium/builders/ci/Linux%20MSan%20Tests/25945

Original change's description:
> [VideoCapture] Handle GPU context lost for the zero-copy path
>
> When the render thread encounters GPU context lost, we need to re-create
> the SharedImage resources to recover the video capture stream.
>
> This is a reland of crrev.com/c/2455328 with a fix to address the MSAN
> test failure
>
> Bug: 1131998, b:169429427
> Test: Manually with chrome://gpuclean on soraka
>
> TBR=jcliang@chromium.org,ccameron@chromium.org,kbr@chromium.org,dcastagna@chromium.org,guidou@chromium.org,ccameron@google.com
>
> Change-Id: I3d00abaea04ccf95a795e116f5fc7ff5c7d7f43a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2494406
> Reviewed-by: Ricky Liang <jcliang@chromium.org>
> Commit-Queue: Ricky Liang <jcliang@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#820175}

TBR=jcliang@chromium.org,ccameron@chromium.org,kbr@chromium.org,dcastagna@chromium.org,guidou@chromium.org,ccameron@google.com

Change-Id: I5b13e249552304881763302cef3fd2334e998452
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1131998
Bug: b:169429427
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2494934Reviewed-by: default avatarBret Sepulveda <bsep@chromium.org>
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#820212}
parent e3a6ce30
...@@ -101,10 +101,8 @@ base::OnceClosure WebVideoCaptureImplManager::UseDevice( ...@@ -101,10 +101,8 @@ base::OnceClosure WebVideoCaptureImplManager::UseDevice(
it = devices_.end() - 1; it = devices_.end() - 1;
it->session_id = id; it->session_id = id;
it->impl = CreateVideoCaptureImplForTesting(id); it->impl = CreateVideoCaptureImplForTesting(id);
if (!it->impl) { if (!it->impl)
it->impl = it->impl.reset(new VideoCaptureImpl(id));
std::make_unique<VideoCaptureImpl>(id, render_main_task_runner_);
}
} }
++it->client_count; ++it->client_count;
......
...@@ -51,7 +51,7 @@ class MockVideoCaptureImpl : public VideoCaptureImpl, ...@@ -51,7 +51,7 @@ class MockVideoCaptureImpl : public VideoCaptureImpl,
MockVideoCaptureImpl(const media::VideoCaptureSessionId& session_id, MockVideoCaptureImpl(const media::VideoCaptureSessionId& session_id,
PauseResumeCallback* pause_callback, PauseResumeCallback* pause_callback,
base::OnceClosure destruct_callback) base::OnceClosure destruct_callback)
: VideoCaptureImpl(session_id, base::ThreadTaskRunnerHandle::Get()), : VideoCaptureImpl(session_id),
pause_callback_(pause_callback), pause_callback_(pause_callback),
destruct_callback_(std::move(destruct_callback)) {} destruct_callback_(std::move(destruct_callback)) {}
......
...@@ -66,9 +66,11 @@ struct VideoCaptureImpl::BufferContext ...@@ -66,9 +66,11 @@ struct VideoCaptureImpl::BufferContext
: public base::RefCountedThreadSafe<BufferContext> { : public base::RefCountedThreadSafe<BufferContext> {
public: public:
BufferContext(media::mojom::blink::VideoBufferHandlePtr buffer_handle, BufferContext(media::mojom::blink::VideoBufferHandlePtr buffer_handle,
media::GpuVideoAcceleratorFactories* gpu_factories,
scoped_refptr<base::SingleThreadTaskRunner> media_task_runner) scoped_refptr<base::SingleThreadTaskRunner> media_task_runner)
: buffer_type_(buffer_handle->which()), : buffer_type_(buffer_handle->which()),
media_task_runner_(media_task_runner) { gpu_factories_(gpu_factories),
media_task_runner_(std::move(media_task_runner)) {
switch (buffer_type_) { switch (buffer_type_) {
case VideoFrameBufferHandleType::SHARED_BUFFER_HANDLE: case VideoFrameBufferHandleType::SHARED_BUFFER_HANDLE:
InitializeFromSharedMemory( InitializeFromSharedMemory(
...@@ -89,6 +91,7 @@ struct VideoCaptureImpl::BufferContext ...@@ -89,6 +91,7 @@ struct VideoCaptureImpl::BufferContext
// On macOS, an IOSurfaces passed as a GpuMemoryBufferHandle can be // On macOS, an IOSurfaces passed as a GpuMemoryBufferHandle can be
// used by both hardware and software paths. // used by both hardware and software paths.
// https://crbug.com/1125879 // https://crbug.com/1125879
CHECK(gpu_factories_);
CHECK(media_task_runner_); CHECK(media_task_runner_);
#endif #endif
InitializeFromGpuMemoryBufferHandle( InitializeFromGpuMemoryBufferHandle(
...@@ -120,31 +123,20 @@ struct VideoCaptureImpl::BufferContext ...@@ -120,31 +123,20 @@ struct VideoCaptureImpl::BufferContext
// the VideoFrame can access the data either through mailboxes (e.g. display) // the VideoFrame can access the data either through mailboxes (e.g. display)
// or through the DMA-buf FDs (e.g. video encoder). // or through the DMA-buf FDs (e.g. video encoder).
static void BindBufferToTextureOnMediaThread( static void BindBufferToTextureOnMediaThread(
media::GpuVideoAcceleratorFactories* gpu_factories,
scoped_refptr<BufferContext> buffer_context, scoped_refptr<BufferContext> buffer_context,
media::mojom::blink::VideoFrameInfoPtr info, media::mojom::blink::VideoFrameInfoPtr info,
std::unique_ptr<gfx::GpuMemoryBuffer> gpu_memory_buffer, std::unique_ptr<gfx::GpuMemoryBuffer> gpu_memory_buffer,
scoped_refptr<media::VideoFrame> frame, scoped_refptr<media::VideoFrame> frame,
base::OnceCallback<void(media::mojom::blink::VideoFrameInfoPtr, base::OnceCallback<void(media::mojom::blink::VideoFrameInfoPtr,
scoped_refptr<media::VideoFrame>, scoped_refptr<media::VideoFrame>,
scoped_refptr<BufferContext>)> on_texture_bound, scoped_refptr<BufferContext>)> on_texture_bound) {
base::OnceCallback<void()> on_gpu_context_lost) {
DCHECK(gpu_factories);
DCHECK(buffer_context->media_task_runner_->BelongsToCurrentThread()); DCHECK(buffer_context->media_task_runner_->BelongsToCurrentThread());
DCHECK(buffer_context->gpu_factories_);
DCHECK_EQ(info->pixel_format, media::PIXEL_FORMAT_NV12); DCHECK_EQ(info->pixel_format, media::PIXEL_FORMAT_NV12);
bool should_recreate_shared_image = false;
if (gpu_factories != buffer_context->gpu_factories_) {
DVLOG(1) << "GPU context changed; re-creating SharedImage objects";
buffer_context->gpu_factories_ = gpu_factories;
should_recreate_shared_image = true;
}
// Create GPU texture and bind GpuMemoryBuffer to the texture. // Create GPU texture and bind GpuMemoryBuffer to the texture.
auto* sii = buffer_context->gpu_factories_->SharedImageInterface(); auto* sii = buffer_context->gpu_factories_->SharedImageInterface();
if (!sii) { if (!sii) {
DVLOG(1) << "GPU context lost";
std::move(on_gpu_context_lost).Run();
std::move(on_texture_bound) std::move(on_texture_bound)
.Run(std::move(info), std::move(frame), std::move(buffer_context)); .Run(std::move(info), std::move(frame), std::move(buffer_context));
return; return;
...@@ -158,8 +150,7 @@ struct VideoCaptureImpl::BufferContext ...@@ -158,8 +150,7 @@ struct VideoCaptureImpl::BufferContext
unsigned texture_target = unsigned texture_target =
buffer_context->gpu_factories_->ImageTextureTarget( buffer_context->gpu_factories_->ImageTextureTarget(
gpu_memory_buffer->GetFormat()); gpu_memory_buffer->GetFormat());
if (should_recreate_shared_image || if (buffer_context->gmb_resources_->mailbox.IsZero()) {
buffer_context->gmb_resources_->mailbox.IsZero()) {
uint32_t usage = uint32_t usage =
gpu::SHARED_IMAGE_USAGE_GLES2 | gpu::SHARED_IMAGE_USAGE_RASTER | gpu::SHARED_IMAGE_USAGE_GLES2 | gpu::SHARED_IMAGE_USAGE_RASTER |
gpu::SHARED_IMAGE_USAGE_DISPLAY | gpu::SHARED_IMAGE_USAGE_SCANOUT | gpu::SHARED_IMAGE_USAGE_DISPLAY | gpu::SHARED_IMAGE_USAGE_SCANOUT |
...@@ -309,17 +300,13 @@ struct VideoCaptureImpl::ClientInfo { ...@@ -309,17 +300,13 @@ struct VideoCaptureImpl::ClientInfo {
VideoCaptureDeliverFrameCB deliver_frame_cb; VideoCaptureDeliverFrameCB deliver_frame_cb;
}; };
VideoCaptureImpl::VideoCaptureImpl( VideoCaptureImpl::VideoCaptureImpl(media::VideoCaptureSessionId session_id)
media::VideoCaptureSessionId session_id,
scoped_refptr<base::SingleThreadTaskRunner> main_task_runner)
: device_id_(session_id), : device_id_(session_id),
session_id_(session_id), session_id_(session_id),
video_capture_host_for_testing_(nullptr), video_capture_host_for_testing_(nullptr),
state_(blink::VIDEO_CAPTURE_STATE_STOPPED), state_(blink::VIDEO_CAPTURE_STATE_STOPPED),
main_task_runner_(std::move(main_task_runner)),
gpu_memory_buffer_support_(new gpu::GpuMemoryBufferSupport()) { gpu_memory_buffer_support_(new gpu::GpuMemoryBufferSupport()) {
CHECK(!session_id.is_empty()); CHECK(!session_id.is_empty());
DCHECK(main_task_runner_->BelongsToCurrentThread());
DETACH_FROM_THREAD(io_thread_checker_); DETACH_FROM_THREAD(io_thread_checker_);
Platform::Current()->GetBrowserInterfaceBroker()->GetInterface( Platform::Current()->GetBrowserInterfaceBroker()->GetInterface(
...@@ -331,25 +318,6 @@ VideoCaptureImpl::VideoCaptureImpl( ...@@ -331,25 +318,6 @@ VideoCaptureImpl::VideoCaptureImpl(
} }
} }
void VideoCaptureImpl::OnGpuContextLost(
base::WeakPtr<VideoCaptureImpl> video_capture_impl) {
// Called on the main task runner.
auto* gpu_factories = Platform::Current()->GetGpuFactories();
Platform::Current()->GetIOTaskRunner()->PostTask(
FROM_HERE,
base::BindOnce(&VideoCaptureImpl::SetGpuFactoriesHandleOnIOTaskRunner,
video_capture_impl, gpu_factories));
}
void VideoCaptureImpl::SetGpuFactoriesHandleOnIOTaskRunner(
media::GpuVideoAcceleratorFactories* gpu_factories) {
DCHECK_CALLED_ON_VALID_THREAD(io_thread_checker_);
if (gpu_factories != gpu_factories_) {
LOG(ERROR) << "GPU factories handle changed; assuming GPU context lost";
gpu_factories_ = gpu_factories;
}
}
VideoCaptureImpl::~VideoCaptureImpl() { VideoCaptureImpl::~VideoCaptureImpl() {
DCHECK_CALLED_ON_VALID_THREAD(io_thread_checker_); DCHECK_CALLED_ON_VALID_THREAD(io_thread_checker_);
if ((state_ == VIDEO_CAPTURE_STATE_STARTING || if ((state_ == VIDEO_CAPTURE_STATE_STARTING ||
...@@ -537,8 +505,9 @@ void VideoCaptureImpl::OnNewBuffer( ...@@ -537,8 +505,9 @@ void VideoCaptureImpl::OnNewBuffer(
const bool inserted = const bool inserted =
client_buffers_ client_buffers_
.emplace(buffer_id, new BufferContext(std::move(buffer_handle), .emplace(buffer_id,
media_task_runner_)) new BufferContext(std::move(buffer_handle), gpu_factories_,
media_task_runner_))
.second; .second;
DCHECK(inserted); DCHECK(inserted);
} }
...@@ -703,15 +672,11 @@ void VideoCaptureImpl::OnBufferReady( ...@@ -703,15 +672,11 @@ void VideoCaptureImpl::OnBufferReady(
media_task_runner_->PostTask( media_task_runner_->PostTask(
FROM_HERE, FROM_HERE,
base::BindOnce( base::BindOnce(
&BufferContext::BindBufferToTextureOnMediaThread, gpu_factories_, &BufferContext::BindBufferToTextureOnMediaThread,
std::move(buffer_context), std::move(info), std::move(gmb), frame, std::move(buffer_context), std::move(info), std::move(gmb), frame,
media::BindToCurrentLoop(base::BindOnce( media::BindToCurrentLoop(base::BindOnce(
&VideoCaptureImpl::OnVideoFrameReady, &VideoCaptureImpl::OnVideoFrameReady,
weak_factory_.GetWeakPtr(), buffer_id, reference_time)), weak_factory_.GetWeakPtr(), buffer_id, reference_time))));
media::BindToLoop(
main_task_runner_,
base::BindOnce(&VideoCaptureImpl::OnGpuContextLost,
weak_factory_.GetWeakPtr()))));
return; return;
} }
} }
......
...@@ -40,9 +40,7 @@ extern const PLATFORM_EXPORT base::Feature kTimeoutHangingVideoCaptureStarts; ...@@ -40,9 +40,7 @@ extern const PLATFORM_EXPORT base::Feature kTimeoutHangingVideoCaptureStarts;
class PLATFORM_EXPORT VideoCaptureImpl class PLATFORM_EXPORT VideoCaptureImpl
: public media::mojom::blink::VideoCaptureObserver { : public media::mojom::blink::VideoCaptureObserver {
public: public:
VideoCaptureImpl( explicit VideoCaptureImpl(media::VideoCaptureSessionId session_id);
media::VideoCaptureSessionId session_id,
scoped_refptr<base::SingleThreadTaskRunner> main_task_runner);
~VideoCaptureImpl() override; ~VideoCaptureImpl() override;
// Stop/resume delivering video frames to clients, based on flag |suspend|. // Stop/resume delivering video frames to clients, based on flag |suspend|.
...@@ -152,15 +150,6 @@ class PLATFORM_EXPORT VideoCaptureImpl ...@@ -152,15 +150,6 @@ class PLATFORM_EXPORT VideoCaptureImpl
void OnStartTimedout(); void OnStartTimedout();
// Callback for when GPU context lost is detected. The method fetches the new
// GPU factories handle on |main_task_runner_| and sets |gpu_factories_| to
// the new handle.
static void OnGpuContextLost(
base::WeakPtr<VideoCaptureImpl> video_capture_impl);
void SetGpuFactoriesHandleOnIOTaskRunner(
media::GpuVideoAcceleratorFactories* gpu_factories);
// |device_id_| and |session_id_| are different concepts, but we reuse the // |device_id_| and |session_id_| are different concepts, but we reuse the
// same numerical value, passed on construction. // same numerical value, passed on construction.
const base::UnguessableToken device_id_; const base::UnguessableToken device_id_;
...@@ -195,9 +184,8 @@ class PLATFORM_EXPORT VideoCaptureImpl ...@@ -195,9 +184,8 @@ class PLATFORM_EXPORT VideoCaptureImpl
int num_first_frame_logs_ = 0; int num_first_frame_logs_ = 0;
// Methods of |gpu_factories_| need to run on |media_task_runner_|. // Methods of |gpu_factories_| need to run on |media_task_runner_|.
media::GpuVideoAcceleratorFactories* gpu_factories_ = nullptr; media::GpuVideoAcceleratorFactories* gpu_factories_;
scoped_refptr<base::SingleThreadTaskRunner> media_task_runner_; scoped_refptr<base::SingleThreadTaskRunner> media_task_runner_;
scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
std::unique_ptr<gpu::GpuMemoryBufferSupport> gpu_memory_buffer_support_; std::unique_ptr<gpu::GpuMemoryBufferSupport> gpu_memory_buffer_support_;
......
...@@ -121,9 +121,7 @@ class MockMojoVideoCaptureHost : public media::mojom::blink::VideoCaptureHost { ...@@ -121,9 +121,7 @@ class MockMojoVideoCaptureHost : public media::mojom::blink::VideoCaptureHost {
class VideoCaptureImplTest : public ::testing::Test { class VideoCaptureImplTest : public ::testing::Test {
public: public:
VideoCaptureImplTest() VideoCaptureImplTest()
: video_capture_impl_( : video_capture_impl_(new VideoCaptureImpl(session_id_)) {
new VideoCaptureImpl(session_id_,
base::ThreadTaskRunnerHandle::Get())) {
params_small_.requested_format = media::VideoCaptureFormat( params_small_.requested_format = media::VideoCaptureFormat(
gfx::Size(176, 144), 30, media::PIXEL_FORMAT_I420); gfx::Size(176, 144), 30, media::PIXEL_FORMAT_I420);
params_large_.requested_format = media::VideoCaptureFormat( params_large_.requested_format = media::VideoCaptureFormat(
......
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