New issue
Advanced search Search tips

Issue 627658 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 619729



Sign in to add a comment

Create mechanism for sending StreamTexture's Surface to the MediaPlayerRenderer.

Project Member Reported by tguilbert@chromium.org, Jul 12 2016

Issue description

This bug tracks the work behind registering SurfaceTextures in the GPU process for later retrieval by the Browser process, in the context of  crbug.com/619729 .

 
Labels: -Pri-3 Pri-1
Summary: Create mechanism for sending StreamTexture's Surface to the MediaPlayerRenderer. (was: Create registration mechanism for SurfaceTextureFrameProvider.)
For future reference, this is what a pure "register a surface into a dictionary" approach looks like:
https://codereview.chromium.org/2268173002/

While integrating this approach within my prototype, I ran into a race conditions (the MediaPlayerRendererClient was responsible for generating a token, and simultaneously sending the token to a) register the surface in the browser's dictionary via the GPU process and the IChildProcessCallback, and b) to ask the MediaPlayerRenderer to fetch the registered surface).

It was also unclear how to elegantly handle destruction edge cases, in terms of making sure that no surface was leaked.

Instead of registering a surface in the browser process, I have decided to register a callback to accept a surface, which solves both of those problems.

The two CLs are following shortly, and I will add any extra information to this bug if the CLs are not sufficiently clear.
Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 21 2016

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

commit f08485bfd9514a905dda4ca23eca02e4f1f3d72c
Author: tguilbert <tguilbert@chromium.org>
Date: Wed Sep 21 04:11:41 2016

Add ScopedSurfaceRequestManager

WebMediaPlayerAndroid (WMPA) used the EstablishSurfacePeer construct and
the BrowserMediaPlayerManager to send a SurfaceTexture from the GPU
process to a MediaPlayer in the BrowserProcess.

MediaPlayerRender does not use a BrowserMediaPlayerManager, but needs a
way to receive surface textures from the GPU. This change introduces the
ScopedSurfaceRequestManager and the ScopedSurfaceRequestConduit.

The Manager allows the registration of a callback in the Browser
process, which can receive a ScopedJavaSurface. Upon registration,
the manager creates a token that can be used at a later time to
fulfill the request.
The Conduit forwards surface texture to the Manager, using the
issued token to route the ScopedJavaSurface to the right request.
From the GPU process, surfaces are sent via the ChildProcessServices
and the IChildProcessCallback, the same way that EstablishSurfacePeer
currently does. In single process mode, the Manager directly implements
the Conduit interface.

Security recommended 128 bits as the token size. It is represented by
base::Nonce class, which supports serialization accross IPC and Mojo,
and is easy to send as two uint64_t through JNI and AIDL.

The CL that covers the integration within StreamTexture and
MediaPlayerRenderer is tracked by the same bug, and can be
found at https://codereview.chromium.org/2282633002/.

Adding palmer@ as TBR, since he is the only owner for aidl files, but
is unavailable ATM.

TBR=palmer
BUG= 627658 
TEST=Added UTs, manually verified that it worked in the prototype.
CQ_INCLUDE_TRYBOTS=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

Review-Url: https://codereview.chromium.org/2285593002
Cr-Commit-Position: refs/heads/master@{#419971}

[modify] https://crrev.com/f08485bfd9514a905dda4ca23eca02e4f1f3d72c/content/app/android/child_process_service_impl.cc
[modify] https://crrev.com/f08485bfd9514a905dda4ca23eca02e4f1f3d72c/content/browser/BUILD.gn
[modify] https://crrev.com/f08485bfd9514a905dda4ca23eca02e4f1f3d72c/content/browser/android/child_process_launcher_android.cc
[add] https://crrev.com/f08485bfd9514a905dda4ca23eca02e4f1f3d72c/content/browser/android/scoped_surface_request_manager.cc
[add] https://crrev.com/f08485bfd9514a905dda4ca23eca02e4f1f3d72c/content/browser/android/scoped_surface_request_manager.h
[add] https://crrev.com/f08485bfd9514a905dda4ca23eca02e4f1f3d72c/content/browser/android/scoped_surface_request_manager_unittest.cc
[modify] https://crrev.com/f08485bfd9514a905dda4ca23eca02e4f1f3d72c/content/browser/browser_main_loop.cc
[modify] https://crrev.com/f08485bfd9514a905dda4ca23eca02e4f1f3d72c/content/public/android/java/src/org/chromium/content/app/ChildProcessServiceImpl.java
[modify] https://crrev.com/f08485bfd9514a905dda4ca23eca02e4f1f3d72c/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java
[modify] https://crrev.com/f08485bfd9514a905dda4ca23eca02e4f1f3d72c/content/public/android/java/src/org/chromium/content/common/IChildProcessCallback.aidl
[modify] https://crrev.com/f08485bfd9514a905dda4ca23eca02e4f1f3d72c/content/test/BUILD.gn
[modify] https://crrev.com/f08485bfd9514a905dda4ca23eca02e4f1f3d72c/gpu/ipc/common/BUILD.gn
[add] https://crrev.com/f08485bfd9514a905dda4ca23eca02e4f1f3d72c/gpu/ipc/common/android/scoped_surface_request_conduit.cc
[add] https://crrev.com/f08485bfd9514a905dda4ca23eca02e4f1f3d72c/gpu/ipc/common/android/scoped_surface_request_conduit.h

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 6 2016

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

commit ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e
Author: tguilbert <tguilbert@chromium.org>
Date: Thu Oct 06 02:11:12 2016

Plumb StreamTexture to MediaPlayerRenderer

The ScopedSurfaceRequestManager and ScopedSurfaceRequestConduit
allow classes to register a ScopedJavaSurface request in the browser
process, and to send SurfaceTexture from the GPU process to the
browser process to fulfill those requests.

This change adds the necessary plumbing to send a StreamTexture to
a MediaPlayerRenderer living in the browser, using the
manager/conduit pair.
N.B: In order to prevent the MojoRendererService from taking a
dependency on the MediaPlayerRenderer, the MojoRendererService
initiates surface requests via a callback (given at construction time).

BUG= 627658 
TEST=successfully exercised the new code path in a working prototype.
CQ_INCLUDE_TRYBOTS=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

Review-Url: https://codereview.chromium.org/2282633002
Cr-Commit-Position: refs/heads/master@{#423393}

[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/content/browser/media/android/media_player_renderer.cc
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/content/browser/media/android/media_player_renderer.h
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/content/renderer/media/android/media_player_renderer_client.cc
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/content/renderer/media/android/media_player_renderer_client.h
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/content/renderer/media/android/stream_texture_factory.cc
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/content/renderer/media/android/stream_texture_factory.h
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/content/renderer/media/android/stream_texture_wrapper_impl.cc
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/content/renderer/media/android/stream_texture_wrapper_impl.h
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/gpu/ipc/common/gpu_messages.h
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/gpu/ipc/service/stream_texture_android.cc
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/gpu/ipc/service/stream_texture_android.h
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/media/base/android/stream_texture_wrapper.h
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/media/mojo/clients/mojo_renderer.cc
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/media/mojo/clients/mojo_renderer.h
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/media/mojo/clients/mojo_renderer_unittest.cc
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/media/mojo/interfaces/renderer.mojom
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/media/mojo/services/mojo_renderer_service.cc
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/media/mojo/services/mojo_renderer_service.h
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/media/mojo/services/service_factory_impl.cc

Status: Fixed (was: Started)
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 27 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e

commit ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e
Author: tguilbert <tguilbert@chromium.org>
Date: Thu Oct 06 02:11:12 2016

Plumb StreamTexture to MediaPlayerRenderer

The ScopedSurfaceRequestManager and ScopedSurfaceRequestConduit
allow classes to register a ScopedJavaSurface request in the browser
process, and to send SurfaceTexture from the GPU process to the
browser process to fulfill those requests.

This change adds the necessary plumbing to send a StreamTexture to
a MediaPlayerRenderer living in the browser, using the
manager/conduit pair.
N.B: In order to prevent the MojoRendererService from taking a
dependency on the MediaPlayerRenderer, the MojoRendererService
initiates surface requests via a callback (given at construction time).

BUG= 627658 
TEST=successfully exercised the new code path in a working prototype.
CQ_INCLUDE_TRYBOTS=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

Review-Url: https://codereview.chromium.org/2282633002
Cr-Commit-Position: refs/heads/master@{#423393}

[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/content/browser/media/android/media_player_renderer.cc
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/content/browser/media/android/media_player_renderer.h
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/content/renderer/media/android/media_player_renderer_client.cc
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/content/renderer/media/android/media_player_renderer_client.h
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/content/renderer/media/android/stream_texture_factory.cc
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/content/renderer/media/android/stream_texture_factory.h
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/content/renderer/media/android/stream_texture_wrapper_impl.cc
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/content/renderer/media/android/stream_texture_wrapper_impl.h
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/gpu/ipc/common/gpu_messages.h
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/gpu/ipc/service/stream_texture_android.cc
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/gpu/ipc/service/stream_texture_android.h
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/media/base/android/stream_texture_wrapper.h
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/media/mojo/clients/mojo_renderer.cc
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/media/mojo/clients/mojo_renderer.h
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/media/mojo/clients/mojo_renderer_unittest.cc
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/media/mojo/interfaces/renderer.mojom
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/media/mojo/services/mojo_renderer_service.cc
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/media/mojo/services/mojo_renderer_service.h
[modify] https://crrev.com/ecb3febf6e08a15ceb4b9fa0af86e0a1591f315e/media/mojo/services/service_factory_impl.cc

Comment 9 by dimu@google.com, Nov 4 2016

Labels: -merge-merged-2840
[Automated comment] removing mislabelled merge-merged-2840

Sign in to add a comment