From e5c4a810e21e74acec09c987f20fb0e6788062fd Mon Sep 17 00:00:00 2001 From: sprang Date: Tue, 11 Jul 2017 03:44:17 -0700 Subject: [PATCH] Move RTP keep-alive config from VideoSendStream::Config to Call::Config This makes more sense since logically it's a transport level feature, not a media stream feature. Even if the implementation details forces it to be an rtp stream detail, for the moment. BUG=webrtc:7907 Review-Url: https://codereview.webrtc.org/2978503002 Cr-Commit-Position: refs/heads/master@{#18963} --- webrtc/call/call.cc | 3 ++- webrtc/call/call.h | 6 ++++++ webrtc/video/video_send_stream.cc | 22 ++++++++++++++-------- webrtc/video/video_send_stream.h | 3 ++- webrtc/video/video_send_stream_tests.cc | 17 ++++++++--------- webrtc/video_send_stream.h | 2 -- 6 files changed, 32 insertions(+), 21 deletions(-) diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc index 41f53feee7..3966d5e030 100644 --- a/webrtc/call/call.cc +++ b/webrtc/call/call.cc @@ -727,7 +727,8 @@ webrtc::VideoSendStream* Call::CreateVideoSendStream( num_cpu_cores_, module_process_thread_.get(), &worker_queue_, call_stats_.get(), transport_send_.get(), bitrate_allocator_.get(), video_send_delay_stats_.get(), event_log_, std::move(config), - std::move(encoder_config), suspended_video_send_ssrcs_); + std::move(encoder_config), suspended_video_send_ssrcs_, + config_.keepalive_config); { WriteLockScoped write_lock(*send_crit_); diff --git a/webrtc/call/call.h b/webrtc/call/call.h index 86142f0fea..f99ba851ab 100644 --- a/webrtc/call/call.h +++ b/webrtc/call/call.h @@ -112,6 +112,12 @@ class Call { // RtcEventLog to use for this call. Required. // Use webrtc::RtcEventLog::CreateNull() for a null implementation. RtcEventLog* event_log = nullptr; + + // Enables periodic sending if empty keep-alive messages that helps prevent + // network time-out events. The packets adhere to RFC6263 section 4.6, and + // by default use payload type 20, as described in 3GPP TS 24.229, + // Appendix K.5.2.1. + RtpKeepAliveConfig keepalive_config; }; struct Stats { diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index 3565f9ca70..9bfc9854f3 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -349,7 +349,8 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver, const VideoSendStream::Config* config, int initial_encoder_max_bitrate, std::map suspended_ssrcs, - VideoEncoderConfig::ContentType content_type); + VideoEncoderConfig::ContentType content_type, + const RtpKeepAliveConfig& keepalive_config); ~VideoSendStreamImpl() override; // RegisterProcessThread register |module_process_thread| with those objects @@ -480,7 +481,8 @@ class VideoSendStream::ConstructionTask : public rtc::QueuedTask { const VideoSendStream::Config* config, int initial_encoder_max_bitrate, const std::map& suspended_ssrcs, - VideoEncoderConfig::ContentType content_type) + VideoEncoderConfig::ContentType content_type, + const RtpKeepAliveConfig& keepalive_config) : send_stream_(send_stream), done_event_(done_event), stats_proxy_(stats_proxy), @@ -493,7 +495,8 @@ class VideoSendStream::ConstructionTask : public rtc::QueuedTask { config_(config), initial_encoder_max_bitrate_(initial_encoder_max_bitrate), suspended_ssrcs_(suspended_ssrcs), - content_type_(content_type) {} + content_type_(content_type), + keepalive_config_(keepalive_config) {} ~ConstructionTask() override { done_event_->Set(); } @@ -503,7 +506,7 @@ class VideoSendStream::ConstructionTask : public rtc::QueuedTask { stats_proxy_, rtc::TaskQueue::Current(), call_stats_, transport_, bitrate_allocator_, send_delay_stats_, vie_encoder_, event_log_, config_, initial_encoder_max_bitrate_, std::move(suspended_ssrcs_), - content_type_)); + content_type_, keepalive_config_)); return true; } @@ -520,6 +523,7 @@ class VideoSendStream::ConstructionTask : public rtc::QueuedTask { int initial_encoder_max_bitrate_; std::map suspended_ssrcs_; const VideoEncoderConfig::ContentType content_type_; + const RtpKeepAliveConfig& keepalive_config_; }; class VideoSendStream::DestructAndGetRtpStateTask : public rtc::QueuedTask { @@ -632,7 +636,8 @@ VideoSendStream::VideoSendStream( RtcEventLog* event_log, VideoSendStream::Config config, VideoEncoderConfig encoder_config, - const std::map& suspended_ssrcs) + const std::map& suspended_ssrcs, + const RtpKeepAliveConfig& keepalive_config) : worker_queue_(worker_queue), thread_sync_event_(false /* manual_reset */, false), stats_proxy_(Clock::GetRealTimeClock(), @@ -648,7 +653,7 @@ VideoSendStream::VideoSendStream( &send_stream_, &thread_sync_event_, &stats_proxy_, vie_encoder_.get(), module_process_thread, call_stats, transport, bitrate_allocator, send_delay_stats, event_log, &config_, encoder_config.max_bitrate_bps, - suspended_ssrcs, encoder_config.content_type))); + suspended_ssrcs, encoder_config.content_type, keepalive_config))); // Wait for ConstructionTask to complete so that |send_stream_| can be used. // |module_process_thread| must be registered and deregistered on the thread @@ -767,7 +772,8 @@ VideoSendStreamImpl::VideoSendStreamImpl( const VideoSendStream::Config* config, int initial_encoder_max_bitrate, std::map suspended_ssrcs, - VideoEncoderConfig::ContentType content_type) + VideoEncoderConfig::ContentType content_type, + const RtpKeepAliveConfig& keepalive_config) : send_side_bwe_with_overhead_( webrtc::field_trial::IsEnabled("WebRTC-SendSideBwe-WithOverhead")), stats_proxy_(stats_proxy), @@ -805,7 +811,7 @@ VideoSendStreamImpl::VideoSendStreamImpl( transport->send_side_cc()->GetRetransmissionRateLimiter(), this, config_->rtp.ssrcs.size(), - config_->rtp.keep_alive)), + keepalive_config)), payload_router_(rtp_rtcp_modules_, config_->encoder_settings.payload_type), weak_ptr_factory_(this), diff --git a/webrtc/video/video_send_stream.h b/webrtc/video/video_send_stream.h index 2f1fec0357..59cb7973f7 100644 --- a/webrtc/video/video_send_stream.h +++ b/webrtc/video/video_send_stream.h @@ -57,7 +57,8 @@ class VideoSendStream : public webrtc::VideoSendStream { RtcEventLog* event_log, VideoSendStream::Config config, VideoEncoderConfig encoder_config, - const std::map& suspended_ssrcs); + const std::map& suspended_ssrcs, + const RtpKeepAliveConfig& keepalive_config); ~VideoSendStream() override; diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index aa9d2fb093..53796b8187 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -3415,6 +3415,14 @@ TEST_F(VideoSendStreamTest, SendsKeepAlive) { public: KeepaliveObserver() : SendTest(kDefaultTimeoutMs) {} + Call::Config GetSenderCallConfig() override { + Call::Config config = SendTest::GetSenderCallConfig(); + config.keepalive_config.timeout_interval_ms = kTimeoutMs; + config.keepalive_config.payload_type = + CallTest::kDefaultKeepalivePayloadType; + return config; + } + private: Action OnSendRtp(const uint8_t* packet, size_t length) override { RTPHeader header; @@ -3431,15 +3439,6 @@ TEST_F(VideoSendStreamTest, SendsKeepAlive) { return SEND_PACKET; } - void ModifyVideoConfigs( - VideoSendStream::Config* send_config, - std::vector* receive_configs, - VideoEncoderConfig* encoder_config) override { - send_config->rtp.keep_alive.timeout_interval_ms = kTimeoutMs; - send_config->rtp.keep_alive.payload_type = - CallTest::kDefaultKeepalivePayloadType; - } - void PerformTest() override { EXPECT_TRUE(Wait()) << "Timed out while waiting for keep-alive packet."; } diff --git a/webrtc/video_send_stream.h b/webrtc/video_send_stream.h index 2b3ee78c20..c5a1f9b647 100644 --- a/webrtc/video_send_stream.h +++ b/webrtc/video_send_stream.h @@ -168,8 +168,6 @@ class VideoSendStream { int payload_type = -1; } rtx; - RtpKeepAliveConfig keep_alive; - // RTCP CNAME, see RFC 3550. std::string c_name; } rtp;