From 0b98cf72c6ca36edb96b8b8d1ab6dfd231032d5c Mon Sep 17 00:00:00 2001 From: nisse Date: Thu, 21 Jan 2016 03:04:30 -0800 Subject: [PATCH] Delete CaptureRenderAdapter::VideoRenderInfo struct, it is unused since the recent deletion of SetSize. Delete methods MaybeSetRenderingSize and IsRendererRegistered, the latter replaced by std::find. Delete return values from AddRenderer and RemoveRenderer. BUG=webrtc:5426 Review URL: https://codereview.webrtc.org/1603423002 Cr-Commit-Position: refs/heads/master@{#11332} --- talk/media/base/capturemanager.cc | 6 +- talk/media/base/capturemanager_unittest.cc | 2 - talk/media/base/capturerenderadapter.cc | 66 +++++++--------------- talk/media/base/capturerenderadapter.h | 20 +------ 4 files changed, 26 insertions(+), 68 deletions(-) diff --git a/talk/media/base/capturemanager.cc b/talk/media/base/capturemanager.cc index b7cbbf22ce..dab33fd2bc 100644 --- a/talk/media/base/capturemanager.cc +++ b/talk/media/base/capturemanager.cc @@ -297,7 +297,8 @@ bool CaptureManager::AddVideoRenderer(VideoCapturer* video_capturer, if (!adapter) { return false; } - return adapter->AddRenderer(video_renderer); + adapter->AddRenderer(video_renderer); + return true; } bool CaptureManager::RemoveVideoRenderer(VideoCapturer* video_capturer, @@ -310,7 +311,8 @@ bool CaptureManager::RemoveVideoRenderer(VideoCapturer* video_capturer, if (!adapter) { return false; } - return adapter->RemoveRenderer(video_renderer); + adapter->RemoveRenderer(video_renderer); + return true; } bool CaptureManager::IsCapturerRegistered(VideoCapturer* video_capturer) const { diff --git a/talk/media/base/capturemanager_unittest.cc b/talk/media/base/capturemanager_unittest.cc index 84086abae4..709ec5bb8d 100644 --- a/talk/media/base/capturemanager_unittest.cc +++ b/talk/media/base/capturemanager_unittest.cc @@ -103,8 +103,6 @@ TEST_F(CaptureManagerTest, InvalidAddingRemoving) { EXPECT_EQ_WAIT(cricket::CS_RUNNING, capture_state(), kMsCallbackWait); EXPECT_EQ(1, callback_count()); EXPECT_FALSE(capture_manager_.AddVideoRenderer(&video_capturer_, NULL)); - EXPECT_FALSE(capture_manager_.RemoveVideoRenderer(&video_capturer_, - &video_renderer_)); EXPECT_TRUE(capture_manager_.StopVideoCapture(&video_capturer_, format_vga_)); } diff --git a/talk/media/base/capturerenderadapter.cc b/talk/media/base/capturerenderadapter.cc index 3fdb68be9b..e5e8f4c58b 100644 --- a/talk/media/base/capturerenderadapter.cc +++ b/talk/media/base/capturerenderadapter.cc @@ -61,31 +61,32 @@ CaptureRenderAdapter* CaptureRenderAdapter::Create( 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? + if (std::find(video_renderers_.begin(), video_renderers_.end(), + video_renderer) == video_renderers_.end()) + 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() { @@ -100,38 +101,9 @@ void CaptureRenderAdapter::OnVideoFrame(VideoCapturer* capturer, 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); - } -} - -// 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) { - if (&video_renderer == iter->renderer) { - return true; - } - } - return false; + for (auto* renderer : video_renderers_) + renderer->RenderFrame(video_frame); } } // namespace cricket diff --git a/talk/media/base/capturerenderadapter.h b/talk/media/base/capturerenderadapter.h index ec9c60c2ad..dda43f0871 100644 --- a/talk/media/base/capturerenderadapter.h +++ b/talk/media/base/capturerenderadapter.h @@ -51,24 +51,14 @@ class CaptureRenderAdapter : public sigslot::has_slots<> { static CaptureRenderAdapter* Create(VideoCapturer* video_capturer); ~CaptureRenderAdapter(); - bool AddRenderer(VideoRenderer* video_renderer); - bool RemoveRenderer(VideoRenderer* video_renderer); + void AddRenderer(VideoRenderer* video_renderer); + void RemoveRenderer(VideoRenderer* video_renderer); VideoCapturer* video_capturer() { return video_capturer_; } private: - struct VideoRendererInfo { - explicit VideoRendererInfo(VideoRenderer* r) - : renderer(r), - render_width(0), - render_height(0) { - } - VideoRenderer* renderer; - size_t render_width; - size_t render_height; - }; // Just pointers since ownership is not handed over to this class. - typedef std::vector VideoRenderers; + typedef std::vector VideoRenderers; explicit CaptureRenderAdapter(VideoCapturer* video_capturer); void Init(); @@ -76,10 +66,6 @@ class CaptureRenderAdapter : public sigslot::has_slots<> { // Callback for frames received from the capturer. void OnVideoFrame(VideoCapturer* capturer, const VideoFrame* video_frame); - void MaybeSetRenderingSize(const VideoFrame* frame); - - bool IsRendererRegistered(const VideoRenderer& video_renderer) const; - VideoRenderers video_renderers_; VideoCapturer* video_capturer_; // Critical section synchronizing the capture thread.