From checking an H264 loopback on Win, I found that everything runs on Chrome IO thread as created[0] and doesn't have a hold up. So TryToSetupEncodeOnSeparateThread() that was responsible for deploying tasks on IO thread can be deprecated safely in ToT.

mcasas@ has there been any issues regarding the use of IO thread, i.e. Initialize() call can take some time and this blocking can affect other mojo messages. Is there any plans to deploy work on main like it was during IPC. Then, we cna make use of this TryToSetupEncodeOnSeparateThread() method.

hiroh@ this method isn't used at all and all the calls happen on Gpu IO thread. Does that answer your questions re flush? 

[0] https://cs.chromium.org/chromium/src/components/viz/service/gl/gpu_service_impl.cc?type=cs&q=CreateJpegDecodeAccelerator&sq=package:chromium&l=388

Deleted: trace_mfvea_on_io_thread.json.gz 1.5 MB

trace_mfvea_on_io_thread.json.gz 1.5 MB Download

emircan@, thank you for investigating.
Sounds like we can deprecate TryToSetupEncodeOnSeparateThread().
I would like to deprecate it; it is a simple design that one specific thread communicates with (i.e., calls VEA API for) one VEA instance.

What do you think?

> I would like to deprecate it; it is a simple design that one specific thread communicates with (i.e., calls VEA API for) one VEA instance.
Sure, I will assign the bug to you then. 

Thank you. Let me change the summary to stand for that, and include more people to discuss that here.

I would like to remove TryToSetupEncodeOnSeparateThread() from VEA API.
This was introduced for VEA on Windows and had been used by that. https://codereview.chromium.org/2427053002
In addition, it is currently not used even by VEA on Windows. There is no use case on Chrome video encoder stack. It can safely be deprecated.
I would like to deprecate it; it is a good design that one specific thread communicates with (i.e., calls VEA API for) one VEA instance.

I would like to ask the opinions of more familiar people about this.

Thanks
-Hiro

I'm only familiar with ChromeOS and less experienced with VEA (than VDA). Anyway, deprecating TryToSetupEncodeOnSeparateThread sounds good to me.

+1 for nuking TryToSetupEncodeOnSeparateThread(). 

The mojom VEA [1] doesn't even have this method, so it's effectively
superfluous. (non-ARC++) VEAs are created on the GPU IO thread (because 
Mojo svcs are listening there): Perhaps in the future we could move the
whole VEA to a background thread, but that's beyond this Issue and anyway
it seems like most (all) have an inner thread for doing the hard work.


[1] https://cs.chromium.org/chromium/src/media/mojo/interfaces/video_encode_accelerator.mojom

mcasas@, thank you for discussing offline.
I will start working on this in earnest tomorrow!

Create three CLs for this issue.
(1) crrev.com/c/997314 VEA: Remove TryToSetupEncodeOnSeparateThread from VEA API
(2) crrev.com/c/997035 MediaFoundationVEA: Deprecate TryToSetupEncodeOnSeparateThread
(3) crrev.com/c/997235 VEA unittest: Remove TryToSetupEncodeOnSeparateThread
and one nit CL
(4) crrev.com/c/997054 MojoVideoEncodeAcceleratorService: Delete one outdated TODO

(1) must be merged at last, but be approved at first

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

commit fef0bad0a76631093af23077cf1ddc7d50150083
Author: Hirokazu Honda <hiroh@chromium.org>
Date: Thu Apr 05 13:29:26 2018

MojoVideoEncodeAcceleratorService: Delete one outdated TODO

This CL just deletes one TODO. It stated that we would call
VEA::TryToSetupEncodeOnSeparateThread() with an ad-hoc background worker thread.
In our current stack, there is no need to do this.

BUG= chromium:828047 
TEST=None

Change-Id: I034e6a9e20101e54b08a664479bffb4e8f3df0f9
Reviewed-on: https://chromium-review.googlesource.com/997054
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548404}
[modify] https://crrev.com/fef0bad0a76631093af23077cf1ddc7d50150083/media/mojo/services/mojo_video_encode_accelerator_service.cc

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

commit 8b33caf331142860d336e284cb60beac957fc10f
Author: Hirokazu Honda <hiroh@chromium.org>
Date: Thu Apr 05 23:12:01 2018

MediaFoundationVEA: Deprecate TryToSetupEncodeOnSeparateThread

TryToSetupEncodeOnSeparateThread() in MediaFoundationVEA is not being called at
all and will never be used.
This CL deprecates the function in MediaFoundationVEA. As a result, all the VEA
interface functions are executed by GPU IO thread.

BUG= chromium:828047 
TEST=VEA unittest

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ie6b236b637ce99a29116dba9fffda89562830067
Reviewed-on: https://chromium-review.googlesource.com/997035
Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548605}
[modify] https://crrev.com/8b33caf331142860d336e284cb60beac957fc10f/media/gpu/windows/media_foundation_video_encode_accelerator_win.cc
[modify] https://crrev.com/8b33caf331142860d336e284cb60beac957fc10f/media/gpu/windows/media_foundation_video_encode_accelerator_win.h

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

commit 0a45d774c8f70751f38c2b9fc474143c24db630f
Author: Hirokazu Honda <hiroh@chromium.org>
Date: Sat Apr 07 04:18:08 2018

VEA unittest: Remove TryToSetupEncodeOnSeparateThread

TryToSetupEncodeOnSeparateThread will be removed from VEA API.
Thanks to removing the function, VEA client runs only one specific thread and
the thread executes all the VEA client functions.

BUG= chromium:828047 
TEST=VEA unittest

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I2cf1fa9e5df3472d81f93405014d2b96b9ad39c1
Reviewed-on: https://chromium-review.googlesource.com/997235
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549032}
[modify] https://crrev.com/0a45d774c8f70751f38c2b9fc474143c24db630f/media/gpu/video_encode_accelerator_unittest.cc

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

commit 717bdb737a2157e336024590985e0c7bb6dad34c
Author: Hirokazu Honda <hiroh@chromium.org>
Date: Sat Apr 07 06:32:19 2018

VEA: Remove TryToSetupEncodeOnSeparateThread from VEA API

TryToSetupEncodeOnSeparateThread() was introduced to avoid frame drops on
Windows by allowing frequent VEA functions like Encode()
and and BitstreamBufferReady() to run on a worker thread.
Usage of TryToSetupEncodeOnSeparateThread() was dropped when we moved to
MojoVEA. No VEA uses the function and will never.

BUG= chromium:828047 
TEST=VEA unittest

Change-Id: I3ff677803f3c3e08604a15cf32e073e107c9ed56
Reviewed-on: https://chromium-review.googlesource.com/997314
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549043}
[modify] https://crrev.com/717bdb737a2157e336024590985e0c7bb6dad34c/media/video/video_encode_accelerator.cc
[modify] https://crrev.com/717bdb737a2157e336024590985e0c7bb6dad34c/media/video/video_encode_accelerator.h