From 13d38a13e3a6b55d0a95bbac32963014bd0a2002 Mon Sep 17 00:00:00 2001 From: "pbos@webrtc.org" Date: Thu, 28 Nov 2013 11:59:31 +0000 Subject: [PATCH] Set up SSRCs correctly after switching codec. Before SSRCs were not set up correctly, as the old VideoEngine API doesn't support setting additional SSRCs before a codec with as many streams are set. No test was in place to catch this, so two tests are added to make sure that we send the SSRCs that are set, and also that we can switch from using one to using all SSRCs, even though initially not all of them are set up. BUG= R=mflodman@webrtc.org Review URL: https://webrtc-codereview.appspot.com/4539004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@5188 4adac7df-926f-26a2-2b94-8c16560cd09d --- webrtc/call.cc | 3 +- webrtc/test/rtp_rtcp_observer.h | 4 +- webrtc/video/video_send_stream.cc | 51 +++++++------- webrtc/video/video_send_stream_tests.cc | 94 ++++++++++++++++++++++--- 4 files changed, 114 insertions(+), 38 deletions(-) diff --git a/webrtc/call.cc b/webrtc/call.cc index 2ba169277..9e23b24cc 100644 --- a/webrtc/call.cc +++ b/webrtc/call.cc @@ -215,8 +215,7 @@ VideoSendStream::Config Call::GetDefaultSendConfig() { VideoSendStream* Call::CreateVideoSendStream( const VideoSendStream::Config& config) { assert(config.rtp.ssrcs.size() > 0); - assert(config.codec.numberOfSimulcastStreams == 0 || - config.codec.numberOfSimulcastStreams == config.rtp.ssrcs.size()); + assert(config.rtp.ssrcs.size() >= config.codec.numberOfSimulcastStreams); VideoSendStream* send_stream = new VideoSendStream( config_.send_transport, config_.overuse_detection, video_engine_, config); diff --git a/webrtc/test/rtp_rtcp_observer.h b/webrtc/test/rtp_rtcp_observer.h index f82d27b76..56c96fae7 100644 --- a/webrtc/test/rtp_rtcp_observer.h +++ b/webrtc/test/rtp_rtcp_observer.h @@ -43,7 +43,9 @@ class RtpRtcpObserver { } virtual EventTypeWrapper Wait() { - return observation_complete_->Wait(timeout_ms_); + EventTypeWrapper result = observation_complete_->Wait(timeout_ms_); + observation_complete_->Reset(); + return result; } protected: diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index b5fec72d9..d90c733c1 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -95,33 +95,9 @@ VideoSendStream::VideoSendStream(newapi::Transport* transport, assert(rtp_rtcp_ != NULL); assert(config_.rtp.ssrcs.size() > 0); - if (config_.rtp.ssrcs.size() == 1) { - rtp_rtcp_->SetLocalSSRC(channel_, config_.rtp.ssrcs[0]); - } else { - for (size_t i = 0; i < config_.rtp.ssrcs.size(); ++i) { - rtp_rtcp_->SetLocalSSRC(channel_, - config_.rtp.ssrcs[i], - kViEStreamTypeNormal, - static_cast(i)); - } - } if (config_.suspend_below_min_bitrate) config_.pacing = true; rtp_rtcp_->SetTransmissionSmoothingStatus(channel_, config_.pacing); - if (!config_.rtp.rtx.ssrcs.empty()) { - assert(config_.rtp.rtx.ssrcs.size() == config_.rtp.ssrcs.size()); - for (size_t i = 0; i < config_.rtp.rtx.ssrcs.size(); ++i) { - rtp_rtcp_->SetLocalSSRC(channel_, - config_.rtp.rtx.ssrcs[i], - kViEStreamTypeRtx, - static_cast(i)); - } - - if (config_.rtp.rtx.rtx_payload_type != 0) { - rtp_rtcp_->SetRtxSendPayloadType(channel_, - config_.rtp.rtx.rtx_payload_type); - } - } for (size_t i = 0; i < config_.rtp.extensions.size(); ++i) { const std::string& extension = config_.rtp.extensions[i].name; @@ -279,14 +255,37 @@ void VideoSendStream::StopSending() { } bool VideoSendStream::SetCodec(const VideoCodec& codec) { - if (codec.numberOfSimulcastStreams > 0) - assert(config_.rtp.ssrcs.size() >= codec.numberOfSimulcastStreams); + assert(config_.rtp.ssrcs.size() >= codec.numberOfSimulcastStreams); CriticalSectionScoped crit(codec_lock_.get()); if (codec_->SetSendCodec(channel_, codec) != 0) return false; + for (size_t i = 0; i < config_.rtp.ssrcs.size(); ++i) { + rtp_rtcp_->SetLocalSSRC(channel_, + config_.rtp.ssrcs[i], + kViEStreamTypeNormal, + static_cast(i)); + } + config_.codec = codec; + if (config_.rtp.rtx.ssrcs.empty()) + return true; + + // Set up RTX. + assert(config_.rtp.rtx.ssrcs.size() == config_.rtp.ssrcs.size()); + for (size_t i = 0; i < config_.rtp.ssrcs.size(); ++i) { + rtp_rtcp_->SetLocalSSRC(channel_, + config_.rtp.rtx.ssrcs[i], + kViEStreamTypeRtx, + static_cast(i)); + } + + if (config_.rtp.rtx.rtx_payload_type != 0) { + rtp_rtcp_->SetRtxSendPayloadType(channel_, + config_.rtp.rtx.rtx_payload_type); + } + return true; } diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index 9cd2350ec..d46a72c2f 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -74,6 +74,8 @@ class VideoSendStreamTest : public ::testing::Test { uint8_t retransmit_payload_type, bool enable_pacing); + void SendsSetSsrcs(size_t num_ssrcs, bool send_single_ssrc_first); + enum { kNumSendSsrcs = 3 }; static const uint8_t kSendPayloadType; static const uint8_t kSendRtxPayloadType; @@ -95,30 +97,104 @@ const uint32_t VideoSendStreamTest::kSendSsrcs[kNumSendSsrcs] = { 0xC0FFED, const uint32_t VideoSendStreamTest::kSendSsrc = VideoSendStreamTest::kSendSsrcs[0]; -TEST_F(VideoSendStreamTest, SendsSetSsrc) { +void VideoSendStreamTest::SendsSetSsrcs(size_t num_ssrcs, + bool send_single_ssrc_first) { class SendSsrcObserver : public test::RtpRtcpObserver { public: - SendSsrcObserver() : RtpRtcpObserver(30 * 1000) {} + SendSsrcObserver(const uint32_t* ssrcs, + size_t num_ssrcs, + bool send_single_ssrc_first) + : RtpRtcpObserver(30 * 1000), + ssrcs_to_observe_(num_ssrcs), + expect_single_ssrc_(send_single_ssrc_first) { + for (size_t i = 0; i < num_ssrcs; ++i) + valid_ssrcs_[ssrcs[i]] = true; + } virtual Action OnSendRtp(const uint8_t* packet, size_t length) OVERRIDE { RTPHeader header; - EXPECT_TRUE( - parser_->Parse(packet, static_cast(length), &header)); + EXPECT_TRUE(parser_->Parse(packet, static_cast(length), &header)); - if (header.ssrc == kSendSsrc) + EXPECT_TRUE(valid_ssrcs_[header.ssrc]) + << "Received unknown SSRC: " << header.ssrc; + + if (!valid_ssrcs_[header.ssrc]) + observation_complete_->Set(); + + if (!is_observed_[header.ssrc]) { + is_observed_[header.ssrc] = true; + --ssrcs_to_observe_; + if (expect_single_ssrc_) { + expect_single_ssrc_ = false; + observation_complete_->Set(); + } + } + + if (ssrcs_to_observe_ == 0) observation_complete_->Set(); return SEND_PACKET; } - } observer; + + private: + std::map valid_ssrcs_; + std::map is_observed_; + size_t ssrcs_to_observe_; + bool expect_single_ssrc_; + } observer(kSendSsrcs, num_ssrcs, send_single_ssrc_first); Call::Config call_config(observer.SendTransport()); scoped_ptr call(Call::Create(call_config)); - VideoSendStream::Config send_config = GetSendTestConfig(call.get(), 1); - send_config.rtp.max_packet_size = 128; + VideoSendStream::Config send_config = + GetSendTestConfig(call.get(), num_ssrcs); - RunSendTest(call.get(), send_config, &observer); + if (num_ssrcs > 1) { + // Set low simulcast bitrates to not have to wait for bandwidth ramp-up. + for (size_t i = 0; i < num_ssrcs; ++i) { + send_config.codec.simulcastStream[i].minBitrate = 10; + send_config.codec.simulcastStream[i].targetBitrate = 10; + send_config.codec.simulcastStream[i].maxBitrate = 10; + } + } + + if (send_single_ssrc_first) + send_config.codec.numberOfSimulcastStreams = 1; + + send_stream_ = call->CreateVideoSendStream(send_config); + scoped_ptr frame_generator_capturer( + test::FrameGeneratorCapturer::Create( + send_stream_->Input(), 320, 240, 30, Clock::GetRealTimeClock())); + send_stream_->StartSending(); + frame_generator_capturer->Start(); + + EXPECT_EQ(kEventSignaled, observer.Wait()) + << "Timed out while waiting for " + << (send_single_ssrc_first ? "first SSRC." : "SSRCs."); + + if (send_single_ssrc_first) { + // Set full simulcast and continue with the rest of the SSRCs. + send_config.codec.numberOfSimulcastStreams = + static_cast(num_ssrcs); + send_stream_->SetCodec(send_config.codec); + EXPECT_EQ(kEventSignaled, observer.Wait()) + << "Timed out while waiting on additional SSRCs."; + } + + observer.StopSending(); + frame_generator_capturer->Stop(); + send_stream_->StopSending(); + call->DestroyVideoSendStream(send_stream_); +} + +TEST_F(VideoSendStreamTest, SendsSetSsrc) { SendsSetSsrcs(1, false); } + +TEST_F(VideoSendStreamTest, SendsSetSimulcastSsrcs) { + SendsSetSsrcs(kNumSendSsrcs, false); +} + +TEST_F(VideoSendStreamTest, CanSwitchToUseAllSsrcs) { + SendsSetSsrcs(kNumSendSsrcs, true); } TEST_F(VideoSendStreamTest, SupportsCName) {