Make cricket::VideoCapturer implement VideoSourceInterface

webrtc/media/base/videosourceinterface.h

+/*
+ *  Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef MEDIA_BASE_VIDEOSOURCEINTERFACE_H_
+#define MEDIA_BASE_VIDEOSOURCEINTERFACE_H_
+
+#include "webrtc/media/base/videosinkinterface.h"
+
+namespace rtc {
+
+template <typename VideoFrameT>
+class VideoSourceInterface {
+ public:
+  virtual void AddSink(VideoSinkInterface<VideoFrameT>* sink) = 0;
+  virtual void RemoveSink(VideoSinkInterface<VideoFrameT>* sink) = 0;
+
+protected:
+        virtual ~VideoSourceInterface() {};
+};
+
+template <typename VideoFrameT>
+class VideoSourceBase : public VideoSourceInterface<VideoFrameT> {

[pthatcher, 2016/02/01 18:40:54]

Where is the impl of this?

[perkj_webrtc, 2016/02/02 16:24:00]

Sorry - maybe I was unclear yesterday- the cl was not done. It was more a
statement that I had started on this and wanted no let you and nisse know so he
did not spend time on updating all unittests to pass his attempt.

Not this is moved to talk/media/base since I have to add the cc-file to a gyp
file.

+ public:
+  void AddSink(VideoSinkInterface<VideoFrameT>* sink) override;
+  void RemoveSink(VideoSinkInterface<VideoFrameT>* sink) override;
+
+ protected:
+  virtual ~VideoSourceBase();
+
+  bool HasSinks() const { return !sinks_.empty(); }
+  void DeliverFrame(const VideoFrameT*);

[pthatcher, 2016/02/01 18:40:54]

Can you call this DeliverFrameToSinks, or better, SendFrameToSinks?

[nisse-webrtc, 2016/02/02 09:52:30]

I'm not entirely sure we should have a general mux object here. We have muxes,
e.g., VideoTrack ought to ingerit both VideoSourceInterface and
VideoSinkInterface.

I think we should do one of the following:

1. Let VideoSourceBase maintain a protected: sink list (should it be std::list
or std::vector, btw?), implement AddSink and RemoveSink, and nothing more.

2. Write a VideoMux class which implements both VideoSourceInterface and
VideoSinkInterface. It should also be responsible for merging of capabilities.
VideoTrack would inherit this class, and only add the enabled state and mute
logic (by overriding OnFrame).

3. Do no generic implementation now, just implement what's needed right now in
each VideoSource classes.

My gut feeling is that (1) is mostly useless abstraction, it will be just a
std::vector with renamed methods (this depends also on whether or not we include
the capabilities with the AddSink method), while (2) makes sense but I think
it's a bit premature to do now, let's do this abstraction after we have two
places which needs it.

By the way, do you think we will need a "capability" saying that the sink is
muted, i.e., currently not interested in any frames? May be useful for a
disabled VideoTrack, or for a general mux with no downstream sinks (I guess your
thinking of something similar with the HasSinks method?). 

[perkj_webrtc, 2016/02/02 16:24:00]

Done.

+
+ private:
+  VideoSinkCapabilities capabilities_;

[pthatcher, 2016/02/01 18:40:54]

Is this a merged capabilities?  Is it worth storing?  Could you not just have a
capabilities() method that computes it on the fly?

[perkj_webrtc, 2016/02/02 16:24:00]

First attempt I would just like this to handle the case where all sinks have the
same capabilities. Ie- the way we use it today. I use this to DCHECK that they
actually are the same.

+  std::vector<VideoSinkInterface<VideoFrameT>*> sinks_;
+};
+
+}  // namespace rtc
+#endif  // MEDIA_BASE_VIDEOSOURCEINTERFACE_H_