From 760a076a5293b65d70c4f3cd171e148439b3608a Mon Sep 17 00:00:00 2001 From: eladalon Date: Wed, 31 May 2017 09:12:25 -0700 Subject: [PATCH] Create unit tests for RtpDemuxer 1. Create unit tests for RtpDemuxer. 2. Add an RTC_DCHECK in RtpDemuxer that makes sure that the sink<->ssrc multimap does not allow multiple instances of the same association. BUG=None Review-Url: https://codereview.webrtc.org/2902823004 Cr-Commit-Position: refs/heads/master@{#18357} --- webrtc/BUILD.gn | 3 + webrtc/call/BUILD.gn | 1 + webrtc/call/rtp_demuxer.cc | 15 +++ webrtc/call/rtp_demuxer_unittest.cc | 147 ++++++++++++++++++++++++++++ 4 files changed, 166 insertions(+) create mode 100644 webrtc/call/rtp_demuxer_unittest.cc diff --git a/webrtc/BUILD.gn b/webrtc/BUILD.gn index 38112211e0..f8e0e8de15 100644 --- a/webrtc/BUILD.gn +++ b/webrtc/BUILD.gn @@ -435,6 +435,9 @@ if (rtc_include_tests) { deps = [ "audio:audio_tests", "base:rtc_base_tests_utils", + + # TODO(eladalon): call_tests aren't actually video-specific, so we + # should move them to a more appropriate test suite. "call:call_tests", "modules/video_capture", "test:test_common", diff --git a/webrtc/call/BUILD.gn b/webrtc/call/BUILD.gn index a6afe30240..8360534f41 100644 --- a/webrtc/call/BUILD.gn +++ b/webrtc/call/BUILD.gn @@ -89,6 +89,7 @@ if (rtc_include_tests) { "bitrate_estimator_tests.cc", "call_unittest.cc", "flexfec_receive_stream_unittest.cc", + "rtp_demuxer_unittest.cc", "rtx_receive_stream_unittest.cc", ] deps = [ diff --git a/webrtc/call/rtp_demuxer.cc b/webrtc/call/rtp_demuxer.cc index aea414e08d..d7179c04ac 100644 --- a/webrtc/call/rtp_demuxer.cc +++ b/webrtc/call/rtp_demuxer.cc @@ -14,6 +14,20 @@ namespace webrtc { +namespace { + +template +bool MultimapAssociationExists(const std::multimap& multimap, + Key key, + Value val) { + auto it_range = multimap.equal_range(key); + using Reference = typename std::multimap::const_reference; + return std::any_of(it_range.first, it_range.second, + [val](Reference elem) { return elem.second == val; }); +} + +} // namespace + RtpDemuxer::RtpDemuxer() {} RtpDemuxer::~RtpDemuxer() { @@ -22,6 +36,7 @@ RtpDemuxer::~RtpDemuxer() { void RtpDemuxer::AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink) { RTC_DCHECK(sink); + RTC_DCHECK(!MultimapAssociationExists(sinks_, ssrc, sink)); sinks_.emplace(ssrc, sink); } diff --git a/webrtc/call/rtp_demuxer_unittest.cc b/webrtc/call/rtp_demuxer_unittest.cc new file mode 100644 index 0000000000..6aee11e763 --- /dev/null +++ b/webrtc/call/rtp_demuxer_unittest.cc @@ -0,0 +1,147 @@ +/* + * Copyright (c) 2017 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/call/rtp_demuxer.h" + +#include + +#include "webrtc/base/arraysize.h" +#include "webrtc/base/checks.h" +#include "webrtc/base/ptr_util.h" +#include "webrtc/modules/rtp_rtcp/source/rtp_packet_received.h" +#include "webrtc/test/gmock.h" +#include "webrtc/test/gtest.h" + +namespace webrtc { + +namespace { + +using ::testing::_; + +class MockRtpPacketSink : public RtpPacketSinkInterface { + public: + MOCK_METHOD1(OnRtpPacket, void(const RtpPacketReceived&)); +}; + +constexpr uint32_t kSsrcs[] = {101, 202, 303}; + +MATCHER_P(SsrcSameAsIn, other, "") { + return arg.Ssrc() == other.Ssrc(); +} + +std::unique_ptr CreateRtpPacketReceived(uint32_t ssrc) { + auto packet = rtc::MakeUnique(); + packet->SetSsrc(ssrc); + return packet; +} + +class RtpDemuxerTest : public ::testing::Test { + protected: + RtpDemuxerTest() { + for (size_t i = 0; i < arraysize(sinks); i++) { + demuxer.AddSink(kSsrcs[i], &sinks[i]); + } + } + + ~RtpDemuxerTest() override { + for (auto& sink : sinks) { + EXPECT_EQ(demuxer.RemoveSink(&sink), 1u); + } + } + + RtpDemuxer demuxer; + MockRtpPacketSink sinks[arraysize(kSsrcs)]; +}; + +TEST_F(RtpDemuxerTest, OnRtpPacketCalledOnCorrectSink) { + for (size_t i = 0; i < arraysize(sinks); i++) { + auto packet = CreateRtpPacketReceived(kSsrcs[i]); + EXPECT_CALL(sinks[i], OnRtpPacket(SsrcSameAsIn(*packet))); + demuxer.OnRtpPacket(*packet); + } +} + +TEST_F(RtpDemuxerTest, MultipleSinksMappedToSameSsrc) { + // |sinks| associated with different SSRCs each. Add a few additional sinks + // that are all associated with one new, distinct SSRC. + MockRtpPacketSink same_ssrc_sinks[arraysize(sinks)]; + constexpr uint32_t kSharedSsrc = 404; + for (auto& sink : same_ssrc_sinks) { + demuxer.AddSink(kSharedSsrc, &sink); + } + + // Reception of an RTP packet associated with the shared SSRC triggers the + // callback on all of the interfaces associated with it. + auto packet = CreateRtpPacketReceived(kSharedSsrc); + for (auto& sink : same_ssrc_sinks) { + EXPECT_CALL(sink, OnRtpPacket(SsrcSameAsIn(*packet))); + } + demuxer.OnRtpPacket(*packet); + + // Test-specific tear-down + for (auto& sink : same_ssrc_sinks) { + EXPECT_EQ(demuxer.RemoveSink(&sink), 1u); + } +} + +TEST_F(RtpDemuxerTest, SinkMappedToMultipleSsrcs) { + // |sinks| associated with different SSRCs each. We set one of them to also + // be mapped to additional SSRCs. + constexpr uint32_t kSsrcsOfMultiSsrcSink[] = {404, 505, 606}; + MockRtpPacketSink multi_ssrc_sink; + for (uint32_t ssrc : kSsrcsOfMultiSsrcSink) { + demuxer.AddSink(ssrc, &multi_ssrc_sink); + } + + // The sink which is associated with multiple SSRCs gets the callback + // triggered for each of those SSRCs. + for (uint32_t ssrc : kSsrcsOfMultiSsrcSink) { + auto packet = CreateRtpPacketReceived(ssrc); + EXPECT_CALL(multi_ssrc_sink, OnRtpPacket(SsrcSameAsIn(*packet))); + demuxer.OnRtpPacket(*packet); + } + + // Test-specific tear-down + EXPECT_EQ(demuxer.RemoveSink(&multi_ssrc_sink), + arraysize(kSsrcsOfMultiSsrcSink)); +} + +TEST_F(RtpDemuxerTest, OnRtpPacketNotCalledOnRemovedSinks) { + // |sinks| associated with different SSRCs each. We set one of them to also + // be mapped to additional SSRCs. + constexpr uint32_t kSsrcsOfMultiSsrcSink[] = {404, 505, 606}; + MockRtpPacketSink multi_ssrc_sink; + for (uint32_t ssrc : kSsrcsOfMultiSsrcSink) { + demuxer.AddSink(ssrc, &multi_ssrc_sink); + } + + // Remove the sink. + EXPECT_EQ(demuxer.RemoveSink(&multi_ssrc_sink), + arraysize(kSsrcsOfMultiSsrcSink)); + + // The removed sink does not get callbacks triggered for any of the SSRCs + // with which it was previously associated. + EXPECT_CALL(multi_ssrc_sink, OnRtpPacket(_)).Times(0); + for (uint32_t ssrc : kSsrcsOfMultiSsrcSink) { + auto packet = CreateRtpPacketReceived(ssrc); + demuxer.OnRtpPacket(*packet); + } +} + +#if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID) +TEST_F(RtpDemuxerTest, RepeatedAssociationsForbidden) { + // Set-up already associated sinks[0] with kSsrcs[0]. Repeating the + // association is an error. + EXPECT_DEATH(demuxer.AddSink(kSsrcs[0], &sinks[0]), ""); +} +#endif + +} // namespace +} // namespace webrtc