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

media/cast/sender/video_encoder.h

Index: media/cast/sender/video_encoder.h
diff --git a/media/cast/sender/video_encoder.h b/media/cast/sender/video_encoder.h
index b3bdbe44f0b86d559e3289427d1bcf4f9d98902e..d088d8430ec7ab17642788f94fbd78c7b0e4c3b2 100644
--- a/media/cast/sender/video_encoder.h
+++ b/media/cast/sender/video_encoder.h
@@ -17,6 +17,8 @@
 namespace media {
 namespace cast {
 
+class VideoFrameFactory;
+
 // All these functions are called from the main cast thread.
 class VideoEncoder {
  public:
@@ -41,6 +43,10 @@ class VideoEncoder {
 
   // Inform the encoder to only reference frames older or equal to frame_id;
   virtual void LatestFrameIdToReference(uint32 frame_id) = 0;
+
+  // Allow the encoder to return a specialized video frame factory. May return
+  // null, in which case a default implementation will be used instead.
+  virtual scoped_refptr<VideoFrameFactory> GetVideoFrameFactory() = 0;

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

I don't think the threading model for this method is good.

According to this interface. VideoframeFactory is going to be called on the main
thread, while encoder is running on the encoder thread. Which means
VideoFrameFactory will have to perform cross thread communication with encoder
implementation. It is easy to make mistakes if each encoder & VideoFrameFactory
has to implement this. And it violates the design that VideoEncoder is
sinlge-threaded.

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

That's not quite the design I had in mind.

The expectation is that GetVideoFrameFactory() will always be called on the main
cast thread, in keeping with the design of VideoEncoder. Its purpose is to
create a or return an existing video frame factory (essentially a specialized
allocator) that is assumed to be usable from any thread (but perhaps not
concurrently; that detail is up to the implementation). This is why the video
frame factory is a thread-safe ref-counted object.

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

In this patch VideoFrameFactory is always accessed through VideoFrameInput which
is already a refcounted thread safe. Making VideoFrameFactory doesn't add more
value. Also there's only one owner of VideoFrameFactory in this patch, what
about change this to:
virtual scoped_ptr<VidoeFrameFactory> CreateVideoFrameFactory() = 0;

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

Sounds good.

 };
 
 }  // namespace cast