MediaPerceptionPrivate IDL and skeleton.

extensions/common/api/media_perception_private.idl

Index: extensions/common/api/media_perception_private.idl
diff --git a/extensions/common/api/media_perception_private.idl b/extensions/common/api/media_perception_private.idl
new file mode 100644
index 0000000000000000000000000000000000000000..fb38f423f9d0866ce01d04e6b6f6ad501805d10a
--- /dev/null
+++ b/extensions/common/api/media_perception_private.idl
@@ -0,0 +1,170 @@
+// Copyright 2017 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.
+
+// Private API for receiving real-time media perception information.
+[platforms=("chromeos")]
+namespace mediaPerceptionPrivate {
+  enum Status {
+    // Media analytics process waiting to be launched.
+    UNINITIALIZED,
+
+    // Analytics process running and media processing pipeline started,

[Devlin, 2017/05/17 20:22:42]

nit: use complete sentences in comments, so something like: "*The* analytics
process *is* running and..." etc (throughout this file).

[Luke Sorenson, 2017/05/17 21:01:36]

Done.

+    // but it is not yet receiving image frames. This is a transitional state
+    // between SUSPENDED and RUNNING for the time it takes to warm up the
+    // media processing pipeline, which can take anywhere from a few seconds
+    // to a minute.
+    // Note: STARTED is the initial reply to SetState RUNNING.
+    STARTED,
+
+    // Analytics process running and media processing pipeling injesting image
+    // frames. At this point, MediaPerception signals should be coming over
+    // D-Bus.
+    RUNNING,
+
+    // Analytics process is running and ready to be set to state RUNNING.
+    // The D-Bus communications are enabled but the media processing pipeline
+    // is suspended.
+    SUSPENDED
+  };
+
+  // The system and configuration state of the analytics process and v4lplugin.
+  dictionary State {
+    Status status;
+
+    // Optional $(ref:setState) parameter. Specifies the video device the media
+    // analytics process should open while the media processing pipeline is
+    // starting. To set this parameter, status has to be RUNNING.
+    DOMString? deviceContext;
+  };
+
+  dictionary Point {
+    // The horizontal distance from the top left corner of the image.
+    double? x;

[Devlin, 2017/05/17 20:22:42]

When would these be null?

[Luke Sorenson, 2017/05/17 21:01:36]

Previously discussed the reason for these null values this with tbarzic, here's
a copy and paste of a blanket explanation for the reasoning behind using
optional values throughout:

Because many of these Dictionary definitions are based on proto messages in
media_perception.proto (and required values are now highly discouraged in
protos), to me it makes sense for the values here to be optional as well. If
we're converting from protos to these IDL classes, and for some reason, a value
isn't provided in the proto message, I don't think it makes sense for the
default value to appear in the JS Dictionary.

[Devlin, 2017/05/18 21:21:24]

That's a bit weird, but since this is a private API, I won't press the issue.

[Luke Sorenson, 2017/05/18 21:56:33]

Acknowledged.

+
+    // The vertical distance from the top left corner of the image.
+    double? y;
+  };
+
+  dictionary BoundingBox {
+    // Specifies whether the points are normalized to the size of the image.
+    boolean? normalized;
+
+    // The two points that define the corners of a bounding box.
+    Point? topLeft;
+    Point? bottomRight;
+  };
+
+  enum EntityType {
+    UNSPECIFIED,
+    FACE,
+    PERSON
+  };
+
+  dictionary Entity {
+    // A unique id associated with the detected entity, which can be used to
+    // track the entity over time.
+    long? id;

[Devlin, 2017/05/17 20:22:41]

Why would these be null?

I'm not familiar enough with the implementation to know if the others are
appropriately optional or not.

[Luke Sorenson, 2017/05/17 21:01:36]

See above explanation.

+
+    EntityType? type;
+
+    // Minimum box which captures entire detected entity.
+    BoundingBox? boundingBox;
+
+    // A value for the quality of this detection.
+    double? confidence;
+
+    // The estimated depth of the entity from the camera.
+    double? depthInMeters;
+  };
+
+  // The set of computer vision metadata for an image frame.
+  dictionary FramePerception {
+    long? frameId;
+
+    long? frameWidthInPx;
+    long? frameHeightInPx;
+
+    // The timestamp associated with the frame (when its recieved by the
+    // analytics process).
+    double? timestamp;
+
+    // The list of entities detected in this frame.
+    Entity[]? entities;

[Devlin, 2017/05/17 20:22:41]

Usually prefer an empty list rather than an optional one.

[Luke Sorenson, 2017/05/17 21:01:36]

Above explanation. Modeled after proto behavior.

+  };
+
+  dictionary MediaPerception {
+    // The time the media perception data was emitted by the media processing
+    // pipeline. This value will be greater than the timestamp stored within
+    // the FramePerception dictionary and the difference between them can be
+    // viewed as the processing time for a single frame.
+    double? timestamp;
+
+    // An array of framePerceptions.
+    FramePerception[]? framePerceptions;
+  };
+
+  enum ImageFormat {
+    UNSPECIFIED,
+    RGB,

[Devlin, 2017/05/17 20:22:41]

Is RGB an image format?

[Luke Sorenson, 2017/05/17 21:01:36]

Based on media_perception.proto, but this could be changed to RAW?

[Devlin, 2017/05/18 21:21:24]

RAW makes a bit more sense to me.  Comments would be helpful. :)

[Luke Sorenson, 2017/05/18 21:56:33]

Done.

+    PNG,
+    JPEG
+  };
+
+  dictionary ImageFrame {
+    long? width;
+    long? height;
+
+    ImageFormat? format;
+
+    long? dataLength;
+
+    // The bytes of the image frame.
+    ArrayBuffer? frame;
+  };
+
+  dictionary PerceptionSample {
+    // The video analytics FramePerception for the associated image frame
+    // data.
+    FramePerception? framePerception;
+
+    // The image frame data for the associated FramePerception object.
+    ImageFrame? imageFrame;
+  };
+
+  dictionary Diagnostics {
+    // A buffer of image frames and the associated video analytics information
+    // that can be used to diagnose a malfunction.
+    PerceptionSample[]? perceptionSamples;
+  };
+
+  callback StateCallback = void(State state);
+
+  callback DiagnosticsCallback = void(Diagnostics diagnostics);
+
+  interface Functions {
+    // Get the status of the media perception process.

[Devlin, 2017/05/17 20:22:42]

Function comments should be descriptive, not imperative (i.e., use "Gets"). 
Same for below.

[Luke Sorenson, 2017/05/17 21:01:36]

Done.

+    // |callback| : The current state of the system.
+    static void getState(StateCallback callback);
+
+    // Set the desired state of the system.
+    // |state| : A dictionary with the desired new state. The only settable
+    // states are RUNNING and SUSPENDED.
+    // |callback| : The State of the system after setting it. Can be used to

[Devlin, 2017/05/17 20:22:42]

The callback isn't the state of the system; it's invoked with the state of the
system.

That said, I think we'd probably just set this to void and indicate failure via
an error, rather than a different return result.

Additionally, the callback can probably be optional.

[Luke Sorenson, 2017/05/17 21:01:36]

We do indicate failure via an error, however there are cases where the callback
state isn't always the same as the requested state, but it's not necessarily an
error. We prefer to have the flexibility to return a different state than
requested from the media analytics process back to the frontend.

[Devlin, 2017/05/18 21:21:24]

When is it the case that calling setState() not setting the state is not an
error?

[Luke Sorenson, 2017/05/18 21:56:33]

For simplicity, the app can only setState RUNNING and SUSPENDED, because we
don't want the app to have to handle process management of the media analytics
process. If the process is not launched yet and the app calls setState RUNNING,
the reply will be { status: STARTED } which indicates that the process is
launched but the media analytics pipeline hasn't started running.

In the future, as our media analytics process potentially gets more complicated
/ adds features, there may be other cases where the return status may be
different than the provided status in setState.

[Devlin, 2017/05/19 15:08:12]

Is there a reason those cases shouldn't be surfaced through lastError?  When
would the behavior of the app be different based on that result?  It seems like,
from the app's perspective, if setState doesn't actually set the state, it's an
error.  Note that not all errors are fatal - so some errors can be handled,
which it sounds like is more what we'd want to do here.

It just seems like it's very convoluted to have to check both lastError *and*
the returned state to find out if setState didn't set the state...

[Luke Sorenson, 2017/05/19 15:51:04]

So it's never the case currently that you need to check both lastError and the
returned state (we only ever actually Respond with one). In general, the return
state in setState will either be the requested state or some transitional state,
in cases where the final state change to requested state may take more than a
few seconds. In the future, we intend to add an onStateChanged event as well to
notify the app about those changes, but because it wasn't expressly necessary
for the M60 implementation, it's not part of this initial IDL.

[Devlin, 2017/05/19 15:58:17]

Just because we only respond with one doesn't mean you only need to check one.
:)  If an app calls this:

chrome.mediaPerceptionPrivate.setState(desiredState, (newState) => {
  // Here, if I only check last error, I might miss that newState !=
desiredState.  If I only check
  // newState, I might miss the last error.  I need to check both.
});

But, again, I suppose it's not worth delving too deep into this.

+    // verify the state was set as desired.
+    static void setState(State state, StateCallback callback);
+
+    // Get a diagnostics buffer out of the video analytics process.
+    // |callback| : Returns a Diagnostics dictionary object.
+    static void getDiagnostics(DiagnosticsCallback callback);
+  };
+
+  interface Events {
+    // Fired when media perception information is received from the media
+    // analytics process.
+    // |mediaPerception| : The dictionary which contains a dump of everything
+    // the analytics process has detected or determined from the incoming media
+    // streams.
+    static void onMediaPerception(MediaPerception mediaPerception);
+  };
+};