ArcBridge: Rename VideoAcceleratorService to VideoDecodeAccelerator.

ArcBridge: Rename VideoAcceleratorService to VideoDecodeAccelerator.

The mojo interface "VideoAcceleratorService" is actually only used
for decoding. For encoding, we are going to add another interface
which will follow the media::VideoEncodeAccelerator design. To prevent
confusion, rename VideoAcceleratorService to VideoDecodeAccelerator.

BUG=b:32692654
TEST=Play YT in ARC on Cyan

Review-Url: https://codereview.chromium.org/2919193002
Cr-Commit-Position: refs/heads/master@{#478221}
Committed: https://chromium.googlesource.com/chromium/src/+/5c60ac3e0a44116f5876fc462c7be4884ecbf8d9

[Owen Lin, 2017-06-03 03:11:10]

owenlin@chromium.org changed reviewers:
+ dcheng@chromium.org, kcwu@chromium.org, lhchavez@chromium.org,
posciak@chromium.org

[Owen Lin, 2017-06-03 03:11:11]

PTAL

[Owen Lin, 2017-06-03 03:31:52]

Description was changed from

==========
ArcBridge: Rename VideoAcceleratorService to VideoDecodeAccelerator.

The mojo interface "VideoAcceleratorService" is actually only used
for decoding. For encoding, we are going to add another interface
which will follow the media::VideoEncodeAccelerator design. To prevent
confusion, rename VideoAcceleratorService to VideoDecodeAccelerator.

BUG=b:32692654
TEST=Play YT in ARC++ on Cyan
==========

to

==========
ArcBridge: Rename VideoAcceleratorService to VideoDecodeAccelerator.

The mojo interface "VideoAcceleratorService" is actually only used
for decoding. For encoding, we are going to add another interface
which will follow the media::VideoEncodeAccelerator design. To prevent
confusion, rename VideoAcceleratorService to VideoDecodeAccelerator.

BUG=b:32692654
TEST=Play YT in ARC on Cyan
==========

[kcwu, 2017-06-03 10:36:02]

lgtm

[dcheng, 2017-06-05 18:29:06]

LGTM

[Pawel Osciak, 2017-06-06 09:09:38]

I'm a bit worried that having two similarly named classes,
"GpuArcVideoDecodeAccelerator" and "ArcGpuVideoDecodeAccelerator", would be
confusing.


It's hard to find distinctive names with all the different APIs, but here is
what I could come up with. We have:

- VideoDecodeAccelerator IPC/API
- ArcVideoDecodeAccelerator IPC/API



Our naming convention for VDA API has been:

1) Prefix "VideoDecodeAccelerator" with implementation-specific strings, e.g.
V4L2VideoDecodeAccelerator (i.e. "V4L2 implementation of
VideoDecodeAccelerator").

2) Prefix the common IPC handler in GPU process that forwards
"VideoDecodeAccelerator" IPC to implementations with "Gpu", i.e.
GpuVideoDecodeAccelerator.



In this convention, for ArcVDA API we'd have:

1) Implementations of ArcVideoDecodeAccelerator: as we have a common "Chrome"
implementation of ArcVDA, "ChromeArcVideoDecodeAccelerator" (i.e. "Chrome
implementation of ArcVideoDecodeAccelerator").

2) IPC handler in GPU process that forwards ArcVideoDecodeAccelerator IPC to
implementations: GpuArcVideoDecodeAccelerator.



This would mean s/ArcGpuVideoDecodeAccelerator/ChromeArcVideoDecodeAccelerator/,
while GpuArcVideoDecodeAccelerator would keep its name.


WDYT?

[Luis Héctor Chávez, 2017-06-07 14:56:50]

lgtm with nits

https://codereview.chromium.org/2919193002/diff/1/components/arc/common/video...
File components/arc/common/video_decode_accelerator.mojom (right):

https://codereview.chromium.org/2919193002/diff/1/components/arc/common/video...
components/arc/common/video_decode_accelerator.mojom:43: enum DeviceType {
Can you add a "Deprecated" suffix?

https://codereview.chromium.org/2919193002/diff/1/components/arc/common/video...
components/arc/common/video_decode_accelerator.mojom:49: DeviceType device_type;
same here, "_deprecated".

[Owen Lin, 2017-06-08 02:32:40]

On 2017/06/06 09:09:38, Pawel Osciak wrote:
> I'm a bit worried that having two similarly named classes,
> "GpuArcVideoDecodeAccelerator" and "ArcGpuVideoDecodeAccelerator", would be
> confusing.
> 
> 
> It's hard to find distinctive names with all the different APIs, but here is
> what I could come up with. We have:
> 
> - VideoDecodeAccelerator IPC/API
> - ArcVideoDecodeAccelerator IPC/API
> 
> 
> 
> Our naming convention for VDA API has been:
> 
> 1) Prefix "VideoDecodeAccelerator" with implementation-specific strings, e.g.
> V4L2VideoDecodeAccelerator (i.e. "V4L2 implementation of
> VideoDecodeAccelerator").
> 
> 2) Prefix the common IPC handler in GPU process that forwards
> "VideoDecodeAccelerator" IPC to implementations with "Gpu", i.e.
> GpuVideoDecodeAccelerator.
> 
> 
> 
> In this convention, for ArcVDA API we'd have:
> 
> 1) Implementations of ArcVideoDecodeAccelerator: as we have a common "Chrome"
> implementation of ArcVDA, "ChromeArcVideoDecodeAccelerator" (i.e. "Chrome
> implementation of ArcVideoDecodeAccelerator").
> 
> 2) IPC handler in GPU process that forwards ArcVideoDecodeAccelerator IPC to
> implementations: GpuArcVideoDecodeAccelerator.
> 
> 
> 
> This would mean
s/ArcGpuVideoDecodeAccelerator/ChromeArcVideoDecodeAccelerator/,
> while GpuArcVideoDecodeAccelerator would keep its name.
> 
> 
> WDYT?

I sounds good. It looks much more clear.

[Owen Lin, 2017-06-08 02:32:50]

https://codereview.chromium.org/2919193002/diff/1/components/arc/common/video...
File components/arc/common/video_decode_accelerator.mojom (right):

https://codereview.chromium.org/2919193002/diff/1/components/arc/common/video...
components/arc/common/video_decode_accelerator.mojom:43: enum DeviceType {
On 2017/06/07 14:56:49, Luis Héctor Chávez wrote:
> Can you add a "Deprecated" suffix?

Done.

https://codereview.chromium.org/2919193002/diff/1/components/arc/common/video...
components/arc/common/video_decode_accelerator.mojom:49: DeviceType device_type;
On 2017/06/07 14:56:49, Luis Héctor Chávez wrote:
> same here, "_deprecated".

Done.

[Pawel Osciak, 2017-06-09 03:58:28]

lgtm

[Owen Lin, 2017-06-09 05:51:20]

The CQ bit was checked by owenlin@chromium.org

[Owen Lin, 2017-06-09 05:51:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
lhchavez@chromium.org, kcwu@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2919193002/#ps20001
(title: "Address review comments")

[commit-bot: I haz the power, 2017-06-09 05:51:40]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-09 07:19:46]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1496987480745100,
"parent_rev": "f08b6088f9c359d3fc6862b26ac136c4dbe10fe6", "commit_rev":
"5c60ac3e0a44116f5876fc462c7be4884ecbf8d9"}

[commit-bot: I haz the power, 2017-06-09 07:19:56]

Description was changed from

==========
ArcBridge: Rename VideoAcceleratorService to VideoDecodeAccelerator.

The mojo interface "VideoAcceleratorService" is actually only used
for decoding. For encoding, we are going to add another interface
which will follow the media::VideoEncodeAccelerator design. To prevent
confusion, rename VideoAcceleratorService to VideoDecodeAccelerator.

BUG=b:32692654
TEST=Play YT in ARC on Cyan
==========

to

==========
ArcBridge: Rename VideoAcceleratorService to VideoDecodeAccelerator.

The mojo interface "VideoAcceleratorService" is actually only used
for decoding. For encoding, we are going to add another interface
which will follow the media::VideoEncodeAccelerator design. To prevent
confusion, rename VideoAcceleratorService to VideoDecodeAccelerator.

BUG=b:32692654
TEST=Play YT in ARC on Cyan

Review-Url: https://codereview.chromium.org/2919193002
Cr-Commit-Position: refs/heads/master@{#478221}
Committed:
https://chromium.googlesource.com/chromium/src/+/5c60ac3e0a44116f5876fc462c7b...
==========

[commit-bot: I haz the power, 2017-06-09 07:19:58]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/5c60ac3e0a44116f5876fc462c7b...