[Cast] VideoFrameFactory interface to vend frames with encoder affinity.

media/cast/sender/video_frame_factory.h

Index: media/cast/sender/video_frame_factory.h
diff --git a/media/cast/sender/video_frame_factory.h b/media/cast/sender/video_frame_factory.h
new file mode 100644
index 0000000000000000000000000000000000000000..c17214bd272c77dec9beb0b2babea6c3278884fd
--- /dev/null
+++ b/media/cast/sender/video_frame_factory.h
@@ -0,0 +1,37 @@
+// Copyright 2014 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_CAST_SENDER_VIDEO_FRAME_FACTORY_H_
+#define MEDIA_CAST_SENDER_VIDEO_FRAME_FACTORY_H_
+
+#include "base/memory/ref_counted.h"
+
+namespace media {
+
+class VideoFrame;
+
+namespace cast {
+
+// Interface for an object capable of vending video frames. Not thread safe, but
+// must be usable from any thread. Implementations do not have to protect
+// themselves from concurrent access for performance reasons. Clients should
+// instead synchronize access themselves by design.
+class VideoFrameFactory : public base::RefCountedThreadSafe<VideoFrameFactory> {

[Alpha Left Google, 2014/11/17 20:23:14]

I think it's unrealistic that this class is not thread safe, but can be used on
any thread and communicates with encoder to creates the most performant
VideoFrame. This just means internally it has some cross thread communication
with the encoder or encoder thread.

[jfroy, 2014/11/17 21:23:06]

Not at all. The default implementation I wrote as well as the specialized
implementation for the VideoToolbox encoder do not interact with the encoder in
any way past the point of creation. This is exactly why this is a separate
object as opposed to a method on the VIdeoEncoder. It is expected to be fully
independent.

My comment is poorly worded I think. I meant to say that video frame factories
may not be concurrent (entry my multiple threads at the same time). They should
not be pinned to any particular thread. I make the recommendation that
synchronization should be done by the client / user for performance reasons,
since in many cases a factory will be used either by a single rendering thread
or by a rendering callback which may jump between threads (in a thread pool job
queue system, say) but never run concurrently with a prior invocation of itself.

[Alpha Left Google, 2014/11/18 02:57:58]

This default implementation and the one you mentioned both don't interact with
the encoder pass creation. If this is the case then the design can be much
simpler: there's no need for VideoFrameFactory to be RefCountedThreadSafe.
VideoEncoder just needs to provide a factory method for creating a specific
implementation of VideoFrameFactory. What do you think?

[jfroy, 2014/11/18 18:03:59]

Sounds good!

+ public:
+  // Creates a |VideoFrame| suitable for input via |InsertRawVideoFrame|. Frames
+  // obtained in this manner may provide benefits such memory reuse and affinity
+  // with the encoder. The format is guaranteed to be I420 or NV12.
+  virtual scoped_refptr<VideoFrame> CreateFrame(base::TimeDelta timestamp) = 0;
+
+ protected:
+  virtual ~VideoFrameFactory() {}
+
+ private:
+  friend class base::RefCountedThreadSafe<VideoFrameFactory>;
+};
+
+}  // namespace cast
+}  // namespace media
+
+#endif  // MEDIA_CAST_SENDER_VIDEO_FRAME_FACTORY_H_