Interfaces for encoded video sources

media/video/encoded_video_source.h

Index: media/video/encoded_video_source.h
diff --git a/media/video/encoded_video_source.h b/media/video/encoded_video_source.h
new file mode 100644
index 0000000000000000000000000000000000000000..7cb2c292c30e8093b1b2bdd064114dbb751e9334
--- /dev/null
+++ b/media/video/encoded_video_source.h
@@ -0,0 +1,112 @@
+// Copyright (c) 2013 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.
+
+#ifndef MEDIA_VIDEO_ENCODED_VIDEO_SOURCE_H_
+#define MEDIA_VIDEO_ENCODED_VIDEO_SOURCE_H_
+
+#include "base/memory/ref_counted.h"
+#include "media/base/encoded_bitstream_buffer.h"
+#include "media/video/video_encode_types.h"
+
+namespace media {
+
+// Class to represent an encoded video bitstream which originates from an
+// EncodedVideoSource. The interface is usually obtained through initialization
+// with OpenBitstream call to EncodedVideoSource. Each EncodedVideoBitstream
+// instance has only one client, but there can be multiple EncodedVideoBitstream
+// objects representing the same concrete bitstream from a EncodedVideoSource.

[Ami GONE FROM CHROMIUM, 2013/03/18 22:53:45]

What is the concrete use-case here?

[Ville-Mikko Rautio, 2013/03/19 16:45:43]

E.g. considering two peerconnections on the same page looking to stream 720p and
a device that can do 1x720p. The device could still serve both by duplicating
the stream into two separate clients.

+//
+// Client can close the bitstream simply by discarding the scoped_refptr to the

[Ami GONE FROM CHROMIUM, 2013/03/18 22:53:45]

How does this work?  The concrete impl of EVB will have a pointer to the Client,
and it's quite likely that the Client will not be the only thing that's holding
a ref on the EVB impl.  Therefore how do you prevent the EVB from making further
calls on Client (which may be gone by then)?

The problem would be more apparent if you documented the lifetime/ownership
requirements on OpenBitstream for the client pointer, which are that Client must
outlive the returned EVB, but Client can't guarantee that without owning the
EVB, and it can't own the EVB while refcounting is happening.

[Ville-Mikko Rautio, 2013/03/19 16:45:43]

I implemented it by having proxy object (owned by client) derive from EVB and
delegating the functionality to concrete object (owned by EVS). If client goes
away, proxy object will go away and inform the concrete object that the proxy
went away and EVS may choose to destroy the concrete object if no more clients
are there for the concrete object. If concrete object goes away, it informs the
proxy (and consequently client through the proxy) and client can discard the
proxy without causing calls to a dangling pointer.

+// object. Also, if client receives OnRemoved callback from the bitstream, it
+// should consider bitstream instance invalid and can discard the pointer to it.
+class MEDIA_EXPORT EncodedVideoBitstream :
+    public base::RefCountedThreadSafe<EncodedVideoBitstream> {
+ public:
+  class MEDIA_EXPORT Client {
+   public:
+    // After OnStreaming callback stream shall be considered to be streaming.

[Ami GONE FROM CHROMIUM, 2013/03/18 22:53:45]

Comment style is unstylish.  Specifically here I'm thinking of:

    These comments should be descriptive ("Opens the file") rather than
imperative ("Open the file");

(from
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Function_Comments
make sure to click the black triangle to expand details) but that entire section
is worth internalizing.

[Ami GONE FROM CHROMIUM, 2013/03/18 22:53:45]

doco modifiable, probably rename to "is_mutable".

[Ville-Mikko Rautio, 2013/03/19 16:45:43]

Done.

[Ville-Mikko Rautio, 2013/03/19 16:45:43]

Done.

+    virtual void OnStreaming(
+        scoped_refptr<EncodedVideoBitstream> bitstream, bool modifiable) = 0;
+
+    // After OnRemoved client has to stop using the bitstream object and
+    // expecting bitstream buffers for that stream. OnRemoved will be called
+    // also in case of any unrecoverable failure in the device. After
+    // OnRemoved is called for a bitstream it is guaranteed that that there will
+    // be no longer pending calls coming in to the client.
+    virtual void OnRemoved(scoped_refptr<EncodedVideoBitstream> bitstream) = 0;
+
+    // OnBufferReady delivers the captured bitstream buffer by buffer to the
+    // client.
+    virtual void OnBufferReady(
+        scoped_refptr<EncodedVideoBitstream> bitstream,
+        scoped_refptr<const EncodedBitstreamBuffer> buffer) = 0;
+
+    // OnConfigChanged informs about change in bitstream parameters that can
+    // change during runtime.

[Ami GONE FROM CHROMIUM, 2013/03/18 22:53:45]

What about the initial parameters?  Shouldn't they be an arg to OnStreaming?

[Ville-Mikko Rautio, 2013/03/19 16:45:43]

Original idea was that if parameters given in OpenBitstream are not exactly
obeyed, initialization would fail. Behaviour was changed to "init with closest
possible parameters" and for this reason and your comment I added initial
parameters to OnStreaming.

+    virtual void OnConfigChanged(
+        scoped_refptr<EncodedVideoBitstream> bitstream,
+        const RuntimeVideoEncodingParameters& params) = 0;
+  };
+
+  // TryConfigure issues a reconfiguration request. Old configuration must be
+  // considered to be the valid configuration until OnBitstreamConfigChanged
+  // callback has been issued with value signalling successful change.

[Ami GONE FROM CHROMIUM, 2013/03/18 22:53:45]

Please rewrite this comment.

[Ville-Mikko Rautio, 2013/03/19 16:45:43]

Done.

+  virtual void TryConfigure(
+      const RuntimeVideoEncodingParameters& params) = 0;
+
+  // RequestSpecialFrame allows the client to request special frames from the
+  // encoded video bitstream. The effect of the request is only visible in the
+  // bitstream buffers passed to client through the OnBitstreamReady callback.
+  // This request is served on best-effort basis and client is not given any
+  // guarantees of the realization or timing of the request. Flags parameters
+  // will be interpreted in format specific manner using enumerations.
+  virtual void RequestSpecialFrame(int flags) = 0;
+
+ protected:
+  virtual ~EncodedVideoBitstream() {};
+  friend class base::RefCountedThreadSafe<EncodedVideoBitstream>;
+};
+
+// Interface to represent any encoded video source. EncodedVideoSource tries
+// to capture the essentials of what a client of an encoder would expect from
+// the *output video bitstream*. Therefore EncodedVideoSource does not specify
+// where the input pictures come from or how the lifetime of the device is
+// managed. Interface is primarily focused around the concept of bitstream,
+// discovery the configuration space of such bitstreams from device with video
+// encoding capability, mechanisms to instantiate such a bitstream, manipulation
+// of the bitstream properties, reception of interesting bitstream events and
+// reception of the stream of buffers in the bitstream.
+//
+// Anything that provides encoded video bitstream to clients can be an
+// EncodedVideoSource. Typical examples of this can be video encoder (duh!) and
+// webcam that has encoding capability. In case of video encoder implementation
+// would inherit this interface and add at least instantiatiation and
+// destruction functionality for the encoder instance and some mechanism to feed
+// the input to it. In case of webcam implementation would again inherit this
+// same interface and add mechanisms to instantiate and close the webcam, but it
+// would not have to have a mechanism to feed the input since it has internal
+// video source.
+class MEDIA_EXPORT EncodedVideoSource {

[Ami GONE FROM CHROMIUM, 2013/03/18 22:53:45]

This is conceptually the entry point to this file, so it should be at the top of
the file.

[Ville-Mikko Rautio, 2013/03/19 16:45:43]

I considered this earlier. Problem with that is inner class
EncodedVideoBitstream::Client which cannot be forward declared, if kept as inner
class. Of course I could move it outside EVB, but I think having it as inner
class in EVB emphasizes the client-server relationship between the EVB and
clients, which carries some value by itself.

+ public:
+  virtual ~EncodedVideoSource() {};
+
+  // GetCapabilities allows the discovery of the limitations encoded video
+  // bitstreams from this source have.
+  virtual VideoEncodingLimits GetLimits() = 0;
+
+  // OpenBitstream returns object for the stream being added. Client must
+  // consider the returned stream valid until OnBitstreamRemoved callback is
+  // called with the id. Client should check the parameters agains limits

[Ami GONE FROM CHROMIUM, 2013/03/18 22:53:45]

typo: agains

[Ville-Mikko Rautio, 2013/03/19 16:45:43]

Done.

+  // reported by GetCapabilities before trying to issue an request to add an

[Ami GONE FROM CHROMIUM, 2013/03/18 22:53:45]

s/an request/a request/

[Ville-Mikko Rautio, 2013/03/19 16:45:43]

Done.

+  // encoded bitstream. Returned EncodedVideoBitstream is only a proxy handle
+  // to the actual bitstream object.
+  virtual scoped_refptr<EncodedVideoBitstream> OpenBitstream(
+      EncodedVideoBitstream::Client* client,
+      const VideoEncodingParameters& params) = 0;
+};
+
+}  // namespace media
+
+#endif  // MEDIA_VIDEO_ENCODED_VIDEO_SOURCE_H_
+