New issue
Advanced search Search tips

Issue 847598 link

Starred by 1 user
Status: Fixed
Owner:
chfremer@chromium.org
Closed: Aug 31
Cc:
guidou@chromium.org
niklase@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

[Video Capture Service] Add API support for legacy Mojo on ChromeOS

Project Member Reported by chfremer@chromium.org, May 29 2018 
The goal is to be able to use the video capture service from a ChromeOS context. ChromeOS currently uses an older version of Mojo, which has certain incompatibilities with the latest version of Mojo used in Chromium.

Workarounds for the incompatibilities exist, and this issue track entry is for adding such workarounds.

1. The video capture service API, has a dependency on the native enum VideoPixelFormat in media_types.mojom. In the ChromeOS context, the native enums cannot be used. The workaround is to switch to Mojo enums. This should be a step in the right direction, since native enums are considered a stopgap for the transition from legacy IPC to Mojo, and are supposed to eventually all be replaced with Mojo enums.

2. The video capture service API uses Mojo type |handle<shared_buffer>| to share frame buffers across process boundaries. There is an incompatibility in the serialization/deserialization of this type across the two Mojo versions. The workaround is to use the Mojo type |handle|, which requires a bit more manual wrapping/unwrapping.
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 12 2018 

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

commit 4610d04fac766c5f47e51a8294e3a48da94c0190
Author: Christian Fremerey <chfremer@chromium.org>
Date: Tue Jun 12 20:09:02 2018

Define enum media.mojom.VideoPixelFormat instead of using native enum

A recent CL https://chromium-review.googlesource.com/c/chromium/src/+/1026795
caused a regression by introducing a new mojo enum VideoCapturePixelType and
typemapping it to media::VideoPixelFormat, see  Bug 839742 .

The motivation for that change was to make the video capture service API,
which had a dependency on the native enum VideoPixelFormat in media_types.mojom,
compatible with an older version of Mojo used on ChromeOS. This older version
does not support native enums.

This CL replaces the bad solution from the CL above with the following:
* Instead of using a native enum VideoPixelFormat in media_types.mojom, declare
  a Mojo enum and move it to a separate file.
* Add typmapping from the Mojo enum to the native enum.
* Have the video capture service API mojoms depend on only the new file
  containing the Mojo enum for VideoPixelFormat. This way we avoid the API
  depending on any file that uses native enums.

Bug:  839742 ,  847598 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Ifcff064b1f0eb95579119916b757dc6512f49a15
Reviewed-on: https://chromium-review.googlesource.com/1050489
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566543}
[modify] https://crrev.com/4610d04fac766c5f47e51a8294e3a48da94c0190/media/capture/mojom/video_capture_types.mojom
[modify] https://crrev.com/4610d04fac766c5f47e51a8294e3a48da94c0190/media/capture/mojom/video_capture_types_mojom_traits.cc
[modify] https://crrev.com/4610d04fac766c5f47e51a8294e3a48da94c0190/media/mojo/interfaces/video_decoder_config_struct_traits.cc
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 12 2018 

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

commit 6ceadf1def1186c0db6a9a47ccbf04054593c4d0
Author: Christian Fremerey <chfremer@chromium.org>
Date: Tue Jun 12 22:15:54 2018

Revert "Define enum media.mojom.VideoPixelFormat instead of using native enum"

This reverts commit 4610d04fac766c5f47e51a8294e3a48da94c0190.

Reason for revert: Forgot to update the CL description. Will reland to avoid a quite confusing/misleading CL description.

Original change's description:
> Define enum media.mojom.VideoPixelFormat instead of using native enum
> 
> A recent CL https://chromium-review.googlesource.com/c/chromium/src/+/1026795
> caused a regression by introducing a new mojo enum VideoCapturePixelType and
> typemapping it to media::VideoPixelFormat, see  Bug 839742 .
> 
> The motivation for that change was to make the video capture service API,
> which had a dependency on the native enum VideoPixelFormat in media_types.mojom,
> compatible with an older version of Mojo used on ChromeOS. This older version
> does not support native enums.
> 
> This CL replaces the bad solution from the CL above with the following:
> * Instead of using a native enum VideoPixelFormat in media_types.mojom, declare
>   a Mojo enum and move it to a separate file.
> * Add typmapping from the Mojo enum to the native enum.
> * Have the video capture service API mojoms depend on only the new file
>   containing the Mojo enum for VideoPixelFormat. This way we avoid the API
>   depending on any file that uses native enums.
> 
> Bug:  839742 ,  847598 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
> Change-Id: Ifcff064b1f0eb95579119916b757dc6512f49a15
> Reviewed-on: https://chromium-review.googlesource.com/1050489
> Commit-Queue: Christian Fremerey <chfremer@chromium.org>
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#566543}

TBR=xhwang@chromium.org,rockot@chromium.org,sandersd@chromium.org,rsesek@chromium.org,chfremer@chromium.org,lasoren@chromium.org

Change-Id: Iab9f2c00a6e5aa8ffd167b4d89f294691b37d456
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  839742 ,  847598 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1097900
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566601}
[modify] https://crrev.com/6ceadf1def1186c0db6a9a47ccbf04054593c4d0/media/capture/mojom/video_capture_types.mojom
[modify] https://crrev.com/6ceadf1def1186c0db6a9a47ccbf04054593c4d0/media/capture/mojom/video_capture_types_mojom_traits.cc
[modify] https://crrev.com/6ceadf1def1186c0db6a9a47ccbf04054593c4d0/media/mojo/interfaces/video_decoder_config_struct_traits.cc
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 3 

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

commit 336a7d28eacbf60bb07cdf961fd451dc588951d2
Author: Christian Fremerey <chfremer@chromium.org>
Date: Tue Jul 03 18:09:21 2018

[Video Capture Service] Add support for raw file descriptors as buffer handles

There is an incompatibility in the serialization of shared memory handles
between the Mojo libraries used in Chromium vs. the older ones used in
ChromiumOS. As a workaround, this CL allows clients of the video capture service
to ask it to use raw file descriptors for sharing shared memory handles instead.
Note, this is only supported on platforms that use file descriptors for
base::SharedMemoryHandle, i.e. Posix platforms.

Design Doc: https://docs.google.com/document/d/1cmtWA8yua5sqjZC0tswVUZwJeQR-KSZBr3Mj1jUagKk/edit?usp=sharing

Bug:  847598 
Change-Id: Ie269231b98149cbbc104ea62c1ff522c665cf187
Reviewed-on: https://chromium-review.googlesource.com/1043646
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572295}
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/content/browser/renderer_host/media/fake_video_capture_device_launcher.cc
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/content/browser/renderer_host/media/in_process_video_capture_device_launcher.cc
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/content/browser/renderer_host/media/in_process_video_capture_device_launcher.h
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/content/browser/renderer_host/media/video_capture_controller_unittest.cc
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/content/renderer/media/video_capture_impl.cc
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/media/capture/mojom/video_capture_types.mojom
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/media/capture/mojom/video_capture_types.typemap
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/media/capture/mojom/video_capture_types_mojom_traits.cc
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/media/capture/mojom/video_capture_types_mojom_traits.h
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/media/capture/video/fake_video_capture_device_unittest.cc
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/media/capture/video/shared_memory_buffer_tracker.cc
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/media/capture/video/shared_memory_buffer_tracker.h
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/media/capture/video/shared_memory_handle_provider.cc
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/media/capture/video/shared_memory_handle_provider.h
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/media/capture/video/video_capture_buffer_pool.h
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/media/capture/video/video_capture_buffer_pool_impl.cc
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/media/capture/video/video_capture_buffer_pool_impl.h
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/media/capture/video/video_capture_buffer_tracker.h
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/media/capture/video/video_capture_device.h
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/media/capture/video/video_capture_device_client.cc
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/media/capture/video/video_capture_device_client.h
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/media/capture/video/video_capture_device_client_unittest.cc
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/media/capture/video_capture_types.cc
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/media/capture/video_capture_types.h
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/media/mojo/interfaces/video_decoder_config_struct_traits.cc
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/services/video_capture/device_media_to_mojo_adapter.cc
[modify] https://crrev.com/336a7d28eacbf60bb07cdf961fd451dc588951d2/services/video_capture/test/fake_device_unittest.cc
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 10 

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

commit 9e85e6764e0180bf0dde0312a0b812451b4cae6b
Author: Christian Fremerey <chfremer@chromium.org>
Date: Tue Jul 10 19:58:06 2018

[Video Capture Service] Add support for raw file descriptors for virtual devices

Follow-up for https://chromium-review.googlesource.com/c/chromium/src/+/1043646.
Here we extend the support for raw file descriptors to the virtual device
functionality.

There is an incompatibility in the serialization of shared memory handles
between the Mojo libraries used in Chromium vs. the older ones used in
ChromiumOS. As a workaround, this CL allows clients of the virtual device API of
the video capture service to ask it to use raw file descriptors for sharing
shared memory handles instead.
Note, this is only supported on platforms that use file descriptors for
base::SharedMemoryHandle, i.e. Posix platforms.

Design Doc: https://docs.google.com/document/d/1cmtWA8yua5sqjZC0tswVUZwJeQR-KSZBr3Mj1jUagKk/edit?usp=sharing

Bug:  847598 
Change-Id: I8a49e5181a622e14ea050cde27ad91ff7feda30d
Reviewed-on: https://chromium-review.googlesource.com/1053210
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573868}
[modify] https://crrev.com/9e85e6764e0180bf0dde0312a0b812451b4cae6b/content/browser/renderer_host/media/service_video_capture_device_launcher_unittest.cc
[modify] https://crrev.com/9e85e6764e0180bf0dde0312a0b812451b4cae6b/content/browser/renderer_host/media/service_video_capture_provider_unittest.cc
[modify] https://crrev.com/9e85e6764e0180bf0dde0312a0b812451b4cae6b/content/browser/webrtc/webrtc_video_capture_service_browsertest.cc
[modify] https://crrev.com/9e85e6764e0180bf0dde0312a0b812451b4cae6b/media/capture/video/fake_video_capture_device_unittest.cc
[modify] https://crrev.com/9e85e6764e0180bf0dde0312a0b812451b4cae6b/media/capture/video/shared_memory_handle_provider.h
[modify] https://crrev.com/9e85e6764e0180bf0dde0312a0b812451b4cae6b/media/capture/video/video_capture_buffer_pool.h
[modify] https://crrev.com/9e85e6764e0180bf0dde0312a0b812451b4cae6b/media/capture/video/video_capture_buffer_pool_impl.cc
[modify] https://crrev.com/9e85e6764e0180bf0dde0312a0b812451b4cae6b/media/capture/video/video_capture_buffer_pool_impl.h
[modify] https://crrev.com/9e85e6764e0180bf0dde0312a0b812451b4cae6b/media/capture/video/video_capture_device.h
[modify] https://crrev.com/9e85e6764e0180bf0dde0312a0b812451b4cae6b/media/capture/video/video_capture_device_client.cc
[modify] https://crrev.com/9e85e6764e0180bf0dde0312a0b812451b4cae6b/media/capture/video/video_capture_device_client.h
[modify] https://crrev.com/9e85e6764e0180bf0dde0312a0b812451b4cae6b/services/video_capture/device_factory_media_to_mojo_adapter.cc
[modify] https://crrev.com/9e85e6764e0180bf0dde0312a0b812451b4cae6b/services/video_capture/device_factory_media_to_mojo_adapter.h
[modify] https://crrev.com/9e85e6764e0180bf0dde0312a0b812451b4cae6b/services/video_capture/public/mojom/device_factory.mojom
[modify] https://crrev.com/9e85e6764e0180bf0dde0312a0b812451b4cae6b/services/video_capture/public/mojom/producer.mojom
[modify] https://crrev.com/9e85e6764e0180bf0dde0312a0b812451b4cae6b/services/video_capture/shared_memory_virtual_device_mojo_adapter.cc
[modify] https://crrev.com/9e85e6764e0180bf0dde0312a0b812451b4cae6b/services/video_capture/shared_memory_virtual_device_mojo_adapter.h
[modify] https://crrev.com/9e85e6764e0180bf0dde0312a0b812451b4cae6b/services/video_capture/test/device_factory_provider_unittest.cc
[modify] https://crrev.com/9e85e6764e0180bf0dde0312a0b812451b4cae6b/services/video_capture/test/mock_producer.cc
[modify] https://crrev.com/9e85e6764e0180bf0dde0312a0b812451b4cae6b/services/video_capture/test/mock_producer.h
[modify] https://crrev.com/9e85e6764e0180bf0dde0312a0b812451b4cae6b/services/video_capture/test/virtual_device_unittest.cc
[modify] https://crrev.com/9e85e6764e0180bf0dde0312a0b812451b4cae6b/services/video_capture/virtual_device_enabled_device_factory.cc
[modify] https://crrev.com/9e85e6764e0180bf0dde0312a0b812451b4cae6b/services/video_capture/virtual_device_enabled_device_factory.h

Comment 5 by chfremer@chromium.org, Aug 31

Status: Fixed (was: Started)
►

Sign in to add a comment