Make cricket::VideoCapturer implement VideoSourceInterface

webrtc/media/base/videobroadcaster.cc

+/*
+ *  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.
+ */
+
+#include "webrtc/media/base/videobroadcaster.h"
+
+#include "webrtc/base/checks.h"
+
+namespace rtc {
+
+VideoBroadcaster::VideoBroadcaster() {
+  thread_checker_.DetachFromThread();

[nisse-webrtc, 2016/02/09 14:10:54]

Might need a comment. From the docs, this seems to imply that the object may be
created on one thread, but exclusively used on a different thread. Why?

[perkj_webrtc, 2016/02/10 10:54:36]

I think this is common enough to not do it. thread_checker_.DetachFromThread()
allows something to be created on one thread and later used on another. It is
fairly common in libjingle that an object is created on the signaling thread and
later used on the worker thread. 

+}
+
+void VideoBroadcaster::AddOrUpdateSink(
+    VideoSinkInterface<cricket::VideoFrame>* sink,
+    const VideoWants& hints) {
+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+  RTC_DCHECK(sink != nullptr);
+  auto lambda = [sink](const SinkPair& sink_pair) {

[nisse-webrtc, 2016/02/09 14:10:54]

If the function is named, please use a descriptive name. (I'm happy with unnamed
functions too, but I'm not used to it in C++).

[pthatcher1, 2016/02/09 17:13:41]

Or just put it inline:

auto sink_pair_it = std::find_if(sinks_.begin(), sinks_.end(), [sink](const
SinkPair& sink_pair) {
  return sink_pair.first == sink;
});

[perkj_webrtc, 2016/02/10 10:54:36]

use FindSinkPair

[perkj_webrtc, 2016/02/10 10:54:36]

Done.

+    return sink_pair.first == sink;
+  };
+
+  auto sink_pair_it = std::find_if(sinks_.begin(), sinks_.end(), lambda);
+  if (sink_pair_it == sinks_.end()) {
+    sinks_.push_back(SinkPair(sink, hints));
+  } else {
+    sink_pair_it->second = hints;
+  }
+
+  // Rotation must be applied by the source if one sink request it.

[pthatcher1, 2016/02/09 17:13:41]

request it => wants it

[perkj_webrtc, 2016/02/10 10:54:36]

Done.

+  current_hints_.rotation_applied = false;
+  for (auto& sink_pair : sinks_) {
+    current_hints_.rotation_applied |= sink_pair.second.rotation_applied;
+  }
+}
+
+void VideoBroadcaster::RemoveSink(
+    VideoSinkInterface<cricket::VideoFrame>* sink) {
+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+  RTC_DCHECK(sink != nullptr);
+  auto lambda = [sink](const SinkPair& sink_pair) {

[nisse-webrtc, 2016/02/09 14:10:54]

Same function here.

[pthatcher1, 2016/02/09 17:13:41]

It might be worth making a private method like:

SinkPair* FindSinkPair(const VideoSinkInterface<cricket::VideoFrame>* sink);

That would remove duplication and let the caller just do if(pair) instead  of if
(pair != foo.end())

[perkj_webrtc, 2016/02/10 10:54:36]

Good idea.

+    return sink_pair.first == sink;
+  };
+  RTC_DCHECK(std::find_if(sinks_.begin(), sinks_.end(), lambda) !=
+             sinks_.end());
+  sinks_.erase(std::remove_if(sinks_.begin(), sinks_.end(), lambda),
+               sinks_.end());
+}
+
+bool VideoBroadcaster::frame_wanted() const {
+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+  return !sinks_.empty();
+}
+
+VideoWants VideoBroadcaster::sink_wants() const {

[pthatcher1, 2016/02/09 17:13:41]

Why not just wants()?  If not wants(), then at least sinks_wants() (since there
is more than one sink).

[perkj_webrtc, 2016/02/10 10:54:36]

Done.

+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+  return current_hints_;
+}
+
+void VideoBroadcaster::OnFrame(const cricket::VideoFrame& frame) {
+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+  for (auto& sink : sinks_) {
+    sink.first->OnFrame(frame);
+  }
+}
+
+}  // namespace rtc