Simplify CaptureRenderAdapter.

talk/media/base/capturerenderadapter.cc

 /*
  * libjingle
  * Copyright 2012 Google Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *
  *  1. Redistributions of source code must retain the above copyright notice,
  *     this list of conditions and the following disclaimer.
  *  2. Redistributions in binary form must reproduce the above copyright notice,
@@ skipping 43 matching lines @@
 CaptureRenderAdapter* CaptureRenderAdapter::Create(
     VideoCapturer* video_capturer) {
   if (!video_capturer) {
     return NULL;
   }
   CaptureRenderAdapter* return_value = new CaptureRenderAdapter(video_capturer);
   return_value->Init();  // Can't fail.
   return return_value;
 }
 
-bool CaptureRenderAdapter::AddRenderer(VideoRenderer* video_renderer) {
-  if (!video_renderer) {
-    return false;
-  }
+void CaptureRenderAdapter::AddRenderer(VideoRenderer* video_renderer) {
+  RTC_DCHECK(video_renderer);
+
   rtc::CritScope cs(&capture_crit_);
-  if (IsRendererRegistered(*video_renderer)) {
-    return false;
-  }
-  video_renderers_.push_back(VideoRendererInfo(video_renderer));
-  return true;
+  // This implements set semantics, the same renderer can only be
+  // added once. TODO(nisse): Is this really needed?

[tommi, 2016/01/21 08:45:34]

nit: move TODO to a separate line.
Agree that this shouldn't be needed.  We could dcheck that
std::find(video_renderers_.begin(), video_renderers_.end(), video_renderer) ==
vector.end()

[nisse-webrtc, 2016/01/21 09:11:44]

Done. I switched to using std::find, and deleted the IsRendererRegistered
method.

+  if (!IsRendererRegistered(*video_renderer))
+    video_renderers_.push_back(video_renderer);
 }
 
-bool CaptureRenderAdapter::RemoveRenderer(VideoRenderer* video_renderer) {
-  if (!video_renderer) {
-    return false;
-  }
+void CaptureRenderAdapter::RemoveRenderer(VideoRenderer* video_renderer) {
+  RTC_DCHECK(video_renderer);
+
   rtc::CritScope cs(&capture_crit_);
+  // TODO(nisse): Switch to using std::list, and use its remove
+  // method. And similarly in VideoTrackRenderers, which this class
+  // mostly duplicates.
   for (VideoRenderers::iterator iter = video_renderers_.begin();
        iter != video_renderers_.end(); ++iter) {
-    if (video_renderer == iter->renderer) {
+    if (video_renderer == *iter) {
       video_renderers_.erase(iter);
-      return true;
+      break;
     }
   }
-  return false;
 }
 
 void CaptureRenderAdapter::Init() {
   video_capturer_->SignalVideoFrame.connect(
       this,
       &CaptureRenderAdapter::OnVideoFrame);
 }
 
 void CaptureRenderAdapter::OnVideoFrame(VideoCapturer* capturer,
                                         const VideoFrame* video_frame) {
   rtc::CritScope cs(&capture_crit_);
   if (video_renderers_.empty()) {
     return;
   }
-  MaybeSetRenderingSize(video_frame);
 
-  for (VideoRenderers::iterator iter = video_renderers_.begin();
-       iter != video_renderers_.end(); ++iter) {
-    VideoRenderer* video_renderer = iter->renderer;
-    video_renderer->RenderFrame(video_frame);
-  }
+  for (auto* renderer : video_renderers_)
+    renderer->RenderFrame(video_frame);
 }
 
 // The renderer_crit_ lock needs to be taken when calling this function.
-void CaptureRenderAdapter::MaybeSetRenderingSize(const VideoFrame* frame) {
-  for (VideoRenderers::iterator iter = video_renderers_.begin();
-       iter != video_renderers_.end(); ++iter) {
-    const bool new_resolution = iter->render_width != frame->GetWidth() ||
-        iter->render_height != frame->GetHeight();
-    if (new_resolution) {
-      iter->render_width = frame->GetWidth();
-      iter->render_height = frame->GetHeight();
-    }
-  }
-}
-
-// The renderer_crit_ lock needs to be taken when calling this function.
 bool CaptureRenderAdapter::IsRendererRegistered(
     const VideoRenderer& video_renderer) const {
   for (VideoRenderers::const_iterator iter = video_renderers_.begin();
        iter != video_renderers_.end(); ++iter) {

[tommi, 2016/01/21 08:45:34]

nit: you could update this loop too, but this method will likely be going away. 
can be replaced by std::find if needed.

-    if (&video_renderer == iter->renderer) {
+    if (&video_renderer == *iter) {
       return true;
     }
   }
   return false;
 }
 
 }  // namespace cricket