From e950cadba59791f90a6d7abbc7baf583d539f0a4 Mon Sep 17 00:00:00 2001 From: brandtr Date: Tue, 15 Nov 2016 05:25:41 -0800 Subject: [PATCH] Wire up FlexfecSender in RTP module and VideoSendStream. FlexfecSender is owned and configured by VideoSendStream. BUG=webrtc:5654 Review-Url: https://codereview.webrtc.org/2501503003 Cr-Commit-Position: refs/heads/master@{#15082} --- webrtc/modules/rtp_rtcp/include/rtp_rtcp.h | 5 ++ .../modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 2 +- webrtc/video/video_send_stream.cc | 77 ++++++++++++++++++- webrtc/video_send_stream.h | 5 ++ 4 files changed, 86 insertions(+), 3 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h b/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h index a2c7098722..830d895774 100644 --- a/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h @@ -20,6 +20,7 @@ #include "webrtc/base/deprecation.h" #include "webrtc/base/optional.h" #include "webrtc/modules/include/module.h" +#include "webrtc/modules/rtp_rtcp/include/flexfec_sender.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "webrtc/modules/video_coding/include/video_coding_defines.h" @@ -76,6 +77,10 @@ class RtpRtcp : public Module { // Spread any bursts of packets into smaller bursts to minimize packet loss. RtpPacketSender* paced_sender = nullptr; + // Generate FlexFEC packets. + // TODO(brandtr): Remove when FlexfecSender is wired up to PacedSender. + FlexfecSender* flexfec_sender = nullptr; + TransportSequenceNumberAllocator* transport_sequence_number_allocator = nullptr; BitrateStatisticsObserver* send_bitrate_observer = nullptr; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 8642b02d68..0a99f839d3 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -74,7 +74,7 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) configuration.clock, configuration.outgoing_transport, configuration.paced_sender, - nullptr, // TODO(brandtr): Wire up FlexfecSender here. + configuration.flexfec_sender, configuration.transport_sequence_number_allocator, configuration.transport_feedback_callback, configuration.send_bitrate_observer, diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index aac73aba98..51c9dad131 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -44,6 +44,7 @@ std::vector CreateRtpRtcpModules( RtcpRttStats* rtt_stats, RtpPacketSender* paced_sender, TransportSequenceNumberAllocator* transport_sequence_number_allocator, + FlexfecSender* flexfec_sender, SendStatisticsProxy* stats_proxy, SendDelayStats* send_delay_stats, RtcEventLog* event_log, @@ -54,6 +55,7 @@ std::vector CreateRtpRtcpModules( ReceiveStatistics* null_receive_statistics = configuration.receive_statistics; configuration.audio = false; configuration.receiver_only = false; + configuration.flexfec_sender = flexfec_sender; configuration.receive_statistics = null_receive_statistics; configuration.outgoing_transport = outgoing_transport; configuration.intra_frame_callback = intra_frame_callback; @@ -82,6 +84,48 @@ std::vector CreateRtpRtcpModules( return modules; } +// TODO(brandtr): Update this function when we support multistream protection. +std::unique_ptr MaybeCreateFlexfecSender( + const VideoSendStream::Config& config) { + if (config.rtp.flexfec.flexfec_payload_type < 0) { + return nullptr; + } + RTC_DCHECK_GE(config.rtp.flexfec.flexfec_payload_type, 0); + RTC_DCHECK_LE(config.rtp.flexfec.flexfec_payload_type, 127); + if (config.rtp.flexfec.flexfec_ssrc == 0) { + LOG(LS_WARNING) << "FlexFEC is enabled, but no FlexFEC SSRC given. " + "Therefore disabling FlexFEC."; + return nullptr; + } + if (config.rtp.flexfec.protected_media_ssrcs.empty()) { + LOG(LS_WARNING) << "FlexFEC is enabled, but no protected media SSRC given. " + "Therefore disabling FlexFEC."; + return nullptr; + } + + if (config.rtp.ssrcs.size() > 1) { + LOG(LS_WARNING) << "Both FlexFEC and simulcast are enabled. This " + "combination is however not supported by our current " + "FlexFEC implementation. Therefore disabling FlexFEC."; + return nullptr; + } + + if (config.rtp.flexfec.protected_media_ssrcs.size() > 1) { + LOG(LS_WARNING) + << "The supplied FlexfecConfig contained multiple protected " + "media streams, but our implementation currently only " + "supports protecting a single media stream. " + "To avoid confusion, disabling FlexFEC completely."; + return nullptr; + } + + RTC_DCHECK_EQ(1U, config.rtp.flexfec.protected_media_ssrcs.size()); + return std::unique_ptr(new FlexfecSender( + config.rtp.flexfec.flexfec_payload_type, config.rtp.flexfec.flexfec_ssrc, + config.rtp.flexfec.protected_media_ssrcs[0], config.rtp.extensions, + Clock::GetRealTimeClock())); +} + } // namespace std::string @@ -133,6 +177,7 @@ std::string VideoSendStream::Config::Rtp::ToString() const { ss << ", nack: {rtp_history_ms: " << nack.rtp_history_ms << '}'; ss << ", ulpfec: " << ulpfec.ToString(); + ss << ", flexfec: " << flexfec.ToString(); ss << ", rtx: " << rtx.ToString(); ss << ", c_name: " << c_name; ss << '}'; @@ -325,6 +370,9 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver, BitrateAllocator* const bitrate_allocator_; VieRemb* const remb_; + // TODO(brandtr): Consider moving this to a new FlexfecSendStream class. + std::unique_ptr flexfec_sender_; + rtc::CriticalSection ivf_writers_crit_; std::unique_ptr file_writers_[kMaxSimulcastStreams] GUARDED_BY( ivf_writers_crit_); @@ -661,6 +709,7 @@ VideoSendStreamImpl::VideoSendStreamImpl( congestion_controller_(congestion_controller), bitrate_allocator_(bitrate_allocator), remb_(remb), + flexfec_sender_(MaybeCreateFlexfecSender(*config_)), max_padding_bitrate_(0), encoder_min_bitrate_bps_(0), encoder_max_bitrate_bps_(initial_encoder_max_bitrate), @@ -680,6 +729,7 @@ VideoSendStreamImpl::VideoSendStreamImpl( call_stats_->rtcp_rtt_stats(), congestion_controller_->pacer(), congestion_controller_->packet_router(), + flexfec_sender_.get(), stats_proxy_, send_delay_stats, event_log, @@ -943,15 +993,36 @@ EncodedImageCallback::Result VideoSendStreamImpl::OnEncodedImage( void VideoSendStreamImpl::ConfigureProtection() { RTC_DCHECK_RUN_ON(worker_queue_); + // Consistency of FlexFEC parameters is checked in MaybeCreateFlexfecSender. + const bool flexfec_enabled = (flexfec_sender_ != nullptr); + + // Consistency of NACK and RED+ULPFEC parameters is checked in this function. const bool nack_enabled = config_->rtp.nack.rtp_history_ms > 0; int red_payload_type = config_->rtp.ulpfec.red_payload_type; int ulpfec_payload_type = config_->rtp.ulpfec.ulpfec_payload_type; // Shorthands. auto IsRedEnabled = [&]() { return red_payload_type >= 0; }; + auto DisableRed = [&]() { red_payload_type = -1; }; auto IsUlpfecEnabled = [&]() { return ulpfec_payload_type >= 0; }; auto DisableUlpfec = [&]() { ulpfec_payload_type = -1; }; + // If enabled, FlexFEC takes priority over RED+ULPFEC. + if (flexfec_enabled) { + // We can safely disable RED here, because if the remote supports FlexFEC, + // we know that it has a receiver without the RED/RTX workaround. + // See http://crbug.com/webrtc/6650 for more information. + if (IsRedEnabled()) { + LOG(LS_INFO) << "Both FlexFEC and RED are configured. Disabling RED."; + DisableRed(); + } + if (IsUlpfecEnabled()) { + LOG(LS_INFO) + << "Both FlexFEC and ULPFEC are configured. Disabling ULPFEC."; + DisableUlpfec(); + } + } + // Payload types without picture ID cannot determine that a stream is complete // without retransmitting FEC, so using ULPFEC + NACK for H.264 (for instance) // is a waste of bandwidth since FEC packets still have to be transmitted. @@ -999,8 +1070,10 @@ void VideoSendStreamImpl::ConfigureProtection() { } } - protection_bitrate_calculator_.SetProtectionMethod(IsUlpfecEnabled(), - nack_enabled); + // Currently, both ULPFEC and FlexFEC use the same FEC rate calculation logic, + // so enable that logic if either of those FEC schemes are enabled. + protection_bitrate_calculator_.SetProtectionMethod( + flexfec_enabled || IsUlpfecEnabled(), nack_enabled); } void VideoSendStreamImpl::ConfigureSsrcs() { diff --git a/webrtc/video_send_stream.h b/webrtc/video_send_stream.h index 4e727e5cd3..74e9d5a559 100644 --- a/webrtc/video_send_stream.h +++ b/webrtc/video_send_stream.h @@ -137,6 +137,11 @@ class VideoSendStream { // See UlpfecConfig for description. UlpfecConfig ulpfec; + // See FlexfecConfig for description. + // TODO(brandtr): Move this config to a new class FlexfecSendStream + // when we support multistream protection. + FlexfecConfig flexfec; + // Settings for RTP retransmission payload format, see RFC 4588 for // details. struct Rtx {