GpuArcVideoService

components/arc/common/video_accelerator.mojom

Index: components/arc/common/video_accelerator.mojom
diff --git a/components/arc/common/video_accelerator.mojom b/components/arc/common/video_accelerator.mojom
new file mode 100644
index 0000000000000000000000000000000000000000..c0ff715ed9ebfc29f56dbdc936b133ac9b017b6b
--- /dev/null
+++ b/components/arc/common/video_accelerator.mojom
@@ -0,0 +1,88 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+module arc;
+
+[Extensible]
+enum HalPixelFormatExtension {
+  HAL_PIXEL_FORMAT_H264 = 0x34363248,
+  HAL_PIXEL_FORMAT_VP8 = 0x00385056,
+};
+
+enum PortType {
+  PORT_INPUT = 0,
+  PORT_OUTPUT = 1,
+};
+
+[Extensible]

[dcheng, 2016/04/10 05:01:23]

Extensible means that mojo won't do any verifications check on the value right?
Any particular reason that all these enums are extensible?

[kcwu, 2016/04/11 04:07:32]

I could remove Extensible from Error. Other enums are indeed extensible.
HalPixelFormatExtension may expand with new formats. BufferFlag are bitmask used
in BufferMetadata.flags.

[dcheng, 2016/04/11 04:26:32]

Can you help me understand why HalPixelFormatExtension needs to be extensible?
If we need to handle new values, wouldn't we just add them to the enum
definition? It doesn't look like a bitmask to me.

[kcwu, 2016/04/11 04:44:33]

I don't fully understand "Extensible". Luis suggested adding "Extensible" if
enums will need modification. I thought "modification" includes "adding new
values". Isn't it?
https://codereview.chromium.org/1641353003/diff/200001/components/arc/common/...

[dcheng, 2016/04/11 04:56:54]

Hmm, this is surprising to me. In the interest of unblocking this CL, I'll start
a separate thread to resolve this (sammc@ pointed me at
https://codereview.chromium.org/1618963006, which certainly seems to indicate
this is the case)

+enum BufferFlag {
+  BUFFER_FLAG_EOS = 1,
+};
+
+struct BufferMetadata {
+  int64 timestamp;  // in microseconds
+  uint32 flags;
+  uint32 bytes_used;
+};
+
+struct VideoFormat {
+  uint32 pixel_format;
+  uint32 buffer_size;
+
+  // minimal number of buffers required to process the video.
+  uint32 min_num_buffers;
+  uint32 coded_width;
+  uint32 coded_height;
+  uint32 crop_left;
+  uint32 crop_width;
+  uint32 crop_top;
+  uint32 crop_height;
+};
+
+struct ArcVideoAcceleratorConfig {
+  enum DeviceType {
+    DEVICE_ENCODER = 0,
+    DEVICE_DECODER = 1,
+  };
+
+  DeviceType device_type;
+  uint32 num_input_buffers;
+  uint32 input_pixel_format;
+};
+
+interface VideoAcceleratorService {
+  Initialize@0(ArcVideoAcceleratorConfig config) => (bool result);

[dcheng, 2016/04/10 05:01:23]

Please add comments to the interface for what the various methods are for / what
the arguments are (especially things like index)

[kcwu, 2016/04/11 04:07:32]

This mojom file is the clone of ArcVideoAccelerator interface. Since
ArcVideoAccelerator is the canonical definition, I added file comments that
refers to ArcVideoAccelerator for detail. Is this okay?

[dcheng, 2016/04/11 04:26:32]

Sounds good.

+
+  BindSharedMemory@1(PortType port, uint32 index, handle ashmem_fd,
+                   uint32 offset, uint32 length);
+
+  BindDmabuf@2(PortType port, uint32 index, handle dmabuf_fd);
+
+  UseBuffer@3(PortType port, uint32 index, BufferMetadata metadata);
+
+  SetNumberOfOutputBuffers@4(uint32 number);
+
+  Reset@5();
+};
+
+interface VideoAcceleratorServiceClient {
+  [Extensible]
+  enum Error {
+    NO_ERROR = 0,
+    ILLEGAL_STATE = 1,
+    INVALID_ARGUMENT = 2,
+    UNREADABLE_INPUT = 3,
+    PLATFORM_FAILURE = 4,
+  };
+
+  Init@0(VideoAcceleratorService service_ptr);
+
+  OnError@1(Error error);
+
+  OnBufferDone@2(PortType port, uint32 index, BufferMetadata metadata);
+
+  OnResetDone@3();
+
+  OnOutputFormatChanged@4(VideoFormat format);
+};