New issue
Advanced search Search tips

Issue 843117 link

Starred by 2 users

Issue metadata

Status: Untriaged
Owner: ----
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 888829
issue 797470

Blocking:
issue 795291



Sign in to add a comment

Convert VideoCapturePool to new shared memory API

Project Member Reported by mattcary@chromium.org, May 15 2018

Issue description

VideoCaptureBufferPool in media/capture/video/video_capture_buffer_pool.h maintains a pool of shared memory and passes it out. It appears to have a model that any pool can be shared read- or write-only.

If there are only two processes involved, a WritableSharedMemoryRegion may be the right new API to use.
 
Blockedon: 797470
It appears the VideoCapturePool is related to the viz video capture pipeline (see eg crrev.com/c/647411). Waiting for resolution on the viz Writable vs Unsafe memory region decision before proceeding (see blocking  crbug.com/797470 ).

Comment 2 by m...@chromium.org, May 16 2018

Cc: chfremer@chromium.org
Cc: m...@chromium.org
@mui---

How does this relate to the viz video capture pipeline, if at all?  Thx

Comment 4 by m...@chromium.org, May 19 2018

viz::FrameSinkVideoCapturerImpl generates video frames in the VIZ process. A consumer in the browser process, content::FrameSinkVideoCaptureDevice receives the frames and then injects them into the video stack (the pipeline shared with all video recording, such as webcam input; to get video frames dispatched to render processes).

So, VideoCapturePool is part of the shared video stack infrastructure. I believe this pool only has the "writable" hack because of the VIZ frame sink capturer. So, once the upstream crbug is resolved, it should be fairly simple to just pass ReadOnlySharedMemory throughout the video capture stack (and mojo APIs).
Makes sense, thanks!
Owner: m...@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6629d45b3851a580b72b28bc1036475e3a4516d6

commit 6629d45b3851a580b72b28bc1036475e3a4516d6
Author: Yuri Wiitala <miu@chromium.org>
Date: Tue Aug 07 21:46:16 2018

Add base::ReadOnlySharedMemoryRegion to media::mojom::VideoBufferHandle

This is the first in a series of changes to migrate the video capture
stack over to the new Chromium shmem APIs. This change adds the use of
base::ReadOnlySharedMemoryRegion as an alternative to mojo shared_buffer
handles.

Bug:  797470 ,843117
Change-Id: Ifa51a4984f48376a5da4b437618d301669f1ae1e
Reviewed-on: https://chromium-review.googlesource.com/1164637
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581347}
[modify] https://crrev.com/6629d45b3851a580b72b28bc1036475e3a4516d6/content/browser/renderer_host/media/video_capture_controller.cc
[modify] https://crrev.com/6629d45b3851a580b72b28bc1036475e3a4516d6/content/renderer/media/video_capture_impl.cc
[modify] https://crrev.com/6629d45b3851a580b72b28bc1036475e3a4516d6/content/renderer/media/video_capture_impl_unittest.cc
[modify] https://crrev.com/6629d45b3851a580b72b28bc1036475e3a4516d6/media/capture/mojom/video_capture_types.mojom

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5

commit 4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5
Author: Yuri Wiitala <miu@chromium.org>
Date: Wed Aug 29 06:09:35 2018

Migrate viz::FrameSinkVideoCapturer to use new read-only shmem API.

Now that overlay rendering takes place in VIZ, there is no need to pass
read-write handles across processes. This change migrates to the new
base::ReadOnlySharedMemoryRegion API. It is also a prerequisite to
migrating the rest of the video capture stack to the new read-only API.

Bug:  797470 ,843117
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I87e16de42aa00e1fa0edba55a9cbf013fc345440
Reviewed-on: https://chromium-review.googlesource.com/1166248
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Reviewed-by: Xiangjun Zhang <xjz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587022}
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/chrome/browser/devtools/devtools_eye_dropper.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/chrome/browser/devtools/devtools_eye_dropper.h
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/components/mirroring/service/fake_video_capture_host.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/components/mirroring/service/video_capture_client.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/components/mirroring/service/video_capture_client.h
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/components/mirroring/service/video_capture_client_unittest.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/components/viz/host/client_frame_sink_video_capturer.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/components/viz/host/client_frame_sink_video_capturer.h
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/components/viz/service/frame_sinks/video_capture/frame_sink_video_capturer_impl.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/components/viz/service/frame_sinks/video_capture/frame_sink_video_capturer_impl_unittest.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/components/viz/service/frame_sinks/video_capture/interprocess_frame_pool.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/components/viz/service/frame_sinks/video_capture/interprocess_frame_pool.h
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/components/viz/service/frame_sinks/video_capture/interprocess_frame_pool_unittest.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/content/browser/devtools/devtools_video_consumer.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/content/browser/devtools/devtools_video_consumer.h
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/content/browser/devtools/devtools_video_consumer_unittest.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/content/browser/media/capture/fake_video_capture_stack.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/content/browser/media/capture/frame_sink_video_capture_device.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/content/browser/media/capture/frame_sink_video_capture_device.h
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/content/browser/media/capture/frame_sink_video_capture_device_unittest.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/content/browser/media/capture/lame_window_capturer_chromeos.cc
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/content/browser/media/capture/lame_window_capturer_chromeos.h
[modify] https://crrev.com/4a74fb0fdb1fd882d4cc04cc4b8a6d339472e7d5/services/viz/privileged/interfaces/compositing/frame_sink_video_capture.mojom

Project Member

Comment 9 by bugdroid1@chromium.org, Sep 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e73efe3c90c127e11cb5119cd400a40f1d777836

commit e73efe3c90c127e11cb5119cd400a40f1d777836
Author: Xiangjun Zhang <xjz@chromium.org>
Date: Thu Sep 06 19:54:42 2018

Mirroring Service: Support shared buffer handles for video capturing.

VideoCaptureClient was recently changed to only support the new
base::ReadOnlySharedMemoryRegion being passed to the client. However,
for desktop capturing, the read-write shared buffer handles are still
passed by media::VideoCaptureDeviceClient. This CL temporally added the
support of shared buffer handles back before the
media::VideoCaptureDeviceClient is migrated to the new read-only API.

Bug:  734672 ,843117
Change-Id: Ib3f6d58aa2541abfb29f1df014f1a01cfef50ef0
Reviewed-on: https://chromium-review.googlesource.com/1204721
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Commit-Queue: Xiangjun Zhang <xjz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589252}
[modify] https://crrev.com/e73efe3c90c127e11cb5119cd400a40f1d777836/components/mirroring/service/video_capture_client.cc
[modify] https://crrev.com/e73efe3c90c127e11cb5119cd400a40f1d777836/components/mirroring/service/video_capture_client.h
[modify] https://crrev.com/e73efe3c90c127e11cb5119cd400a40f1d777836/components/mirroring/service/video_capture_client_unittest.cc

Blockedon: 888829
Owner: ----
Status: Available (was: Started)
Marking as Available, so others may feel free to pick this up. Also, see dependent bug 888829.
Status: Untriaged (was: Available)
Available, but no owner or component? Please find a component, as no one will ever find this without one.

Sign in to add a comment