From 468da7c0747e078f689eb02cd86ec31cdb5be903 Mon Sep 17 00:00:00 2001 From: brandtr Date: Tue, 22 Nov 2016 02:16:47 -0800 Subject: [PATCH] Wire up FlexFEC in VideoEngine2. This CL interfaces the SDP information (payload types and SSRCs) about FlexFEC with the corresponding configs at the Call layer. It also adds a field trial, which when active will expose FlexFEC in the default codec list, thus showing up in the default SDP. BUG=webrtc:5654 R=magjed@webrtc.org, stefan@webrtc.org CC=perkj@webrtc.org Review-Url: https://codereview.webrtc.org/2511703002 Cr-Commit-Position: refs/heads/master@{#15184} --- webrtc/config.cc | 25 +++ webrtc/config.h | 4 + webrtc/media/engine/fakewebrtccall.cc | 42 +++- webrtc/media/engine/fakewebrtccall.h | 22 ++ webrtc/media/engine/internalencoderfactory.cc | 21 ++ webrtc/media/engine/webrtcvideoengine2.cc | 94 +++++++-- webrtc/media/engine/webrtcvideoengine2.h | 10 +- .../engine/webrtcvideoengine2_unittest.cc | 193 +++++++++++++++++- 8 files changed, 384 insertions(+), 27 deletions(-) diff --git a/webrtc/config.cc b/webrtc/config.cc index a1789a7f1d..f8e3602851 100644 --- a/webrtc/config.cc +++ b/webrtc/config.cc @@ -31,6 +31,12 @@ std::string UlpfecConfig::ToString() const { return ss.str(); } +bool UlpfecConfig::operator==(const UlpfecConfig& other) const { + return ulpfec_payload_type == other.ulpfec_payload_type && + red_payload_type == other.red_payload_type && + red_rtx_payload_type == other.red_rtx_payload_type; +} + FlexfecConfig::FlexfecConfig() : flexfec_payload_type(-1), flexfec_ssrc(0), protected_media_ssrcs() {} @@ -50,6 +56,25 @@ std::string FlexfecConfig::ToString() const { return ss.str(); } +bool FlexfecConfig::IsCompleteAndEnabled() const { + // Check if FlexFEC is enabled. + if (flexfec_payload_type < 0) + return false; + // Do we have the necessary SSRC information? + if (flexfec_ssrc == 0) + return false; + // TODO(brandtr): Update this check when we support multistream protection. + if (protected_media_ssrcs.size() != 1u) + return false; + return true; +} + +bool FlexfecConfig::operator==(const FlexfecConfig& other) const { + return flexfec_payload_type == other.flexfec_payload_type && + flexfec_ssrc == other.flexfec_ssrc && + protected_media_ssrcs == other.protected_media_ssrcs; +} + std::string RtpExtension::ToString() const { std::stringstream ss; ss << "{uri: " << uri; diff --git a/webrtc/config.h b/webrtc/config.h index 3b3c3d90a8..ee81617d8d 100644 --- a/webrtc/config.h +++ b/webrtc/config.h @@ -44,6 +44,8 @@ struct UlpfecConfig { red_payload_type(-1), red_rtx_payload_type(-1) {} std::string ToString() const; + bool operator==(const UlpfecConfig& other) const; + // Payload type used for ULPFEC packets. int ulpfec_payload_type; @@ -60,6 +62,8 @@ struct FlexfecConfig { FlexfecConfig(); ~FlexfecConfig(); std::string ToString() const; + bool IsCompleteAndEnabled() const; + bool operator==(const FlexfecConfig& other) const; // Payload type of FlexFEC. int flexfec_payload_type; diff --git a/webrtc/media/engine/fakewebrtccall.cc b/webrtc/media/engine/fakewebrtccall.cc index c07a2ebadd..a303db5e69 100644 --- a/webrtc/media/engine/fakewebrtccall.cc +++ b/webrtc/media/engine/fakewebrtccall.cc @@ -296,6 +296,28 @@ void FakeVideoReceiveStream::EnableEncodedFrameRecording(rtc::PlatformFile file, rtc::ClosePlatformFile(file); } +FakeFlexfecReceiveStream::FakeFlexfecReceiveStream( + const webrtc::FlexfecReceiveStream::Config& config) + : config_(config), receiving_(false) {} + +const webrtc::FlexfecReceiveStream::Config& +FakeFlexfecReceiveStream::GetConfig() const { + return config_; +} + +void FakeFlexfecReceiveStream::Start() { + receiving_ = true; +} + +void FakeFlexfecReceiveStream::Stop() { + receiving_ = false; +} + +// TODO(brandtr): Implement when the stats have been designed. +webrtc::FlexfecReceiveStream::Stats FakeFlexfecReceiveStream::GetStats() const { + return webrtc::FlexfecReceiveStream::Stats(); +} + FakeCall::FakeCall(const webrtc::Call::Config& config) : config_(config), audio_network_state_(webrtc::kNetworkUp), @@ -348,6 +370,11 @@ const FakeAudioReceiveStream* FakeCall::GetAudioReceiveStream(uint32_t ssrc) { return nullptr; } +const std::list& +FakeCall::GetFlexfecReceiveStreams() { + return flexfec_receive_streams_; +} + webrtc::NetworkState FakeCall::GetNetworkState(webrtc::MediaType media) const { switch (media) { case webrtc::MediaType::AUDIO: @@ -451,13 +478,22 @@ void FakeCall::DestroyVideoReceiveStream( webrtc::FlexfecReceiveStream* FakeCall::CreateFlexfecReceiveStream( webrtc::FlexfecReceiveStream::Config config) { - // TODO(brandtr): Implement when adding integration with WebRtcVideoEngine2. - return nullptr; + flexfec_receive_streams_.push_back( + FakeFlexfecReceiveStream(std::move(config))); + ++num_created_receive_streams_; + return &flexfec_receive_streams_.back(); } void FakeCall::DestroyFlexfecReceiveStream( webrtc::FlexfecReceiveStream* receive_stream) { - // TODO(brandtr): Implement when adding integration with WebRtcVideoEngine2. + for (auto it = flexfec_receive_streams_.begin(); + it != flexfec_receive_streams_.end(); ++it) { + if (&(*it) == receive_stream) { + flexfec_receive_streams_.erase(it); + return; + } + } + ADD_FAILURE() << "DestroyFlexfecReceiveStream called with unknown parameter."; } webrtc::PacketReceiver* FakeCall::Receiver() { diff --git a/webrtc/media/engine/fakewebrtccall.h b/webrtc/media/engine/fakewebrtccall.h index c6b67cfc06..c5e5d09303 100644 --- a/webrtc/media/engine/fakewebrtccall.h +++ b/webrtc/media/engine/fakewebrtccall.h @@ -20,6 +20,7 @@ #ifndef WEBRTC_MEDIA_ENGINE_FAKEWEBRTCCALL_H_ #define WEBRTC_MEDIA_ENGINE_FAKEWEBRTCCALL_H_ +#include #include #include #include @@ -196,6 +197,24 @@ class FakeVideoReceiveStream final : public webrtc::VideoReceiveStream { webrtc::VideoReceiveStream::Stats stats_; }; +class FakeFlexfecReceiveStream final : public webrtc::FlexfecReceiveStream { + public: + explicit FakeFlexfecReceiveStream( + const webrtc::FlexfecReceiveStream::Config& config); + + const webrtc::FlexfecReceiveStream::Config& GetConfig() const; + + private: + // webrtc::FlexfecReceiveStream implementation. + void Start() override; + void Stop() override; + + webrtc::FlexfecReceiveStream::Stats GetStats() const override; + + webrtc::FlexfecReceiveStream::Config config_; + bool receiving_; +}; + class FakeCall final : public webrtc::Call, public webrtc::PacketReceiver { public: explicit FakeCall(const webrtc::Call::Config& config); @@ -210,6 +229,8 @@ class FakeCall final : public webrtc::Call, public webrtc::PacketReceiver { const std::vector& GetAudioReceiveStreams(); const FakeAudioReceiveStream* GetAudioReceiveStream(uint32_t ssrc); + const std::list& GetFlexfecReceiveStreams(); + rtc::SentPacket last_sent_packet() const { return last_sent_packet_; } // This is useful if we care about the last media packet (with id populated) @@ -277,6 +298,7 @@ class FakeCall final : public webrtc::Call, public webrtc::PacketReceiver { std::vector audio_send_streams_; std::vector video_receive_streams_; std::vector audio_receive_streams_; + std::list flexfec_receive_streams_; int num_created_send_streams_; int num_created_receive_streams_; diff --git a/webrtc/media/engine/internalencoderfactory.cc b/webrtc/media/engine/internalencoderfactory.cc index 4c98015447..28e336deb0 100644 --- a/webrtc/media/engine/internalencoderfactory.cc +++ b/webrtc/media/engine/internalencoderfactory.cc @@ -15,9 +15,20 @@ #include "webrtc/modules/video_coding/codecs/h264/include/h264.h" #include "webrtc/modules/video_coding/codecs/vp8/include/vp8.h" #include "webrtc/modules/video_coding/codecs/vp9/include/vp9.h" +#include "webrtc/system_wrappers/include/field_trial.h" namespace cricket { +namespace { + +const char kFlexfecFieldTrialName[] = "WebRTC-FlexFEC-03"; + +bool IsFlexfecFieldTrialEnabled() { + return webrtc::field_trial::FindFullName(kFlexfecFieldTrialName) == "Enabled"; +} + +} // namespace + InternalEncoderFactory::InternalEncoderFactory() { supported_codecs_.push_back(cricket::VideoCodec(kVp8CodecName)); if (webrtc::VP9Encoder::IsSupported()) @@ -36,6 +47,16 @@ InternalEncoderFactory::InternalEncoderFactory() { supported_codecs_.push_back(cricket::VideoCodec(kRedCodecName)); supported_codecs_.push_back(cricket::VideoCodec(kUlpfecCodecName)); + + if (IsFlexfecFieldTrialEnabled()) { + cricket::VideoCodec flexfec_codec(kFlexfecCodecName); + // This value is currently arbitrarily set to 10 seconds. (The unit + // is microseconds.) This parameter MUST be present in the SDP, but + // we never use the actual value anywhere in our code however. + // TODO(brandtr): Consider honouring this value in the sender and receiver. + flexfec_codec.SetParam(kFlexfecFmtpRepairWindow, "10000000"); + supported_codecs_.push_back(flexfec_codec); + } } InternalEncoderFactory::~InternalEncoderFactory() {} diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc index 80395de61e..8f1d1f1ffc 100644 --- a/webrtc/media/engine/webrtcvideoengine2.cc +++ b/webrtc/media/engine/webrtcvideoengine2.cc @@ -39,6 +39,18 @@ namespace cricket { namespace { +// Three things happen when the FlexFEC field trial is enabled: +// 1) FlexFEC is exposed in the default codec list, eventually showing up +// in the default SDP. (See InternalEncoderFactory ctor.) +// 2) FlexFEC send parameters are set in the VideoSendStream config. +// 3) FlexFEC receive parameters are set in the FlexfecReceiveStream config, +// and the corresponding object is instantiated. +const char kFlexfecFieldTrialName[] = "WebRTC-FlexFEC-03"; + +bool IsFlexfecFieldTrialEnabled() { + return webrtc::field_trial::FindFullName(kFlexfecFieldTrialName) == "Enabled"; +} + // Wrap cricket::WebRtcVideoEncoderFactory as a webrtc::VideoEncoderFactory. class EncoderFactoryAdapter : public webrtc::VideoEncoderFactory { public: @@ -1197,7 +1209,8 @@ bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp, receive_ssrcs_.insert(used_ssrc); webrtc::VideoReceiveStream::Config config(this); - ConfigureReceiverRtp(&config, sp); + webrtc::FlexfecConfig flexfec_config; + ConfigureReceiverRtp(&config, &flexfec_config, sp); // Set up A/V sync group based on sync label. config.sync_group = sp.sync_label; @@ -1210,13 +1223,14 @@ bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp, receive_streams_[ssrc] = new WebRtcVideoReceiveStream( call_, sp, std::move(config), external_decoder_factory_, default_stream, - recv_codecs_); + recv_codecs_, flexfec_config); return true; } void WebRtcVideoChannel2::ConfigureReceiverRtp( webrtc::VideoReceiveStream::Config* config, + webrtc::FlexfecConfig* flexfec_config, const StreamParams& sp) const { uint32_t ssrc = sp.first_ssrc(); @@ -1255,6 +1269,14 @@ void WebRtcVideoChannel2::ConfigureReceiverRtp( rtx.payload_type = recv_codecs_[i].rtx_payload_type; } } + + // TODO(brandtr): This code needs to be generalized when we add support for + // multistream protection. + uint32_t flexfec_ssrc; + if (sp.GetFecFrSsrc(ssrc, &flexfec_ssrc)) { + flexfec_config->flexfec_ssrc = flexfec_ssrc; + flexfec_config->protected_media_ssrcs = {ssrc}; + } } bool WebRtcVideoChannel2::RemoveRecvStream(uint32_t ssrc) { @@ -1412,14 +1434,15 @@ void WebRtcVideoChannel2::OnPacketReceived( } // See if this payload_type is registered as one that usually gets its own - // SSRC (RTX) or at least is safe to drop either way (ULPFEC). If it is, and + // SSRC (RTX) or at least is safe to drop either way (FEC). If it is, and // it wasn't handled above by DeliverPacket, that means we don't know what // stream it associates with, and we shouldn't ever create an implicit channel // for these. for (auto& codec : recv_codecs_) { if (payload_type == codec.rtx_payload_type || payload_type == codec.ulpfec.red_rtx_payload_type || - payload_type == codec.ulpfec.ulpfec_payload_type) { + payload_type == codec.ulpfec.ulpfec_payload_type || + payload_type == codec.flexfec.flexfec_payload_type) { return; } } @@ -1562,8 +1585,35 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::WebRtcVideoSendStream( parameters_.conference_mode = send_params.conference_mode; sp.GetPrimarySsrcs(¶meters_.config.rtp.ssrcs); + + // RTX. sp.GetFidSsrcs(parameters_.config.rtp.ssrcs, ¶meters_.config.rtp.rtx.ssrcs); + + // FlexFEC. + // TODO(brandtr): This code needs to be generalized when we add support for + // multistream protection. + if (IsFlexfecFieldTrialEnabled()) { + uint32_t flexfec_ssrc; + bool flexfec_enabled = false; + for (uint32_t primary_ssrc : parameters_.config.rtp.ssrcs) { + if (sp.GetFecFrSsrc(primary_ssrc, &flexfec_ssrc)) { + if (flexfec_enabled) { + LOG(LS_INFO) << "Multiple FlexFEC streams proposed by remote, but " + "our implementation only supports a single FlexFEC " + "stream. Will not enable FlexFEC for proposed " + "stream with SSRC: " + << flexfec_ssrc << "."; + continue; + } + + flexfec_enabled = true; + parameters_.config.rtp.flexfec.flexfec_ssrc = flexfec_ssrc; + parameters_.config.rtp.flexfec.protected_media_ssrcs = {primary_ssrc}; + } + } + } + parameters_.config.rtp.c_name = sp.cname; if (rtp_extensions) { parameters_.config.rtp.extensions = *rtp_extensions; @@ -1744,6 +1794,8 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetCodec( external_encoder_factory_->EncoderTypeHasInternalSource(type); } parameters_.config.rtp.ulpfec = codec_settings.ulpfec; + parameters_.config.rtp.flexfec.flexfec_payload_type = + codec_settings.flexfec.flexfec_payload_type; // Set RTX payload type if RTX is enabled. if (!parameters_.config.rtp.rtx.ssrcs.empty()) { @@ -2119,12 +2171,15 @@ WebRtcVideoChannel2::WebRtcVideoReceiveStream::WebRtcVideoReceiveStream( webrtc::VideoReceiveStream::Config config, WebRtcVideoDecoderFactory* external_decoder_factory, bool default_stream, - const std::vector& recv_codecs) + const std::vector& recv_codecs, + const webrtc::FlexfecConfig& flexfec_config) : call_(call), stream_params_(sp), stream_(NULL), default_stream_(default_stream), config_(std::move(config)), + flexfec_config_(flexfec_config), + flexfec_stream_(nullptr), external_decoder_factory_(external_decoder_factory), sink_(NULL), first_frame_timestamp_(-1), @@ -2253,6 +2308,8 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::ConfigureCodecs( // TODO(pbos): Reconfigure RTX based on incoming recv_codecs. config_.rtp.ulpfec = recv_codecs.front().ulpfec; + flexfec_config_.flexfec_payload_type = + recv_codecs.front().flexfec.flexfec_payload_type; config_.rtp.nack.rtp_history_ms = HasNack(recv_codecs.begin()->codec) ? kNackHistoryMs : 0; } @@ -2324,11 +2381,19 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRecvParameters( } void WebRtcVideoChannel2::WebRtcVideoReceiveStream::RecreateWebRtcStream() { - if (stream_ != NULL) { + if (flexfec_stream_) { + call_->DestroyFlexfecReceiveStream(flexfec_stream_); + flexfec_stream_ = nullptr; + } + if (stream_) { call_->DestroyVideoReceiveStream(stream_); } stream_ = call_->CreateVideoReceiveStream(config_.Copy()); stream_->Start(); + if (IsFlexfecFieldTrialEnabled() && flexfec_config_.IsCompleteAndEnabled()) { + flexfec_stream_ = call_->CreateFlexfecReceiveStream(flexfec_config_); + flexfec_stream_->Start(); + } } void WebRtcVideoChannel2::WebRtcVideoReceiveStream::ClearDecoders( @@ -2443,11 +2508,8 @@ WebRtcVideoChannel2::VideoCodecSettings::VideoCodecSettings() bool WebRtcVideoChannel2::VideoCodecSettings::operator==( const WebRtcVideoChannel2::VideoCodecSettings& other) const { - return codec == other.codec && - ulpfec.ulpfec_payload_type == other.ulpfec.ulpfec_payload_type && - ulpfec.red_payload_type == other.ulpfec.red_payload_type && - ulpfec.red_rtx_payload_type == other.ulpfec.red_rtx_payload_type && - rtx_payload_type == other.rtx_payload_type; + return codec == other.codec && ulpfec == other.ulpfec && + flexfec == other.flexfec && rtx_payload_type == other.rtx_payload_type; } bool WebRtcVideoChannel2::VideoCodecSettings::operator!=( @@ -2466,6 +2528,7 @@ WebRtcVideoChannel2::MapCodecs(const std::vector& codecs) { std::map rtx_mapping; webrtc::UlpfecConfig ulpfec_config; + int flexfec_payload_type = -1; for (size_t i = 0; i < codecs.size(); ++i) { const VideoCodec& in_codec = codecs[i]; @@ -2482,20 +2545,22 @@ WebRtcVideoChannel2::MapCodecs(const std::vector& codecs) { switch (in_codec.GetCodecType()) { case VideoCodec::CODEC_RED: { // RED payload type, should not have duplicates. - RTC_DCHECK(ulpfec_config.red_payload_type == -1); + RTC_DCHECK_EQ(-1, ulpfec_config.red_payload_type); ulpfec_config.red_payload_type = in_codec.id; continue; } case VideoCodec::CODEC_ULPFEC: { // ULPFEC payload type, should not have duplicates. - RTC_DCHECK(ulpfec_config.ulpfec_payload_type == -1); + RTC_DCHECK_EQ(-1, ulpfec_config.ulpfec_payload_type); ulpfec_config.ulpfec_payload_type = in_codec.id; continue; } case VideoCodec::CODEC_FLEXFEC: { - // TODO(brandtr): To be implemented. + // FlexFEC payload type, should not have duplicates. + RTC_DCHECK_EQ(-1, flexfec_payload_type); + flexfec_payload_type = in_codec.id; continue; } @@ -2545,6 +2610,7 @@ WebRtcVideoChannel2::MapCodecs(const std::vector& codecs) { for (size_t i = 0; i < video_codecs.size(); ++i) { video_codecs[i].ulpfec = ulpfec_config; + video_codecs[i].flexfec.flexfec_payload_type = flexfec_payload_type; if (rtx_mapping[video_codecs[i].codec.id] != 0 && rtx_mapping[video_codecs[i].codec.id] != ulpfec_config.red_payload_type) { diff --git a/webrtc/media/engine/webrtcvideoengine2.h b/webrtc/media/engine/webrtcvideoengine2.h index 546799ec88..20884e8bee 100644 --- a/webrtc/media/engine/webrtcvideoengine2.h +++ b/webrtc/media/engine/webrtcvideoengine2.h @@ -194,6 +194,7 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport { VideoCodec codec; webrtc::UlpfecConfig ulpfec; + webrtc::FlexfecConfig flexfec; int rtx_payload_type; }; @@ -220,6 +221,7 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport { void SetMaxSendBandwidth(int bps); void ConfigureReceiverRtp(webrtc::VideoReceiveStream::Config* config, + webrtc::FlexfecConfig* flexfec_config, const StreamParams& sp) const; bool ValidateSendSsrcAvailability(const StreamParams& sp) const EXCLUSIVE_LOCKS_REQUIRED(stream_crit_); @@ -383,7 +385,8 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport { webrtc::VideoReceiveStream::Config config, WebRtcVideoDecoderFactory* external_decoder_factory, bool default_stream, - const std::vector& recv_codecs); + const std::vector& recv_codecs, + const webrtc::FlexfecConfig& flexfec_config); ~WebRtcVideoReceiveStream(); const std::vector& GetSsrcs() const; @@ -430,9 +433,14 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport { webrtc::Call* const call_; StreamParams stream_params_; + // Both |stream_| and |flexfec_stream_| are managed by |this|. They are + // destroyed by calling call_->DestroyVideoReceiveStream and + // call_->DestroyFlexfecReceiveStream, respectively. webrtc::VideoReceiveStream* stream_; const bool default_stream_; webrtc::VideoReceiveStream::Config config_; + webrtc::FlexfecConfig flexfec_config_; + webrtc::FlexfecReceiveStream* flexfec_stream_; WebRtcVideoDecoderFactory* const external_decoder_factory_; std::vector allocated_decoders_; diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc index 70de26711a..c473a4099f 100644 --- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc +++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc @@ -9,6 +9,7 @@ */ #include +#include #include #include #include @@ -38,6 +39,7 @@ static const uint8_t kRedRtxPayloadType = 125; static const uint32_t kSsrcs1[] = {1}; static const uint32_t kSsrcs3[] = {1, 2, 3}; static const uint32_t kRtxSsrcs1[] = {4}; +static const uint32_t kFlexfecSsrc = 5; static const uint32_t kIncomingUnsignalledSsrc = 0xC0FFEE; static const char kUnsupportedExtensionName[] = "urn:ietf:params:rtp-hdrext:unsupported"; @@ -1938,7 +1940,7 @@ TEST_F(Vp9SettingsTest, VerifyVp9SpecificSettings) { class Vp9SettingsTestWithFieldTrial : public Vp9SettingsTest { public: - Vp9SettingsTestWithFieldTrial(const char* field_trials) + explicit Vp9SettingsTestWithFieldTrial(const char* field_trials) : Vp9SettingsTest(field_trials) {} protected: @@ -2286,6 +2288,55 @@ TEST_F(WebRtcVideoChannel2Test, SetDefaultSendCodecs) { // TODO(juberti): Check RTCP, PLI, TMMBR. } +// TODO(brandtr): Remove when FlexFEC _is_ exposed by default. +TEST_F(WebRtcVideoChannel2Test, FlexfecCodecWithoutSsrcNotExposedByDefault) { + FakeVideoSendStream* stream = AddSendStream(); + webrtc::VideoSendStream::Config config = stream->GetConfig().Copy(); + + EXPECT_EQ(-1, config.rtp.flexfec.flexfec_payload_type); +} + +// TODO(brandtr): Remove when FlexFEC _is_ exposed by default. +TEST_F(WebRtcVideoChannel2Test, FlexfecCodecWithSsrcNotExposedByDefault) { + FakeVideoSendStream* stream = AddSendStream( + CreatePrimaryWithFecFrStreamParams("cname", kSsrcs1[0], kFlexfecSsrc)); + webrtc::VideoSendStream::Config config = stream->GetConfig().Copy(); + + EXPECT_EQ(-1, config.rtp.flexfec.flexfec_payload_type); +} + +// TODO(brandtr): When FlexFEC is no longer behind a field trial, merge all +// tests that use this test fixture into the corresponding "non-field trial" +// tests. +class WebRtcVideoChannel2FlexfecTest : public WebRtcVideoChannel2Test { + public: + WebRtcVideoChannel2FlexfecTest() + : WebRtcVideoChannel2Test("WebRTC-FlexFEC-03/Enabled/") {} +}; + +// TODO(brandtr): Merge into "non-field trial" test when FlexFEC is enabled +// by default. +TEST_F(WebRtcVideoChannel2FlexfecTest, SetDefaultSendCodecsWithoutSsrc) { + FakeVideoSendStream* stream = AddSendStream(); + webrtc::VideoSendStream::Config config = stream->GetConfig().Copy(); + + EXPECT_EQ(GetEngineCodec("flexfec-03").id, + config.rtp.flexfec.flexfec_payload_type); + EXPECT_FALSE(config.rtp.flexfec.IsCompleteAndEnabled()); +} + +// TODO(brandtr): Merge into "non-field trial" test when FlexFEC is enabled +// by default. +TEST_F(WebRtcVideoChannel2FlexfecTest, SetDefaultSendCodecsWithSsrc) { + FakeVideoSendStream* stream = AddSendStream( + CreatePrimaryWithFecFrStreamParams("cname", kSsrcs1[0], kFlexfecSsrc)); + webrtc::VideoSendStream::Config config = stream->GetConfig().Copy(); + + EXPECT_EQ(GetEngineCodec("flexfec-03").id, + config.rtp.flexfec.flexfec_payload_type); + EXPECT_TRUE(config.rtp.flexfec.IsCompleteAndEnabled()); +} + TEST_F(WebRtcVideoChannel2Test, SetSendCodecsWithoutFec) { cricket::VideoSendParameters parameters; parameters.codecs.push_back(GetEngineCodec("VP8")); @@ -2298,6 +2349,19 @@ TEST_F(WebRtcVideoChannel2Test, SetSendCodecsWithoutFec) { EXPECT_EQ(-1, config.rtp.ulpfec.red_payload_type); } +// TODO(brandtr): Merge into "non-field trial" test when FlexFEC is enabled +// by default. +TEST_F(WebRtcVideoChannel2FlexfecTest, SetSendCodecsWithoutFec) { + cricket::VideoSendParameters parameters; + parameters.codecs.push_back(GetEngineCodec("VP8")); + ASSERT_TRUE(channel_->SetSendParameters(parameters)); + + FakeVideoSendStream* stream = AddSendStream(); + webrtc::VideoSendStream::Config config = stream->GetConfig().Copy(); + + EXPECT_EQ(-1, config.rtp.flexfec.flexfec_payload_type); +} + TEST_F(WebRtcVideoChannel2Test, SetSendCodecRejectsRtxWithoutAssociatedPayloadType) { const int kUnusedPayloadType = 127; @@ -2349,10 +2413,37 @@ TEST_F(WebRtcVideoChannel2Test, SetSendCodecsWithoutFecDisablesFec) { parameters.codecs.pop_back(); ASSERT_TRUE(channel_->SetSendParameters(parameters)); stream = fake_call_->GetVideoSendStreams()[0]; - ASSERT_TRUE(stream != NULL); + ASSERT_TRUE(stream != nullptr); config = stream->GetConfig().Copy(); EXPECT_EQ(-1, config.rtp.ulpfec.ulpfec_payload_type) - << "SetSendCodec without FEC should disable current FEC."; + << "SetSendCodec without ULPFEC should disable current ULPFEC."; +} + +// TODO(brandtr): Merge into "non-field trial" test when FlexFEC is enabled +// by default. +TEST_F(WebRtcVideoChannel2FlexfecTest, SetSendCodecsWithoutFecDisablesFec) { + cricket::VideoSendParameters parameters; + parameters.codecs.push_back(GetEngineCodec("VP8")); + parameters.codecs.push_back(GetEngineCodec("flexfec-03")); + ASSERT_TRUE(channel_->SetSendParameters(parameters)); + + FakeVideoSendStream* stream = AddSendStream( + CreatePrimaryWithFecFrStreamParams("cname", kSsrcs1[0], kFlexfecSsrc)); + webrtc::VideoSendStream::Config config = stream->GetConfig().Copy(); + + EXPECT_EQ(GetEngineCodec("flexfec-03").id, + config.rtp.flexfec.flexfec_payload_type); + EXPECT_EQ(kFlexfecSsrc, config.rtp.flexfec.flexfec_ssrc); + ASSERT_EQ(1U, config.rtp.flexfec.protected_media_ssrcs.size()); + EXPECT_EQ(kSsrcs1[0], config.rtp.flexfec.protected_media_ssrcs[0]); + + parameters.codecs.pop_back(); + ASSERT_TRUE(channel_->SetSendParameters(parameters)); + stream = fake_call_->GetVideoSendStreams()[0]; + ASSERT_TRUE(stream != nullptr); + config = stream->GetConfig().Copy(); + EXPECT_EQ(-1, config.rtp.flexfec.flexfec_payload_type) + << "SetSendCodec without FlexFEC should disable current FlexFEC."; } TEST_F(WebRtcVideoChannel2Test, SetSendCodecsChangesExistingStreams) { @@ -2698,9 +2789,37 @@ TEST_F(WebRtcVideoChannel2Test, SetRecvCodecsWithoutFecDisablesFec) { recv_parameters.codecs.push_back(GetEngineCodec("VP8")); ASSERT_TRUE(channel_->SetRecvParameters(recv_parameters)); stream = fake_call_->GetVideoReceiveStreams()[0]; - ASSERT_TRUE(stream != NULL); + ASSERT_TRUE(stream != nullptr); EXPECT_EQ(-1, stream->GetConfig().rtp.ulpfec.ulpfec_payload_type) - << "SetSendCodec without FEC should disable current FEC."; + << "SetSendCodec without ULPFEC should disable current ULPFEC."; +} + +// TODO(brandtr): Merge into "non-field trial" test when FlexFEC is enabled +// by default. +TEST_F(WebRtcVideoChannel2FlexfecTest, SetRecvParamsWithoutFecDisablesFec) { + cricket::VideoSendParameters send_parameters; + send_parameters.codecs.push_back(GetEngineCodec("VP8")); + send_parameters.codecs.push_back(GetEngineCodec("flexfec-03")); + ASSERT_TRUE(channel_->SetSendParameters(send_parameters)); + + AddRecvStream( + CreatePrimaryWithFecFrStreamParams("cname", kSsrcs1[0], kFlexfecSsrc)); + const std::list& streams = + fake_call_->GetFlexfecReceiveStreams(); + + ASSERT_EQ(1U, streams.size()); + const FakeFlexfecReceiveStream& stream = streams.front(); + EXPECT_EQ(GetEngineCodec("flexfec-03").id, + stream.GetConfig().flexfec_payload_type); + EXPECT_EQ(kFlexfecSsrc, stream.GetConfig().flexfec_ssrc); + ASSERT_EQ(1U, stream.GetConfig().protected_media_ssrcs.size()); + EXPECT_EQ(kSsrcs1[0], stream.GetConfig().protected_media_ssrcs[0]); + + cricket::VideoRecvParameters recv_parameters; + recv_parameters.codecs.push_back(GetEngineCodec("VP8")); + ASSERT_TRUE(channel_->SetRecvParameters(recv_parameters)); + EXPECT_TRUE(streams.empty()) + << "SetSendCodec without FlexFEC should disable current FlexFEC."; } TEST_F(WebRtcVideoChannel2Test, SetSendParamsWithFecEnablesFec) { @@ -2714,10 +2833,10 @@ TEST_F(WebRtcVideoChannel2Test, SetSendParamsWithFecEnablesFec) { recv_parameters.codecs.push_back(GetEngineCodec("ulpfec")); ASSERT_TRUE(channel_->SetRecvParameters(recv_parameters)); stream = fake_call_->GetVideoReceiveStreams()[0]; - ASSERT_TRUE(stream != NULL); + ASSERT_TRUE(stream != nullptr); EXPECT_EQ(GetEngineCodec("ulpfec").id, stream->GetConfig().rtp.ulpfec.ulpfec_payload_type) - << "FEC should be enabled on the receive stream."; + << "ULPFEC should be enabled on the receive stream."; cricket::VideoSendParameters send_parameters; send_parameters.codecs.push_back(GetEngineCodec("VP8")); @@ -2727,7 +2846,44 @@ TEST_F(WebRtcVideoChannel2Test, SetSendParamsWithFecEnablesFec) { stream = fake_call_->GetVideoReceiveStreams()[0]; EXPECT_EQ(GetEngineCodec("ulpfec").id, stream->GetConfig().rtp.ulpfec.ulpfec_payload_type) - << "FEC should be enabled on the receive stream."; + << "ULPFEC should be enabled on the receive stream."; +} + +// TODO(brandtr): Merge into "non-field trial" test when FlexFEC is enabled +// by default. +TEST_F(WebRtcVideoChannel2FlexfecTest, SetSendParamsWithFecEnablesFec) { + AddRecvStream( + CreatePrimaryWithFecFrStreamParams("cname", kSsrcs1[0], kFlexfecSsrc)); + const std::list& streams = + fake_call_->GetFlexfecReceiveStreams(); + + cricket::VideoRecvParameters recv_parameters; + recv_parameters.codecs.push_back(GetEngineCodec("VP8")); + recv_parameters.codecs.push_back(GetEngineCodec("flexfec-03")); + ASSERT_TRUE(channel_->SetRecvParameters(recv_parameters)); + ASSERT_EQ(1U, streams.size()); + const FakeFlexfecReceiveStream& stream_with_recv_params = streams.front(); + EXPECT_EQ(GetEngineCodec("flexfec-03").id, + stream_with_recv_params.GetConfig().flexfec_payload_type); + EXPECT_EQ(kFlexfecSsrc, stream_with_recv_params.GetConfig().flexfec_ssrc); + EXPECT_EQ(1U, + stream_with_recv_params.GetConfig().protected_media_ssrcs.size()); + EXPECT_EQ(kSsrcs1[0], + stream_with_recv_params.GetConfig().protected_media_ssrcs[0]); + + cricket::VideoSendParameters send_parameters; + send_parameters.codecs.push_back(GetEngineCodec("VP8")); + send_parameters.codecs.push_back(GetEngineCodec("flexfec-03")); + ASSERT_TRUE(channel_->SetSendParameters(send_parameters)); + ASSERT_EQ(1U, streams.size()); + const FakeFlexfecReceiveStream& stream_with_send_params = streams.front(); + EXPECT_EQ(GetEngineCodec("flexfec-03").id, + stream_with_send_params.GetConfig().flexfec_payload_type); + EXPECT_EQ(kFlexfecSsrc, stream_with_send_params.GetConfig().flexfec_ssrc); + EXPECT_EQ(1U, + stream_with_send_params.GetConfig().protected_media_ssrcs.size()); + EXPECT_EQ(kSsrcs1[0], + stream_with_send_params.GetConfig().protected_media_ssrcs[0]); } TEST_F(WebRtcVideoChannel2Test, SetSendCodecsRejectDuplicateFecPayloads) { @@ -2738,6 +2894,17 @@ TEST_F(WebRtcVideoChannel2Test, SetSendCodecsRejectDuplicateFecPayloads) { EXPECT_FALSE(channel_->SetRecvParameters(parameters)); } +// TODO(brandtr): Merge into "non-field trial" test when FlexFEC is enabled +// by default. +TEST_F(WebRtcVideoChannel2FlexfecTest, + SetSendCodecsRejectDuplicateFecPayloads) { + cricket::VideoRecvParameters parameters; + parameters.codecs.push_back(GetEngineCodec("VP8")); + parameters.codecs.push_back(GetEngineCodec("flexfec-03")); + parameters.codecs[1].id = parameters.codecs[0].id; + EXPECT_FALSE(channel_->SetRecvParameters(parameters)); +} + TEST_F(WebRtcVideoChannel2Test, SetRecvCodecsRejectDuplicateCodecPayloads) { cricket::VideoRecvParameters parameters; parameters.codecs.push_back(GetEngineCodec("VP8")); @@ -3377,6 +3544,14 @@ TEST_F(WebRtcVideoChannel2Test, UlpfecPacketDoesntCreateUnsignalledStream) { false /* expect_created_receive_stream */); } +// TODO(brandtr): Change to "non-field trial" test when FlexFEC is enabled +// by default. +TEST_F(WebRtcVideoChannel2FlexfecTest, + FlexfecPacketDoesntCreateUnsignalledStream) { + TestReceiveUnsignaledSsrcPacket(GetEngineCodec("flexfec-03").id, + false /* expect_created_receive_stream */); +} + TEST_F(WebRtcVideoChannel2Test, RedRtxPacketDoesntCreateUnsignalledStream) { TestReceiveUnsignaledSsrcPacket(kRedRtxPayloadType, false /* expect_created_receive_stream */); @@ -3396,7 +3571,7 @@ TEST_F(WebRtcVideoChannel2Test, CanSentMaxBitrateForExistingStream) { capturer.CaptureFrame(); int default_encoder_bitrate = GetMaxEncoderBitrate(); - EXPECT_TRUE(default_encoder_bitrate > 1000); + EXPECT_GT(default_encoder_bitrate, 1000); // TODO(skvlad): Resolve the inconsistency between the interpretation // of the global bitrate limit for audio and video: