From 60bb6fe37acf344f47e5a951a2f38a9816a6b633 Mon Sep 17 00:00:00 2001 From: Rasmus Brandt Date: Mon, 5 Feb 2018 09:51:47 +0100 Subject: [PATCH] Protect VideoReceiveStream<->FlexfecReceiveStream sink association with unit test. BUG=none Change-Id: Id0c504f62d70febc5e846657dc2966f5e9acef39 Reviewed-on: https://webrtc-review.googlesource.com/17301 Reviewed-by: Magnus Jedvert Commit-Queue: Rasmus Brandt Cr-Commit-Position: refs/heads/master@{#21972} --- media/engine/fakewebrtccall.cc | 21 +++++++++++++--- media/engine/fakewebrtccall.h | 6 +++++ media/engine/webrtcvideoengine_unittest.cc | 29 +++++++++++++++++++--- 3 files changed, 50 insertions(+), 6 deletions(-) diff --git a/media/engine/fakewebrtccall.cc b/media/engine/fakewebrtccall.cc index 2a5f08678d..d7826509ff 100644 --- a/media/engine/fakewebrtccall.cc +++ b/media/engine/fakewebrtccall.cc @@ -308,7 +308,10 @@ void FakeVideoSendStream::InjectVideoSinkWants( FakeVideoReceiveStream::FakeVideoReceiveStream( webrtc::VideoReceiveStream::Config config) - : config_(std::move(config)), receiving_(false) {} + : config_(std::move(config)), + receiving_(false), + num_added_secondary_sinks_(0), + num_removed_secondary_sinks_(0) {} const webrtc::VideoReceiveStream::Config& FakeVideoReceiveStream::GetConfig() const { @@ -346,10 +349,22 @@ void FakeVideoReceiveStream::EnableEncodedFrameRecording(rtc::PlatformFile file, } void FakeVideoReceiveStream::AddSecondarySink( - webrtc::RtpPacketSinkInterface* sink) {} + webrtc::RtpPacketSinkInterface* sink) { + ++num_added_secondary_sinks_; +} void FakeVideoReceiveStream::RemoveSecondarySink( - const webrtc::RtpPacketSinkInterface* sink) {} + const webrtc::RtpPacketSinkInterface* sink) { + ++num_removed_secondary_sinks_; +} + +int FakeVideoReceiveStream::GetNumAddedSecondarySinks() const { + return num_added_secondary_sinks_; +} + +int FakeVideoReceiveStream::GetNumRemovedSecondarySinks() const { + return num_removed_secondary_sinks_; +} FakeFlexfecReceiveStream::FakeFlexfecReceiveStream( const webrtc::FlexfecReceiveStream::Config& config) diff --git a/media/engine/fakewebrtccall.h b/media/engine/fakewebrtccall.h index 011276588a..9eb238a6fa 100644 --- a/media/engine/fakewebrtccall.h +++ b/media/engine/fakewebrtccall.h @@ -206,6 +206,9 @@ class FakeVideoReceiveStream final : public webrtc::VideoReceiveStream { void AddSecondarySink(webrtc::RtpPacketSinkInterface* sink) override; void RemoveSecondarySink(const webrtc::RtpPacketSinkInterface* sink) override; + int GetNumAddedSecondarySinks() const; + int GetNumRemovedSecondarySinks() const; + private: // webrtc::VideoReceiveStream implementation. void Start() override; @@ -216,6 +219,9 @@ class FakeVideoReceiveStream final : public webrtc::VideoReceiveStream { webrtc::VideoReceiveStream::Config config_; bool receiving_; webrtc::VideoReceiveStream::Stats stats_; + + int num_added_secondary_sinks_; + int num_removed_secondary_sinks_; }; class FakeFlexfecReceiveStream final : public webrtc::FlexfecReceiveStream { diff --git a/media/engine/webrtcvideoengine_unittest.cc b/media/engine/webrtcvideoengine_unittest.cc index b762225137..1ecc0888db 100644 --- a/media/engine/webrtcvideoengine_unittest.cc +++ b/media/engine/webrtcvideoengine_unittest.cc @@ -2737,6 +2737,13 @@ TEST_F(WebRtcVideoChannelFlexfecRecvTest, SetDefaultRecvCodecsWithoutSsrc) { const std::vector& streams = fake_call_->GetFlexfecReceiveStreams(); EXPECT_TRUE(streams.empty()); + + const std::vector& video_streams = + fake_call_->GetVideoReceiveStreams(); + ASSERT_EQ(1U, video_streams.size()); + const FakeVideoReceiveStream& video_stream = *video_streams.front(); + EXPECT_EQ(0, video_stream.GetNumAddedSecondarySinks()); + EXPECT_EQ(0, video_stream.GetNumRemovedSecondarySinks()); } TEST_F(WebRtcVideoChannelFlexfecRecvTest, SetDefaultRecvCodecsWithSsrc) { @@ -2756,8 +2763,10 @@ TEST_F(WebRtcVideoChannelFlexfecRecvTest, SetDefaultRecvCodecsWithSsrc) { const std::vector& video_streams = fake_call_->GetVideoReceiveStreams(); ASSERT_EQ(1U, video_streams.size()); + const FakeVideoReceiveStream& video_stream = *video_streams.front(); + EXPECT_EQ(1, video_stream.GetNumAddedSecondarySinks()); const webrtc::VideoReceiveStream::Config& video_config = - video_streams.front()->GetConfig(); + video_stream.GetConfig(); EXPECT_TRUE(video_config.rtp.protected_by_flexfec); } @@ -2770,7 +2779,12 @@ TEST_F(WebRtcVideoChannelFlexfecRecvTest, AddRecvStream( CreatePrimaryWithFecFrStreamParams("cname", kSsrcs1[0], kFlexfecSsrc)); EXPECT_EQ(1, fake_call_->GetNumCreatedReceiveStreams()); - EXPECT_EQ(1U, fake_call_->GetVideoReceiveStreams().size()); + const std::vector& video_streams = + fake_call_->GetVideoReceiveStreams(); + ASSERT_EQ(1U, video_streams.size()); + const FakeVideoReceiveStream& video_stream = *video_streams.front(); + EXPECT_EQ(0, video_stream.GetNumAddedSecondarySinks()); + EXPECT_EQ(0, video_stream.GetNumRemovedSecondarySinks()); // Enable FlexFEC. recv_parameters.codecs.push_back(GetEngineCodec("flexfec-03")); @@ -2781,6 +2795,8 @@ TEST_F(WebRtcVideoChannelFlexfecRecvTest, << "Enabling FlexFEC should not create VideoReceiveStream."; EXPECT_EQ(1U, fake_call_->GetFlexfecReceiveStreams().size()) << "Enabling FlexFEC should create a single FlexfecReceiveStream."; + EXPECT_EQ(1, video_stream.GetNumAddedSecondarySinks()); + EXPECT_EQ(0, video_stream.GetNumRemovedSecondarySinks()); } TEST_F(WebRtcVideoChannelFlexfecRecvTest, @@ -2793,8 +2809,13 @@ TEST_F(WebRtcVideoChannelFlexfecRecvTest, AddRecvStream( CreatePrimaryWithFecFrStreamParams("cname", kSsrcs1[0], kFlexfecSsrc)); EXPECT_EQ(2, fake_call_->GetNumCreatedReceiveStreams()); - EXPECT_EQ(1U, fake_call_->GetVideoReceiveStreams().size()); EXPECT_EQ(1U, fake_call_->GetFlexfecReceiveStreams().size()); + const std::vector& video_streams = + fake_call_->GetVideoReceiveStreams(); + ASSERT_EQ(1U, video_streams.size()); + const FakeVideoReceiveStream& video_stream = *video_streams.front(); + EXPECT_EQ(1, video_stream.GetNumAddedSecondarySinks()); + EXPECT_EQ(0, video_stream.GetNumRemovedSecondarySinks()); // Disable FlexFEC. recv_parameters.codecs.clear(); @@ -2806,6 +2827,8 @@ TEST_F(WebRtcVideoChannelFlexfecRecvTest, << "Disabling FlexFEC should not destroy VideoReceiveStream."; EXPECT_TRUE(fake_call_->GetFlexfecReceiveStreams().empty()) << "Disabling FlexFEC should destroy FlexfecReceiveStream."; + EXPECT_EQ(1, video_stream.GetNumAddedSecondarySinks()); + EXPECT_EQ(1, video_stream.GetNumRemovedSecondarySinks()); } // TODO(brandtr): When FlexFEC is no longer behind a field trial, merge all