New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 736517 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android , Windows , Chrome , Mac
Pri: 3
Type: Feature

Blocked on:
issue 740794
issue 781560
issue 787470


Participants' hotlists:
Morrallas


Sign in to add a comment

Migrate VideoEncodeAccelerator IPC messages to Mojo

Project Member Reported by mcasas@chromium.org, Jun 23 2017

Issue description

VideoEncodeAccelerator (VEA) messages [1] are interchanged between a Renderer (the "host") and the GPU process. 

Consider migrating from IPC to Mojo (without adding or removing any functionality).

[1] https://cs.chromium.org/chromium/src/media/gpu/ipc/common/media_messages.h?q=media/gpu/ipc/common/media_messages.h+AcceleratedVideoEncoder&dr=C&l=151
 

Comment 1 by mcasas@chromium.org, Jun 23 2017

Cc: sande...@chromium.org emir...@chromium.org wuchengli@chromium.org dalecur...@chromium.org c.pa...@samsung.com posciak@chromium.org
Components: Internals>GPU>Video
Adding GPU owners and some other interested peeps.

Comment 2 by c.pa...@samsung.com, Jun 27 2017

Owner: c.pa...@samsung.com
Status: Assigned (was: Available)
Adding this to my queue.

Comment 3 by mcasas@chromium.org, Jun 27 2017

Early thoughts and notes DD: https://goo.gl/PbTKnt
I have started working on this as well. Shall submit the initial draft (that introduces mojom interfaces for GpuVEAH and GpuVEA) by tomorrow.
#4: c.padhi@ I did start too, have an experimental CL
adding appropriate factories to gpu.mojom and gpu_service.mojom
and new MojoVEAHost and MojoVEA classes.  

If you don't mind terribly, I could continue with this VEA and
you could do the VDA variant? :-) If so,  could you take a look 
at the DD in https:/goo.gl/PbTKnt and produce a similar 
assessment for the VDA? 
Owner: mcasas@chromium.org
Assigning it to me as per offline conversation.
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 7 2017

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

commit cfcca5d50f371e7c5784be5f0ac39acd7664b513
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Fri Jul 07 02:32:59 2017

Rename content::GpuVideoAcceleratorFactories to ...Impl and move to media/gpu folder

This CL cleans up a bit ToT's RendererGpuVideoAcceleratorFactories name
to be just GpuVideoAcceleratorFactoriesImpl -- removing the "Renderer" prefix
which adds nothing since it's in //content/renderer and it implements
media::GpuVideoAcceleratorFactories which is pure virtual.

Also it moves the file to //content/renderer/media/gpu to be together
with the other hardware accelerated video files.

Styled according to clang-format where presubmit complained.

** No new code, just moving things around **

Bug:  736517 
Change-Id: Ic27773d51f63b4c13aee8f49ae8712ac6b7b1092
Reviewed-on: https://chromium-review.googlesource.com/557399
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484804}
[modify] https://crrev.com/cfcca5d50f371e7c5784be5f0ac39acd7664b513/content/renderer/BUILD.gn
[modify] https://crrev.com/cfcca5d50f371e7c5784be5f0ac39acd7664b513/content/renderer/media/gpu/OWNERS
[rename] https://crrev.com/cfcca5d50f371e7c5784be5f0ac39acd7664b513/content/renderer/media/gpu/gpu_video_accelerator_factories_impl.cc
[rename] https://crrev.com/cfcca5d50f371e7c5784be5f0ac39acd7664b513/content/renderer/media/gpu/gpu_video_accelerator_factories_impl.h
[modify] https://crrev.com/cfcca5d50f371e7c5784be5f0ac39acd7664b513/content/renderer/media_recorder/vea_encoder.cc
[modify] https://crrev.com/cfcca5d50f371e7c5784be5f0ac39acd7664b513/content/renderer/media_recorder/video_track_recorder.cc
[modify] https://crrev.com/cfcca5d50f371e7c5784be5f0ac39acd7664b513/content/renderer/render_thread_impl.cc
[modify] https://crrev.com/cfcca5d50f371e7c5784be5f0ac39acd7664b513/content/renderer/render_thread_impl.h

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 10 2017

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

commit 2785ae9624694fa57eab0502ead054e0aea48b49
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Mon Jul 10 05:14:09 2017

VEA mojification: RTCVideoEncoder comments cleanup

This CL proposes a few editorial changes to some VEA-related
files, namely to rtc_video_encoder.h:
- Makes RecordInitEncodeUMA() a file-static function 
- Uses __func__ pervasively in DVLOG() statements to save some
 lines.
In gpu_video_encode_accelerator_host.cc:
- prefers early return
- micro DVLOG cleanup

** No new code, except the early returns **

Bug:  736517 
Change-Id: I2c6cd351a0716810338fa3a2e87e2ef3ca708cbf
Reviewed-on: https://chromium-review.googlesource.com/564743
Reviewed-by: Wu-Cheng Li <wuchengli@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485188}
[modify] https://crrev.com/2785ae9624694fa57eab0502ead054e0aea48b49/content/renderer/media/gpu/rtc_video_encoder.cc
[modify] https://crrev.com/2785ae9624694fa57eab0502ead054e0aea48b49/content/renderer/media/gpu/rtc_video_encoder.h
[modify] https://crrev.com/2785ae9624694fa57eab0502ead054e0aea48b49/media/gpu/ipc/client/gpu_video_encode_accelerator_host.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Jul 11 2017

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

commit daca325647b2bb09810cbcaf956ef45f13aa4eba
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Tue Jul 11 08:05:45 2017

VEA mojification: add factory methods to gpu.mojom and gpu_service.mojom

This CL adds a CreateVideoEncodeAccelerator() methods to the gpu.mojom and
gpu_service.mojom files (renderer-to-browser interface and browser-to-gpu
interfaces, resp.), and wires the new mojom::VideoEncodeAcceleratorRequest
pass through.

This is step 0 of VEA mojification, which is currently IPC-based. More
info can be found in the DD https://goo.gl/PbTKnt, concretely this
CL can be better understood in the context of the first diagram in the
document, also here, for conveniency: https://goo.gl/HkwYvA.

A playground with most of the changes pre-review can be found in 
https://crrev.com/c/558846.

Bug:  736517 
Change-Id: Id1f6d60d487d97ce3f304bead73738b2ba531408
Reviewed-on: https://chromium-review.googlesource.com/558833
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485550}
[modify] https://crrev.com/daca325647b2bb09810cbcaf956ef45f13aa4eba/components/viz/host/server_gpu_memory_buffer_manager_unittest.cc
[modify] https://crrev.com/daca325647b2bb09810cbcaf956ef45f13aa4eba/content/browser/gpu/gpu_client.cc
[modify] https://crrev.com/daca325647b2bb09810cbcaf956ef45f13aa4eba/content/browser/gpu/gpu_client.h
[modify] https://crrev.com/daca325647b2bb09810cbcaf956ef45f13aa4eba/content/renderer/media/gpu/gpu_video_accelerator_factories_impl.cc
[modify] https://crrev.com/daca325647b2bb09810cbcaf956ef45f13aa4eba/content/renderer/media/gpu/gpu_video_accelerator_factories_impl.h
[modify] https://crrev.com/daca325647b2bb09810cbcaf956ef45f13aa4eba/content/renderer/render_thread_impl.cc
[modify] https://crrev.com/daca325647b2bb09810cbcaf956ef45f13aa4eba/media/mojo/interfaces/BUILD.gn
[add] https://crrev.com/daca325647b2bb09810cbcaf956ef45f13aa4eba/media/mojo/interfaces/video_encode_accelerator.mojom
[modify] https://crrev.com/daca325647b2bb09810cbcaf956ef45f13aa4eba/services/ui/gpu/gpu_service.cc
[modify] https://crrev.com/daca325647b2bb09810cbcaf956ef45f13aa4eba/services/ui/gpu/gpu_service.h
[modify] https://crrev.com/daca325647b2bb09810cbcaf956ef45f13aa4eba/services/ui/gpu/interfaces/BUILD.gn
[modify] https://crrev.com/daca325647b2bb09810cbcaf956ef45f13aa4eba/services/ui/gpu/interfaces/gpu_service.mojom
[modify] https://crrev.com/daca325647b2bb09810cbcaf956ef45f13aa4eba/services/ui/public/cpp/gpu/gpu.cc
[modify] https://crrev.com/daca325647b2bb09810cbcaf956ef45f13aa4eba/services/ui/public/cpp/gpu/gpu.h
[modify] https://crrev.com/daca325647b2bb09810cbcaf956ef45f13aa4eba/services/ui/public/cpp/tests/gpu_unittest.cc
[modify] https://crrev.com/daca325647b2bb09810cbcaf956ef45f13aa4eba/services/ui/public/interfaces/BUILD.gn
[modify] https://crrev.com/daca325647b2bb09810cbcaf956ef45f13aa4eba/services/ui/public/interfaces/gpu.mojom
[modify] https://crrev.com/daca325647b2bb09810cbcaf956ef45f13aa4eba/services/ui/ws/gpu_client.cc
[modify] https://crrev.com/daca325647b2bb09810cbcaf956ef45f13aa4eba/services/ui/ws/gpu_client.h

Project Member

Comment 10 by bugdroid1@chromium.org, Jul 12 2017

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

commit 5296b4547240b4ecb4251c468ef833bfa556837a
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Wed Jul 12 02:16:01 2017

VEA mojification: fill in VEA mojom file

This CL fills in the video_encode_accelerator.mojom file, 
replicating what is now in media_messages.h [0] (see also the 
DD at https://goo.gl/PbTKnt) plus a bunch of explanations 
added for good measure.

For reference, the next two CLs in the pipeline implement the 
"service"-side [1] and the "client"-side [2], both with unit 
tests, and there's a  playground w/ all the changes together 
in [3].

[1] https://crrev.com/c/566262
[2] https://crrev.com/c/566145
[3] https://crrev.com/c/558846

Bug:  736517 
Change-Id: I9f42ca5a9f0572a8dd2c02ae1421c6d3649b3d80
Reviewed-on: https://chromium-review.googlesource.com/566769
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485796}
[modify] https://crrev.com/5296b4547240b4ecb4251c468ef833bfa556837a/media/mojo/interfaces/video_encode_accelerator.mojom

Project Member

Comment 11 by bugdroid1@chromium.org, Jul 12 2017

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

commit c493060374532c389091c3c138cf8afb35d92354
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Wed Jul 12 04:05:16 2017

VEA micro cleanup: remove unused member variable

This CL removes the unused member variable |make_context_current_| and
associated file static function MakeDecoderContextCurrent(), because
they are unused.

Bug:  736517 
Change-Id: I5e294cdc00b5703fafed3afb96fbd5e54aa31991
Reviewed-on: https://chromium-review.googlesource.com/567603
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485827}
[modify] https://crrev.com/c493060374532c389091c3c138cf8afb35d92354/media/gpu/ipc/service/gpu_video_encode_accelerator.cc
[modify] https://crrev.com/c493060374532c389091c3c138cf8afb35d92354/media/gpu/ipc/service/gpu_video_encode_accelerator.h

Blockedon: 740794
Project Member

Comment 13 by bugdroid1@chromium.org, Jul 17 2017

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

commit 414c18ea80f303b607ea6f9912048e6033436e4c
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Mon Jul 17 01:51:43 2017

VEA mojification: service-side implementation w/ unittest

This CL lands the service-side implementation of the mojom::VEA
and unit tests for it. It doesn't connect it yet, but the 
final result can be found in https://crrev.com/c/558846.

Bug:  736517 
Change-Id: I6d77da653cfa8e4db9b2691707302fa2ad2d4479
Reviewed-on: https://chromium-review.googlesource.com/566262
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487022}
[modify] https://crrev.com/414c18ea80f303b607ea6f9912048e6033436e4c/media/gpu/BUILD.gn
[modify] https://crrev.com/414c18ea80f303b607ea6f9912048e6033436e4c/media/mojo/BUILD.gn
[modify] https://crrev.com/414c18ea80f303b607ea6f9912048e6033436e4c/media/mojo/services/BUILD.gn
[add] https://crrev.com/414c18ea80f303b607ea6f9912048e6033436e4c/media/mojo/services/mojo_video_encode_accelerator_service.cc
[add] https://crrev.com/414c18ea80f303b607ea6f9912048e6033436e4c/media/mojo/services/mojo_video_encode_accelerator_service.h
[add] https://crrev.com/414c18ea80f303b607ea6f9912048e6033436e4c/media/mojo/services/mojo_video_encode_accelerator_service_unittest.cc
[modify] https://crrev.com/414c18ea80f303b607ea6f9912048e6033436e4c/media/video/fake_video_encode_accelerator.cc
[modify] https://crrev.com/414c18ea80f303b607ea6f9912048e6033436e4c/media/video/fake_video_encode_accelerator.h

Project Member

Comment 14 by bugdroid1@chromium.org, Jul 19 2017

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

commit 15e2e598dd72b4737f3dd6a86f954289277ca426
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Wed Jul 19 01:45:25 2017

VEA mojification: enum typemapping for VideoEncodeAccelerator::Error

This CL adds enum typemapping between the existing
media::VideoEncodeAccelerator::Error and the mojom version of it,
and updates the call sites.

Also removes some superfluous media:: namespace resolutions.

Bug:  736517 
Change-Id: Ie3f41c7b534b657137b6750e7d642fa664cd6ffb
Reviewed-on: https://chromium-review.googlesource.com/574896
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487715}
[modify] https://crrev.com/15e2e598dd72b4737f3dd6a86f954289277ca426/media/mojo/interfaces/typemaps.gni
[modify] https://crrev.com/15e2e598dd72b4737f3dd6a86f954289277ca426/media/mojo/interfaces/video_encode_accelerator.mojom
[add] https://crrev.com/15e2e598dd72b4737f3dd6a86f954289277ca426/media/mojo/interfaces/video_encode_accelerator.typemap
[add] https://crrev.com/15e2e598dd72b4737f3dd6a86f954289277ca426/media/mojo/interfaces/video_encode_accelerator_typemap_traits.cc
[add] https://crrev.com/15e2e598dd72b4737f3dd6a86f954289277ca426/media/mojo/interfaces/video_encode_accelerator_typemap_traits.h
[modify] https://crrev.com/15e2e598dd72b4737f3dd6a86f954289277ca426/media/mojo/services/mojo_video_encode_accelerator_service.cc
[modify] https://crrev.com/15e2e598dd72b4737f3dd6a86f954289277ca426/media/mojo/services/mojo_video_encode_accelerator_service.h
[modify] https://crrev.com/15e2e598dd72b4737f3dd6a86f954289277ca426/media/mojo/services/mojo_video_encode_accelerator_service_unittest.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Jul 20 2017

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

commit 1c955f0530f6fe3f0f5bfdbe071d0c680a615b99
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Thu Jul 20 04:16:33 2017

VEA mojification: client-side implementation w/unittest

This CL lands the client-side implementation of the mojom::VEA
and unit tests for it. It doesn't connect it yet, but the 
final result can be found in https://crrev.com/c/558846.

Bug:  736517 
Change-Id: I24f29c8893c59e1f16560026e0ed94b611b0651a

TBR=tsepez@ for adding [Sync] to the mojom interfaces
(that anyway reflects the current IPC).

Change-Id: I24f29c8893c59e1f16560026e0ed94b611b0651a
Reviewed-on: https://chromium-review.googlesource.com/566145
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488135}
[modify] https://crrev.com/1c955f0530f6fe3f0f5bfdbe071d0c680a615b99/media/mojo/BUILD.gn
[modify] https://crrev.com/1c955f0530f6fe3f0f5bfdbe071d0c680a615b99/media/mojo/clients/BUILD.gn
[add] https://crrev.com/1c955f0530f6fe3f0f5bfdbe071d0c680a615b99/media/mojo/clients/mojo_video_encode_accelerator.cc
[add] https://crrev.com/1c955f0530f6fe3f0f5bfdbe071d0c680a615b99/media/mojo/clients/mojo_video_encode_accelerator.h
[add] https://crrev.com/1c955f0530f6fe3f0f5bfdbe071d0c680a615b99/media/mojo/clients/mojo_video_encode_accelerator_unittest.cc
[modify] https://crrev.com/1c955f0530f6fe3f0f5bfdbe071d0c680a615b99/media/mojo/interfaces/video_encode_accelerator.mojom
[modify] https://crrev.com/1c955f0530f6fe3f0f5bfdbe071d0c680a615b99/media/mojo/services/mojo_video_encode_accelerator_service.cc
[modify] https://crrev.com/1c955f0530f6fe3f0f5bfdbe071d0c680a615b99/media/mojo/services/mojo_video_encode_accelerator_service.h
[modify] https://crrev.com/1c955f0530f6fe3f0f5bfdbe071d0c680a615b99/media/mojo/services/mojo_video_encode_accelerator_service_unittest.cc
[modify] https://crrev.com/1c955f0530f6fe3f0f5bfdbe071d0c680a615b99/media/video/video_encode_accelerator.h

Project Member

Comment 16 by bugdroid1@chromium.org, Jul 26 2017

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

commit f5261d998c8d318bc50e5e61db6729c8a3dec254
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Wed Jul 26 06:31:34 2017

VEA mojification: connect factories and moar testing

This CL connects the VEA client and service creation to the
appropriate GpuVideoAcceleratorFactoriesImpl and GpuService,
respectively.

It also adds an integration unit test that does the end2end
 VEA <--> MojoVEA <--> Mojo <--> VEAService <--> FakeVEA

Bug:  736517 
Change-Id: Ifaebbc86a9aa9cc0c38f678eb54d7e8db4ed00d5
Reviewed-on: https://chromium-review.googlesource.com/577333
Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489565}
[modify] https://crrev.com/f5261d998c8d318bc50e5e61db6729c8a3dec254/content/renderer/media/gpu/gpu_video_accelerator_factories_impl.cc
[modify] https://crrev.com/f5261d998c8d318bc50e5e61db6729c8a3dec254/media/mojo/BUILD.gn
[modify] https://crrev.com/f5261d998c8d318bc50e5e61db6729c8a3dec254/media/mojo/clients/mojo_video_encode_accelerator.cc
[add] https://crrev.com/f5261d998c8d318bc50e5e61db6729c8a3dec254/media/mojo/mojo_video_encode_accelerator_integration_test.cc
[modify] https://crrev.com/f5261d998c8d318bc50e5e61db6729c8a3dec254/media/mojo/services/mojo_video_encode_accelerator_service.cc
[modify] https://crrev.com/f5261d998c8d318bc50e5e61db6729c8a3dec254/media/mojo/services/mojo_video_encode_accelerator_service.h
[modify] https://crrev.com/f5261d998c8d318bc50e5e61db6729c8a3dec254/media/mojo/services/mojo_video_encode_accelerator_service_unittest.cc
[modify] https://crrev.com/f5261d998c8d318bc50e5e61db6729c8a3dec254/services/ui/gpu/BUILD.gn
[modify] https://crrev.com/f5261d998c8d318bc50e5e61db6729c8a3dec254/services/ui/gpu/DEPS
[modify] https://crrev.com/f5261d998c8d318bc50e5e61db6729c8a3dec254/services/ui/gpu/gpu_service.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Jul 27 2017

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

commit 10769c836faa906434580a6b2e28ab6b6264a5d1
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Thu Jul 27 23:52:04 2017

VEA mojification: add a base::Feature flag surfaced to chrome::flags

This CL adds a base::Feature flag to disable Mojo VEA, allowing for fall back 
to the previous, stable IPC mode.

Shows up as chrome://flags/#mojo-video-encode-accelerator.

Bug:  736517 , 749384
Change-Id: I35067dc3d497539c039cf9a55a9ba6491b998a42
Reviewed-on: https://chromium-review.googlesource.com/584696
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Reviewed-by: Satoru Takabayashi <satorux@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490137}
[modify] https://crrev.com/10769c836faa906434580a6b2e28ab6b6264a5d1/chrome/browser/about_flags.cc
[modify] https://crrev.com/10769c836faa906434580a6b2e28ab6b6264a5d1/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/10769c836faa906434580a6b2e28ab6b6264a5d1/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/10769c836faa906434580a6b2e28ab6b6264a5d1/content/public/common/content_features.cc
[modify] https://crrev.com/10769c836faa906434580a6b2e28ab6b6264a5d1/content/public/common/content_features.h
[modify] https://crrev.com/10769c836faa906434580a6b2e28ab6b6264a5d1/content/renderer/media/gpu/gpu_video_accelerator_factories_impl.cc
[modify] https://crrev.com/10769c836faa906434580a6b2e28ab6b6264a5d1/tools/metrics/histograms/enums.xml

Project Member

Comment 18 by bugdroid1@chromium.org, Jul 29 2017

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

commit 7d4647068ee82916538e67452ecffab120312d5e
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Sat Jul 29 14:19:21 2017

VEA mojification: allow for Mojo VEAs to be instantiated several times

On ToT, RenderThreadImpl creates one "remote" mojom::VideoEncodeAcceleratorPtr
on main render thread and gives it to GpuVideoAcceleratorFactoriesImpl [1];
this factory gives it to the first constructed MojoVideoEncodeAccelerator;
this is good for the first time, but after that it'd be gone, the construction
of subsequent MojoVideoEncodeAccelerators will fail and we'll be falling back 
to software and/or the old IPC mechanism.

This CL changes that by adding a mojom VideoEncodeAcceleratorProvider interface,
which is bound along the chain:

 RenderThreadImpl --> gpu.mojom --> GpuClient --> gpu_service.mojom --> GpuService

GpuService finally bind the request to a new MojoVideoEncodeAcceleratorProvider 
(added in this CL as well).

GpuVideoAcceleratorFactoriesImpl uses this bound Provider to create 
MojoVideoEncodeAccelerators every time.

[1] https://cs.chromium.org/chromium/src/content/renderer/render_thread_impl.cc?dr=CSs&sq=package:chromium&l=1463

Bug:  736517 
Change-Id: I3c753f951f120e1d2ffefb50a526fac15de6e881
Reviewed-on: https://chromium-review.googlesource.com/590513
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490658}
[modify] https://crrev.com/7d4647068ee82916538e67452ecffab120312d5e/components/viz/host/server_gpu_memory_buffer_manager_unittest.cc
[modify] https://crrev.com/7d4647068ee82916538e67452ecffab120312d5e/content/browser/gpu/gpu_client.cc
[modify] https://crrev.com/7d4647068ee82916538e67452ecffab120312d5e/content/browser/gpu/gpu_client.h
[modify] https://crrev.com/7d4647068ee82916538e67452ecffab120312d5e/content/renderer/media/gpu/gpu_video_accelerator_factories_impl.cc
[modify] https://crrev.com/7d4647068ee82916538e67452ecffab120312d5e/content/renderer/media/gpu/gpu_video_accelerator_factories_impl.h
[modify] https://crrev.com/7d4647068ee82916538e67452ecffab120312d5e/content/renderer/render_thread_impl.cc
[modify] https://crrev.com/7d4647068ee82916538e67452ecffab120312d5e/media/mojo/interfaces/video_encode_accelerator.mojom
[modify] https://crrev.com/7d4647068ee82916538e67452ecffab120312d5e/media/mojo/services/BUILD.gn
[add] https://crrev.com/7d4647068ee82916538e67452ecffab120312d5e/media/mojo/services/mojo_video_encode_accelerator_provider.cc
[add] https://crrev.com/7d4647068ee82916538e67452ecffab120312d5e/media/mojo/services/mojo_video_encode_accelerator_provider.h
[modify] https://crrev.com/7d4647068ee82916538e67452ecffab120312d5e/services/ui/gpu/gpu_service.cc
[modify] https://crrev.com/7d4647068ee82916538e67452ecffab120312d5e/services/ui/gpu/gpu_service.h
[modify] https://crrev.com/7d4647068ee82916538e67452ecffab120312d5e/services/ui/gpu/interfaces/gpu_service.mojom
[modify] https://crrev.com/7d4647068ee82916538e67452ecffab120312d5e/services/ui/public/cpp/gpu/gpu.cc
[modify] https://crrev.com/7d4647068ee82916538e67452ecffab120312d5e/services/ui/public/cpp/gpu/gpu.h
[modify] https://crrev.com/7d4647068ee82916538e67452ecffab120312d5e/services/ui/public/cpp/tests/gpu_unittest.cc
[modify] https://crrev.com/7d4647068ee82916538e67452ecffab120312d5e/services/ui/public/interfaces/gpu.mojom
[modify] https://crrev.com/7d4647068ee82916538e67452ecffab120312d5e/services/ui/ws/gpu_client.cc
[modify] https://crrev.com/7d4647068ee82916538e67452ecffab120312d5e/services/ui/ws/gpu_client.h

Project Member

Comment 19 by bugdroid1@chromium.org, Sep 5 2017

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

commit b4fa5d7faefbd3733358eaf72737f768c39cdee4
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Tue Sep 05 16:54:45 2017

VEA mojification: enable MojoVEA by default for MacOSX

This CL enables MojoVEA by default for MacOSX platforms; the idea is to
start enabling platforms one by one to facilitate monitoring all the
downstream bots in particular those with actual physical hardware.

Bug:  736517 
Change-Id: I04bbb29173d4fd0eef8bf01e9f6d048f8254bf32
Reviewed-on: https://chromium-review.googlesource.com/650426
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499656}
[modify] https://crrev.com/b4fa5d7faefbd3733358eaf72737f768c39cdee4/content/public/common/content_features.cc

Project Member

Comment 20 by bugdroid1@chromium.org, Sep 7 2017

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

commit 7bbaf406d20d8640c28639ed58c6633fc786f303
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Thu Sep 07 18:36:36 2017

VEA mojification: enable MojoVEA by default for Android

This CL enables MojoVEA by default for Android platforms; the idea is to
start enabling platforms one by one to facilitate monitoring all the
downstream bots in particular those with actual physical hardware;
two days ago MacOSX was enabled and no bot has regressed, so let's
follow up with this.

Bug:  736517 
Change-Id: Ifd00097c5b8156d0fbbe3ac5bc8d77e410b4562e
Reviewed-on: https://chromium-review.googlesource.com/655223
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500338}
[modify] https://crrev.com/7bbaf406d20d8640c28639ed58c6633fc786f303/content/public/common/content_features.cc

Project Member

Comment 21 by bugdroid1@chromium.org, Sep 11 2017

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

commit a35c467dc2648bfc1f76492effc81f08ae8982b8
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Mon Sep 11 16:10:08 2017

VEA mojification: enable MojoVEA by default for Win

This CL enables MojoVEA by default for Win platforms; the idea is to
start enabling platforms one by one to facilitate monitoring all the
downstream bots in particular those with actual physical hardware;
last week MacOSX and Android were enabled and no bot has regressed, so 
let's follow up with this one.

Bug:  736517 
Change-Id: Iecf78e3141b17343499444464a1c6fabe0e261fc
Reviewed-on: https://chromium-review.googlesource.com/660357
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500931}
[modify] https://crrev.com/a35c467dc2648bfc1f76492effc81f08ae8982b8/content/public/common/content_features.cc

Project Member

Comment 22 by bugdroid1@chromium.org, Sep 13 2017

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

commit 8c8ff367ace2dcd583fb2837cf3f1c41668e763a
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Wed Sep 13 16:39:54 2017

VEA mojification: enable MojoVEA by default for CrOs

This CL enables Mojo VEA by default for CrOs, which is the last
platform supporting it that needed to be enabled; no downstream
bots have regressed and the RTC UMA did not show changes, so
let's follow up with this one.

FTR the UMA [1] is one of those I'm monitoring, in particular
the failure rate should not spike.

[1] https://uma.googleplex.com/timeline_v2?sid=0d2de736ea34e1d0330a2d17cfe7e42f

Bug:  736517 
Change-Id: Icc84a2b603032158660ad677c6ce80045452fc10
Reviewed-on: https://chromium-review.googlesource.com/663997
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501661}
[modify] https://crrev.com/8c8ff367ace2dcd583fb2837cf3f1c41668e763a/content/public/common/content_features.cc

Just for my records, #22 :
Commit 8c8ff367... initially landed in 63.0.3215.0


Blockedon: 781560
Blockedon: 787470
I see many chromebooks on 63.0.3239.86 in [1], so it's
safe to assume that it's hitting stable on CrOs (obviously
it has hit stable on all other platforms).


[1] https://cros-omahaproxy.appspot.com/
Project Member

Comment 27 by bugdroid1@chromium.org, Dec 20 2017

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

commit 141e77a5d35749ac52b9837f082baf5e1413fd66
Author: Miguel Casas <mcasas@chromium.org>
Date: Wed Dec 20 01:08:57 2017

VEA mojification: remove VEA use from PepperVideoEncoderHost

This CL removes using GPU accelerated decoding from PepperVEHost
to enable migrating the current IPC mechanism to Mojo (see
crrev.com/c/585512 and the bug).

Pepper UMA shows that there's barely any usage of this API [1].
Video encoding should have the possibility to fall back to
a software implementation, but this is only available for
VP8 and VP9 [2] (this is probably historical: H264/AVC1 software
encoding using //third_party/openh264 was only landed in 2016ish,
after this code was added); Android doesn't even have that
possibility [3] (and there's no openh264 in Android, see
crbug.com/719023). Moreover, Windows and Linux have no hw
encoding at all, Mac only supports h264, Android supports
h264 and VP8 (but might be disabled for certain models for
quality reasons IIRC).

PepperVideoEncoderHost was landed as part of crrev.com/905023005.

[1] https://uma.googleplex.com/timeline_v2?sid=02aca6e1540b804a771ae4194ecc4540
[2] https://cs.chromium.org/chromium/src/content/renderer/pepper/video_encoder_shim.cc?type=cs&sq=package:chromium&l=379
[3] https://cs.chromium.org/chromium/src/content/renderer/pepper/pepper_video_encoder_host.cc?type=cs&q=pepper_video_e&sq=package:chromium&l=304

Bug:  736517 
Change-Id: I3fc43f8e8e6178be39a347cc8630527f500bf7fc
Reviewed-on: https://chromium-review.googlesource.com/832768
Reviewed-by: Bill Budge <bbudge@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525211}
[modify] https://crrev.com/141e77a5d35749ac52b9837f082baf5e1413fd66/content/renderer/pepper/pepper_video_encoder_host.cc
[modify] https://crrev.com/141e77a5d35749ac52b9837f082baf5e1413fd66/content/renderer/pepper/pepper_video_encoder_host.h

Project Member

Comment 28 by bugdroid1@chromium.org, Dec 22 2017

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

commit be4dac84d0a0ae0795db74eb68129076ff6bbcbd
Author: Miguel Casas <mcasas@chromium.org>
Date: Fri Dec 22 03:06:44 2017

VEA mojification: remove IPC VEA in favour of Mojo VEA

This CL removes the VEA-related messages from media_messages.h, the
instantiation of both IPC client and service side and the classes
supporting it, leaving only the mojo IPC in place.

The Mojo VEA has been running since September and any bugs have
been fixed so we can now delete the old code (will delete the
flag in the next CL).

TBR=fsamuel@chromium.org for components/viz/service/gl/gpu_service_impl.cc
-- IMHO the changes are TBRable, being a syntax adaptation.

Bug:  736517 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;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
Change-Id: If6d4ff25610073a15c153d824b5e00d7d11f7c23
Reviewed-on: https://chromium-review.googlesource.com/585512
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525917}
[modify] https://crrev.com/be4dac84d0a0ae0795db74eb68129076ff6bbcbd/components/viz/service/gl/gpu_service_impl.cc
[modify] https://crrev.com/be4dac84d0a0ae0795db74eb68129076ff6bbcbd/content/public/renderer/video_encode_accelerator.cc
[modify] https://crrev.com/be4dac84d0a0ae0795db74eb68129076ff6bbcbd/content/renderer/media/gpu/gpu_video_accelerator_factories_impl.cc
[modify] https://crrev.com/be4dac84d0a0ae0795db74eb68129076ff6bbcbd/content/renderer/media/gpu/rtc_video_encoder_factory.cc
[modify] https://crrev.com/be4dac84d0a0ae0795db74eb68129076ff6bbcbd/media/gpu/ipc/client/BUILD.gn
[delete] https://crrev.com/17aba25861acaedee06ff7f84793038ef2bdbd32/media/gpu/ipc/client/gpu_video_encode_accelerator_host.cc
[delete] https://crrev.com/17aba25861acaedee06ff7f84793038ef2bdbd32/media/gpu/ipc/client/gpu_video_encode_accelerator_host.h
[modify] https://crrev.com/be4dac84d0a0ae0795db74eb68129076ff6bbcbd/media/gpu/ipc/common/media_messages.h
[modify] https://crrev.com/be4dac84d0a0ae0795db74eb68129076ff6bbcbd/media/gpu/ipc/service/BUILD.gn
[delete] https://crrev.com/17aba25861acaedee06ff7f84793038ef2bdbd32/media/gpu/ipc/service/gpu_video_encode_accelerator.cc
[delete] https://crrev.com/17aba25861acaedee06ff7f84793038ef2bdbd32/media/gpu/ipc/service/gpu_video_encode_accelerator.h
[modify] https://crrev.com/be4dac84d0a0ae0795db74eb68129076ff6bbcbd/media/gpu/ipc/service/media_gpu_channel.cc
[modify] https://crrev.com/be4dac84d0a0ae0795db74eb68129076ff6bbcbd/media/gpu/ipc/service/media_gpu_channel_manager.cc

Project Member

Comment 29 by bugdroid1@chromium.org, Dec 22 2017

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

commit 6a78cc2fdc65fb91891e183de63fd73dd8f2de05
Author: Miguel Casas <mcasas@chromium.org>
Date: Fri Dec 22 17:52:41 2017

VEA mojification: Remove chrome flag

This CL removes the content features kMojoVideoEncodeAccelerator
and its surfacing to chrome://flags, because crrev.com/c/585512
removed the old IPC parts in favour of the Mojo path, thus
rendering this flag effectively meaningless.

Bug:  736517 
Change-Id: Id0264d1c7e2382425f609219d7031ec799fa5f9f
Reviewed-on: https://chromium-review.googlesource.com/843123
Commit-Queue: Avi Drissman <avi@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526019}
[modify] https://crrev.com/6a78cc2fdc65fb91891e183de63fd73dd8f2de05/chrome/browser/about_flags.cc
[modify] https://crrev.com/6a78cc2fdc65fb91891e183de63fd73dd8f2de05/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/6a78cc2fdc65fb91891e183de63fd73dd8f2de05/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/6a78cc2fdc65fb91891e183de63fd73dd8f2de05/content/public/common/content_features.cc
[modify] https://crrev.com/6a78cc2fdc65fb91891e183de63fd73dd8f2de05/content/public/common/content_features.h
[modify] https://crrev.com/6a78cc2fdc65fb91891e183de63fd73dd8f2de05/content/renderer/media/gpu/gpu_video_accelerator_factories_impl.cc

Status: Fixed (was: Assigned)

Sign in to add a comment