Make cricket::VideoCapturer implement VideoSourceInterface

talk/media/base/videosourcebase.h

Index: talk/media/base/videosourcebase.h
diff --git a/talk/media/base/videorenderer.h b/talk/media/base/videosourcebase.h
similarity index 57%
copy from talk/media/base/videorenderer.h
copy to talk/media/base/videosourcebase.h
index 9771c492d65dfe7a0e9370f1631e10e3bf1c4722..d1ba339715ad7fe7c856405cebc643a2363b95d3 100644
--- a/talk/media/base/videorenderer.h
+++ b/talk/media/base/videosourcebase.h
@@ -1,6 +1,6 @@
 /*
  * libjingle
- * Copyright 2004 Google Inc.
+ * Copyright 2016 Google Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
@@ -25,28 +25,39 @@
  * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef TALK_MEDIA_BASE_VIDEORENDERER_H_
-#define TALK_MEDIA_BASE_VIDEORENDERER_H_
+#ifndef TALK_MEDIA_BASE_VIDEOSOURCEBASE_H_
+#define TALK_MEDIA_BASE_VIDEOSOURCEBASE_H_
 
-#include "webrtc/media/base/videosinkinterface.h"
+#include <unordered_set>
 
-namespace cricket {
+#include "talk/media/base/videoframe.h"
+#include "webrtc/media/base/videosinkinterface.h"
+#include "webrtc/media/base/videosourceinterface.h"
 
-class VideoFrame;
+namespace rtc {
 
-// Abstract interface for rendering VideoFrames.
-class VideoRenderer : public rtc::VideoSinkInterface<VideoFrame> {
+class VideoSourceBase : public VideoSourceInterface<cricket::VideoFrame> {

[pthatcher1, 2016/02/03 15:38:36]

Can we call this thing something more explicit, like VideoBroadcaster?  

[perkj_webrtc, 2016/02/08 14:32:01]

Done.

  public:
-  virtual ~VideoRenderer() {}
-  // Called when a new frame is available for display.
-  virtual bool RenderFrame(const VideoFrame *frame) = 0;
-  // Intended to replace RenderFrame.
-  void OnFrame(const cricket::VideoFrame& frame) override {
-    // Unused return value
-    RenderFrame(&frame);
-  }
+  void AddSink(VideoSinkInterface<cricket::VideoFrame>* sink,
+               const VideoSinkCapabilities& capabilities) override;

[pthatcher1, 2016/02/03 15:38:36]

Why does AddSink take the capabilities instead of calling sink->capabilities()?

[perkj_webrtc, 2016/02/08 14:32:01]

Acknowledged.

+  void RemoveSink(VideoSinkInterface<cricket::VideoFrame>* sink) override;
+
+  // TODO(perkj): Make DeliverFrameToSinks protected when the hack in
+  // FrameInputWrapper->DeliverFrameToSinks (videosource.cc) has been removed.
+  void DeliverFrameToSinks(const cricket::VideoFrame& frame);

[nisse-webrtc, 2016/02/03 09:16:34]

Is there any obvious drawback to rename this OnFrame, and make the class also
inherit VideoSinkInterface?

[pthatcher1, 2016/02/03 15:38:36]

Yes, I was thinking the same thing.  I think that makes sense.  

Of course, then you have to implement capability merging (assuming we stick with
that design), but I think that makes sense as well.

[perkj_webrtc, 2016/02/08 14:32:01]

Done.

+
+ protected:
+  bool HasSinks() const { return !sinks_.empty(); }
+
+  virtual void OnSinkCapabilitiesChanged(
+      const VideoSinkCapabilities& capabilities) = 0;
+
+ private:
+  CriticalSection sink_lock_;

[nisse-webrtc, 2016/02/03 09:16:34]

I guess this makes sense, if used as a base class. If instead used as a member,
I think it would make more sense to have the using class be responsible for
locking.

[pthatcher1, 2016/02/03 15:38:36]

As mentioned earlier, I'd like to see us try making it a member instead of a
base class.

[perkj_webrtc, 2016/02/08 14:32:01]

Done.

+  VideoSinkCapabilities capabilities_;
+  std::unordered_set<VideoSinkInterface<cricket::VideoFrame>*> sinks_;

[nisse-webrtc, 2016/02/03 09:16:34]

I think Tommi objected to using std::set earlier, since it implies unneeded
overhead. If we're not expecting more than a dozen sinks, std::vector or
std::list should be good enough, right?

[pthatcher1, 2016/02/03 15:38:36]

Throughout our code base, we almost alway use std::vector.  That's what I'd
prefer here.

[perkj_webrtc, 2016/02/08 14:32:01]

Done.

 };
 
-}  // namespace cricket
+}  // namespace rtc
 
-#endif  // TALK_MEDIA_BASE_VIDEORENDERER_H_
+#endif  // TALK_MEDIA_BASE_VIDEOSOURCEBASE_H_