diff --git a/talk/app/webrtc/peerconnectionfactory_unittest.cc b/talk/app/webrtc/peerconnectionfactory_unittest.cc index 9fb013b54f..1fdf2ebf14 100644 --- a/talk/app/webrtc/peerconnectionfactory_unittest.cc +++ b/talk/app/webrtc/peerconnectionfactory_unittest.cc @@ -360,13 +360,15 @@ TEST_F(PeerConnectionFactoryTest, LocalRendering) { EXPECT_EQ(0, local_renderer.num_rendered_frames()); EXPECT_TRUE(capturer->CaptureFrame()); EXPECT_EQ(1, local_renderer.num_rendered_frames()); + EXPECT_FALSE(local_renderer.black_frame()); track->set_enabled(false); EXPECT_TRUE(capturer->CaptureFrame()); - EXPECT_EQ(1, local_renderer.num_rendered_frames()); + EXPECT_EQ(2, local_renderer.num_rendered_frames()); + EXPECT_TRUE(local_renderer.black_frame()); track->set_enabled(true); EXPECT_TRUE(capturer->CaptureFrame()); - EXPECT_EQ(2, local_renderer.num_rendered_frames()); + EXPECT_EQ(3, local_renderer.num_rendered_frames()); + EXPECT_FALSE(local_renderer.black_frame()); } - diff --git a/talk/app/webrtc/test/fakevideotrackrenderer.h b/talk/app/webrtc/test/fakevideotrackrenderer.h index 38b84a6aff..747ba760fd 100644 --- a/talk/app/webrtc/test/fakevideotrackrenderer.h +++ b/talk/app/webrtc/test/fakevideotrackrenderer.h @@ -57,6 +57,8 @@ class FakeVideoTrackRenderer : public VideoRendererInterface { int errors() const { return fake_renderer_.errors(); } int width() const { return fake_renderer_.width(); } int height() const { return fake_renderer_.height(); } + bool black_frame() const { return fake_renderer_.black_frame(); } + int num_rendered_frames() const { return fake_renderer_.num_rendered_frames(); } diff --git a/talk/app/webrtc/videotrack_unittest.cc b/talk/app/webrtc/videotrack_unittest.cc index c30a98b5e0..fd7b87d30e 100644 --- a/talk/app/webrtc/videotrack_unittest.cc +++ b/talk/app/webrtc/videotrack_unittest.cc @@ -109,3 +109,38 @@ TEST_F(VideoTrackTest, RenderVideo) { EXPECT_EQ(2, renderer_1->num_rendered_frames()); EXPECT_EQ(2, renderer_2->num_rendered_frames()); } + +// Test that disabling the track results in blacked out frames. +TEST_F(VideoTrackTest, DisableTrackBlackout) { + rtc::scoped_ptr renderer( + new FakeVideoTrackRenderer(video_track_.get())); + + cricket::VideoRenderer* renderer_input = + video_track_->GetSource()->FrameInput(); + ASSERT_FALSE(renderer_input == NULL); + + cricket::WebRtcVideoFrame frame; + frame.InitToBlack(100, 200, 1, 1, 0); + // Make it not all-black + frame.GetUPlane()[0] = 0; + + renderer_input->RenderFrame(&frame); + EXPECT_EQ(1, renderer->num_rendered_frames()); + EXPECT_FALSE(renderer->black_frame()); + EXPECT_EQ(100, renderer->width()); + EXPECT_EQ(200, renderer->height()); + + video_track_->set_enabled(false); + renderer_input->RenderFrame(&frame); + EXPECT_EQ(2, renderer->num_rendered_frames()); + EXPECT_TRUE(renderer->black_frame()); + EXPECT_EQ(100, renderer->width()); + EXPECT_EQ(200, renderer->height()); + + video_track_->set_enabled(true); + renderer_input->RenderFrame(&frame); + EXPECT_EQ(3, renderer->num_rendered_frames()); + EXPECT_FALSE(renderer->black_frame()); + EXPECT_EQ(100, renderer->width()); + EXPECT_EQ(200, renderer->height()); +} diff --git a/talk/app/webrtc/videotrackrenderers.cc b/talk/app/webrtc/videotrackrenderers.cc index 3f9301b718..5b890371a5 100644 --- a/talk/app/webrtc/videotrackrenderers.cc +++ b/talk/app/webrtc/videotrackrenderers.cc @@ -26,7 +26,7 @@ */ #include "talk/app/webrtc/videotrackrenderers.h" -#include "talk/media/base/videoframe.h" +#include "talk/media/webrtc/webrtcvideoframe.h" namespace webrtc { @@ -55,14 +55,44 @@ void VideoTrackRenderers::SetEnabled(bool enable) { } bool VideoTrackRenderers::RenderFrame(const cricket::VideoFrame* frame) { - rtc::CritScope cs(&critical_section_); - if (!enabled_) { + { + rtc::CritScope cs(&critical_section_); + if (enabled_) { + RenderFrameToRenderers(frame); + return true; + } + } + + // Generate the black frame outside of the critical section. Note + // that this may result in unexpected frame order, in the unlikely + // case that RenderFrame is called from multiple threads without + // proper serialization, and the track is switched from disabled to + // enabled in the middle of the first call. + cricket::WebRtcVideoFrame black(new rtc::RefCountedObject( + static_cast(frame->GetWidth()), + static_cast(frame->GetHeight())), + frame->GetTimeStamp(), + frame->GetVideoRotation()); + black.SetToBlack(); + + { + rtc::CritScope cs(&critical_section_); + // Check enabled_ flag again, since the track might have been + // enabled while we generated the black frame. I think the + // enabled-ness ought to be applied at the track output, and hence + // an enabled track shouldn't send any blacked out frames. + RenderFrameToRenderers(enabled_ ? frame : &black); + return true; } +} + +// Called with critical_section_ already locked +void VideoTrackRenderers::RenderFrameToRenderers( + const cricket::VideoFrame* frame) { for (VideoRendererInterface* renderer : renderers_) { renderer->RenderFrame(frame); } - return true; } } // namespace webrtc diff --git a/talk/app/webrtc/videotrackrenderers.h b/talk/app/webrtc/videotrackrenderers.h index 3262e22dff..2a73ed4fef 100644 --- a/talk/app/webrtc/videotrackrenderers.h +++ b/talk/app/webrtc/videotrackrenderers.h @@ -47,7 +47,8 @@ class VideoTrackRenderers : public cricket::VideoRenderer { VideoTrackRenderers(); ~VideoTrackRenderers(); - // Implements cricket::VideoRenderer + // Implements cricket::VideoRenderer. If the track is disabled, + // incoming frames are replaced by black frames. virtual bool RenderFrame(const cricket::VideoFrame* frame); void AddRenderer(VideoRendererInterface* renderer); @@ -55,6 +56,10 @@ class VideoTrackRenderers : public cricket::VideoRenderer { void SetEnabled(bool enable); private: + // Pass the frame on to to each registered renderer. Requires + // critical_section_ already locked. + void RenderFrameToRenderers(const cricket::VideoFrame* frame); + bool enabled_; std::set renderers_;