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

media/cast/cast_sender_impl.cc

Index: media/cast/cast_sender_impl.cc
diff --git a/media/cast/cast_sender_impl.cc b/media/cast/cast_sender_impl.cc
index 571e92ef54093eb37c86221f8139c0eab5c1fe26..9d6ce84b53a359bce29b6cbdb0b2bb6a89a24291 100644
--- a/media/cast/cast_sender_impl.cc
+++ b/media/cast/cast_sender_impl.cc
@@ -18,8 +18,11 @@ namespace cast {
 class LocalVideoFrameInput : public VideoFrameInput {
  public:
   LocalVideoFrameInput(scoped_refptr<CastEnvironment> cast_environment,
-                       base::WeakPtr<VideoSender> video_sender)
-      : cast_environment_(cast_environment), video_sender_(video_sender) {}
+                       base::WeakPtr<VideoSender> video_sender,
+                       scoped_refptr<VideoFrameFactory> video_frame_factory)
+      : cast_environment_(cast_environment),
+        video_sender_(video_sender),
+        video_frame_factory_(video_frame_factory) {}
 
   void InsertRawVideoFrame(const scoped_refptr<media::VideoFrame>& video_frame,
                            const base::TimeTicks& capture_time) override {
@@ -31,6 +34,10 @@ class LocalVideoFrameInput : public VideoFrameInput {
                                            capture_time));
   }
 
+  scoped_refptr<VideoFrame> CreateFrame(base::TimeDelta timestamp) override {
+    return video_frame_factory_->CreateFrame(timestamp);

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

Why not create VideoFrame through VideoSender?

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

My understanding is that VideoFrameInput is ref countable to allow it to be used
from any thread and have a lifetime independent from the rest of the cast
instance it belongs to. I wanted to allow clients to create frames from an
arbitrary thread (say a rendering thread) to keep latency in the system to a
minimum. Therefore, I had to have an intermediate ref counted object (the video
frame factory) for the same reason.

VideoSender supports weak pointers for the purpose of allowing the scheme where
some arbitrary thread submits a callback to the main cast thread, where the weak
pointer can be safely checked for validity before proceeding. I certainly could
have followed this model, but I wanted something that could provide better
performance.

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

In a latter comment you mentioned that the created VidoeFrameFactory will be
completely isolated from VidoeEncoder after creation.

Consider that VideoFrameFactory is enclosed in VideoFrameInput and
VideoFrameInput is already refcounted thread safe. What about removing
RefCountedThreadSafe from the definition of VideoFrameFactory? Then
VideoFrameInput would just keep it through a scoped_ptr<>.

This would satisfy the need to create VideoFrame on any thread (through
VideoFrameInput). I think scoped_refptr<> for VideoFrameFactory doesn't add much
value.

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

That sounds pretty good.

+  }
+
  protected:
   ~LocalVideoFrameInput() override {}
 
@@ -39,6 +46,7 @@ class LocalVideoFrameInput : public VideoFrameInput {
 
   scoped_refptr<CastEnvironment> cast_environment_;
   base::WeakPtr<VideoSender> video_sender_;
+  scoped_refptr<VideoFrameFactory> video_frame_factory_;

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

Please don't save VideoFrameFactory. It's an internal member of VideoSender. You
can already access VidoeFrameFactory through VideoSender or VideoSender can
create a VideoFrame using internal objects.

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

See my note above. I made VideoFrameFactory a thread safe ref-counted object for
the explicit purpose of being able to use it outside of the 3 dedicated cast
threads for performance / latency reasons.

 
   DISALLOW_COPY_AND_ASSIGN(LocalVideoFrameInput);
 };
@@ -170,7 +178,9 @@ void CastSenderImpl::OnVideoInitialized(
     media::cast::CastInitializationStatus result) {
   DCHECK(cast_environment_->CurrentlyOn(CastEnvironment::MAIN));
   video_frame_input_ =
-      new LocalVideoFrameInput(cast_environment_, video_sender_->AsWeakPtr());
+      new LocalVideoFrameInput(cast_environment_,
+                               video_sender_->AsWeakPtr(),
+                               video_sender_->video_frame_factory());
   initialization_cb.Run(result);
 }