diff --git a/call/call_perf_tests.cc b/call/call_perf_tests.cc index 58435f03cc..5dd69bd497 100644 --- a/call/call_perf_tests.cc +++ b/call/call_perf_tests.cc @@ -238,9 +238,8 @@ void CallPerfTest::TestAudioVideoSync(FecMode fec, if (fec == FecMode::kOn) { video_send_config_.rtp.ulpfec.red_payload_type = kRedPayloadType; video_send_config_.rtp.ulpfec.ulpfec_payload_type = kUlpfecPayloadType; - video_receive_configs_[0].rtp.ulpfec.red_payload_type = kRedPayloadType; - video_receive_configs_[0].rtp.ulpfec.ulpfec_payload_type = - kUlpfecPayloadType; + video_receive_configs_[0].rtp.red_payload_type = kRedPayloadType; + video_receive_configs_[0].rtp.ulpfec_payload_type = kUlpfecPayloadType; } video_receive_configs_[0].rtp.nack.rtp_history_ms = 1000; video_receive_configs_[0].renderer = &observer; diff --git a/call/rampup_tests.cc b/call/rampup_tests.cc index da7095cbe6..6ce2efd6ac 100644 --- a/call/rampup_tests.cc +++ b/call/rampup_tests.cc @@ -202,9 +202,9 @@ void RampUpTester::ModifyVideoConfigs( recv_config.rtp.nack.rtp_history_ms = send_config->rtp.nack.rtp_history_ms; if (red_) { - recv_config.rtp.ulpfec.red_payload_type = + recv_config.rtp.red_payload_type = send_config->rtp.ulpfec.red_payload_type; - recv_config.rtp.ulpfec.ulpfec_payload_type = + recv_config.rtp.ulpfec_payload_type = send_config->rtp.ulpfec.ulpfec_payload_type; if (rtx_) { recv_config.rtp.rtx_associated_payload_types diff --git a/call/video_receive_stream.cc b/call/video_receive_stream.cc index 76892ea922..f338805746 100644 --- a/call/video_receive_stream.cc +++ b/call/video_receive_stream.cc @@ -110,7 +110,8 @@ std::string VideoReceiveStream::Config::Rtp::ToString() const { ss << ", remb: " << (remb ? "on" : "off"); ss << ", transport_cc: " << (transport_cc ? "on" : "off"); ss << ", nack: {rtp_history_ms: " << nack.rtp_history_ms << '}'; - ss << ", ulpfec: " << ulpfec.ToString(); + ss << ", ulpfec_payload_type: " << ulpfec_payload_type; + ss << ", red_type: " << red_payload_type; ss << ", rtx_ssrc: " << rtx_ssrc; ss << ", rtx_payload_types: {"; for (auto& kv : rtx_associated_payload_types) { diff --git a/call/video_receive_stream.h b/call/video_receive_stream.h index 7d996ab44f..de6f712901 100644 --- a/call/video_receive_stream.h +++ b/call/video_receive_stream.h @@ -166,13 +166,9 @@ class VideoReceiveStream { // See NackConfig for description. NackConfig nack; - // See UlpfecConfig for description. - // TODO(nisse): UlpfecConfig includes the field red_rtx_payload_type, - // which duplicates info in the rtx_associated_payload_types mapping. So - // delete the use of UlpfecConfig here, and replace by the values which - // make sense in this context, likely those are ulpfec_payload_type_ and - // red_payload_type_. - UlpfecConfig ulpfec; + // Payload types for ULPFEC and RED, respectively. + int ulpfec_payload_type = -1; + int red_payload_type = -1; // SSRC for retransmissions. uint32_t rtx_ssrc = 0; diff --git a/media/engine/webrtcvideoengine.cc b/media/engine/webrtcvideoengine.cc index 29fc442cb1..69af25b01d 100644 --- a/media/engine/webrtcvideoengine.cc +++ b/media/engine/webrtcvideoengine.cc @@ -2228,6 +2228,7 @@ CricketDecoderFactoryAdapter::CreateVideoDecoder( void WebRtcVideoChannel::WebRtcVideoReceiveStream::ConfigureCodecs( const std::vector& recv_codecs, DecoderMap* old_decoders) { + RTC_DCHECK(!recv_codecs.empty()); *old_decoders = std::move(allocated_decoders_); config_.decoders.clear(); config_.rtp.rtx_associated_payload_types.clear(); @@ -2263,14 +2264,15 @@ void WebRtcVideoChannel::WebRtcVideoReceiveStream::ConfigureCodecs( RTC_CHECK(did_insert); } - config_.rtp.ulpfec = recv_codecs.front().ulpfec; + const auto& codec = recv_codecs.front(); + config_.rtp.ulpfec_payload_type = codec.ulpfec.ulpfec_payload_type; + config_.rtp.red_payload_type = codec.ulpfec.red_payload_type; - config_.rtp.nack.rtp_history_ms = - HasNack(recv_codecs.begin()->codec) ? kNackHistoryMs : 0; - if (config_.rtp.ulpfec.red_rtx_payload_type != -1) { + config_.rtp.nack.rtp_history_ms = HasNack(codec.codec) ? kNackHistoryMs : 0; + if (codec.ulpfec.red_rtx_payload_type != -1) { config_.rtp - .rtx_associated_payload_types[config_.rtp.ulpfec.red_rtx_payload_type] = - config_.rtp.ulpfec.red_payload_type; + .rtx_associated_payload_types[codec.ulpfec.red_rtx_payload_type] = + codec.ulpfec.red_payload_type; } } diff --git a/media/engine/webrtcvideoengine_unittest.cc b/media/engine/webrtcvideoengine_unittest.cc index 1b3ec720bd..80d8a07334 100644 --- a/media/engine/webrtcvideoengine_unittest.cc +++ b/media/engine/webrtcvideoengine_unittest.cc @@ -3326,14 +3326,14 @@ TEST_F(WebRtcVideoChannelTest, SetRecvCodecsWithoutFecDisablesFec) { FakeVideoReceiveStream* stream = AddRecvStream(); EXPECT_EQ(GetEngineCodec("ulpfec").id, - stream->GetConfig().rtp.ulpfec.ulpfec_payload_type); + stream->GetConfig().rtp.ulpfec_payload_type); cricket::VideoRecvParameters recv_parameters; recv_parameters.codecs.push_back(GetEngineCodec("VP8")); ASSERT_TRUE(channel_->SetRecvParameters(recv_parameters)); stream = fake_call_->GetVideoReceiveStreams()[0]; ASSERT_TRUE(stream != nullptr); - EXPECT_EQ(-1, stream->GetConfig().rtp.ulpfec.ulpfec_payload_type) + EXPECT_EQ(-1, stream->GetConfig().rtp.ulpfec_payload_type) << "SetSendCodec without ULPFEC should disable current ULPFEC."; } @@ -3360,7 +3360,7 @@ TEST_F(WebRtcVideoChannelFlexfecRecvTest, SetRecvParamsWithoutFecDisablesFec) { TEST_F(WebRtcVideoChannelTest, SetSendParamsWithFecEnablesFec) { FakeVideoReceiveStream* stream = AddRecvStream(); EXPECT_EQ(GetEngineCodec("ulpfec").id, - stream->GetConfig().rtp.ulpfec.ulpfec_payload_type); + stream->GetConfig().rtp.ulpfec_payload_type); cricket::VideoRecvParameters recv_parameters; recv_parameters.codecs.push_back(GetEngineCodec("VP8")); @@ -3370,7 +3370,7 @@ TEST_F(WebRtcVideoChannelTest, SetSendParamsWithFecEnablesFec) { stream = fake_call_->GetVideoReceiveStreams()[0]; ASSERT_TRUE(stream != nullptr); EXPECT_EQ(GetEngineCodec("ulpfec").id, - stream->GetConfig().rtp.ulpfec.ulpfec_payload_type) + stream->GetConfig().rtp.ulpfec_payload_type) << "ULPFEC should be enabled on the receive stream."; cricket::VideoSendParameters send_parameters; @@ -3380,7 +3380,7 @@ TEST_F(WebRtcVideoChannelTest, SetSendParamsWithFecEnablesFec) { ASSERT_TRUE(channel_->SetSendParameters(send_parameters)); stream = fake_call_->GetVideoReceiveStreams()[0]; EXPECT_EQ(GetEngineCodec("ulpfec").id, - stream->GetConfig().rtp.ulpfec.ulpfec_payload_type) + stream->GetConfig().rtp.ulpfec_payload_type) << "ULPFEC should be enabled on the receive stream."; } diff --git a/video/end_to_end_tests.cc b/video/end_to_end_tests.cc index 06a7118228..2ae3ceefbc 100644 --- a/video/end_to_end_tests.cc +++ b/video/end_to_end_tests.cc @@ -140,7 +140,7 @@ class EndToEndTest : public test::CallTest { void RespectsRtcpMode(RtcpMode rtcp_mode); void TestSendsSetSsrcs(size_t num_ssrcs, bool send_single_ssrc_first); void TestRtpStatePreservation(bool use_rtx, bool provoke_rtcpsr_before_rtp); - void VerifyHistogramStats(bool use_rtx, bool use_red, bool screenshare); + void VerifyHistogramStats(bool use_rtx, bool use_fec, bool screenshare); void VerifyNewVideoSendStreamsRespectNetworkState( MediaType network_to_bring_up, VideoEncoder* encoder, @@ -696,8 +696,8 @@ TEST_F(EndToEndTest, ReceivesUlpfec) { // Enable ULPFEC over RED. send_config->rtp.ulpfec.red_payload_type = kRedPayloadType; send_config->rtp.ulpfec.ulpfec_payload_type = kUlpfecPayloadType; - (*receive_configs)[0].rtp.ulpfec.red_payload_type = kRedPayloadType; - (*receive_configs)[0].rtp.ulpfec.ulpfec_payload_type = kUlpfecPayloadType; + (*receive_configs)[0].rtp.red_payload_type = kRedPayloadType; + (*receive_configs)[0].rtp.ulpfec_payload_type = kUlpfecPayloadType; (*receive_configs)[0].renderer = this; } @@ -853,16 +853,11 @@ class FlexfecRenderObserver : public test::EndToEndTest, if (enable_nack_) { send_config->rtp.nack.rtp_history_ms = test::CallTest::kNackRtpHistoryMs; - send_config->rtp.ulpfec.red_rtx_payload_type = - test::CallTest::kRtxRedPayloadType; send_config->rtp.rtx.ssrcs.push_back(test::CallTest::kSendRtxSsrcs[0]); send_config->rtp.rtx.payload_type = test::CallTest::kSendRtxPayloadType; (*receive_configs)[0].rtp.nack.rtp_history_ms = test::CallTest::kNackRtpHistoryMs; - (*receive_configs)[0].rtp.ulpfec.red_rtx_payload_type = - test::CallTest::kRtxRedPayloadType; - (*receive_configs)[0].rtp.rtx_ssrc = test::CallTest::kSendRtxSsrcs[0]; (*receive_configs)[0] .rtp @@ -1043,8 +1038,8 @@ TEST_F(EndToEndTest, ReceivedUlpfecPacketsNotNacked) { send_config->encoder_settings.payload_type = kFakeVideoSendPayloadType; (*receive_configs)[0].rtp.nack.rtp_history_ms = kNackRtpHistoryMs; - (*receive_configs)[0].rtp.ulpfec.red_payload_type = kRedPayloadType; - (*receive_configs)[0].rtp.ulpfec.ulpfec_payload_type = kUlpfecPayloadType; + (*receive_configs)[0].rtp.red_payload_type = kRedPayloadType; + (*receive_configs)[0].rtp.ulpfec_payload_type = kUlpfecPayloadType; (*receive_configs)[0].decoders.resize(1); (*receive_configs)[0].decoders[0].payload_type = @@ -1171,12 +1166,10 @@ void EndToEndTest::DecodesRetransmittedFrame(bool enable_rtx, bool enable_red) { send_config->rtp.ulpfec.red_payload_type = kRedPayloadType; if (retransmission_ssrc_ == kSendRtxSsrcs[0]) send_config->rtp.ulpfec.red_rtx_payload_type = kRtxRedPayloadType; - (*receive_configs)[0].rtp.ulpfec.ulpfec_payload_type = + (*receive_configs)[0].rtp.ulpfec_payload_type = send_config->rtp.ulpfec.ulpfec_payload_type; - (*receive_configs)[0].rtp.ulpfec.red_payload_type = + (*receive_configs)[0].rtp.red_payload_type = send_config->rtp.ulpfec.red_payload_type; - (*receive_configs)[0].rtp.ulpfec.red_rtx_payload_type = - send_config->rtp.ulpfec.red_rtx_payload_type; } if (retransmission_ssrc_ == kSendRtxSsrcs[0]) { @@ -1207,8 +1200,8 @@ void EndToEndTest::DecodesRetransmittedFrame(bool enable_rtx, bool enable_red) { << "Timed out while waiting for retransmission to render."; } - int GetPayloadType(bool use_rtx, bool use_red) { - if (use_red) { + int GetPayloadType(bool use_rtx, bool use_fec) { + if (use_fec) { if (use_rtx) return kRtxRedPayloadType; return kRedPayloadType; @@ -2700,18 +2693,18 @@ TEST_F(EndToEndTest, VerifyNackStats) { } void EndToEndTest::VerifyHistogramStats(bool use_rtx, - bool use_red, + bool use_fec, bool screenshare) { class StatsObserver : public test::EndToEndTest, public rtc::VideoSinkInterface { public: - StatsObserver(bool use_rtx, bool use_red, bool screenshare) + StatsObserver(bool use_rtx, bool use_fec, bool screenshare) : EndToEndTest(kLongTimeoutMs), use_rtx_(use_rtx), - use_red_(use_red), + use_fec_(use_fec), screenshare_(screenshare), // This test uses NACK, so to send FEC we can't use a fake encoder. - vp8_encoder_(use_red ? VP8Encoder::Create() : nullptr), + vp8_encoder_(use_fec ? VP8Encoder::Create() : nullptr), sender_call_(nullptr), receiver_call_(nullptr), start_runtime_ms_(-1), @@ -2762,15 +2755,14 @@ void EndToEndTest::VerifyHistogramStats(bool use_rtx, (*receive_configs)[0].rtp.nack.rtp_history_ms = kNackRtpHistoryMs; (*receive_configs)[0].renderer = this; // FEC - if (use_red_) { + if (use_fec_) { send_config->rtp.ulpfec.ulpfec_payload_type = kUlpfecPayloadType; send_config->rtp.ulpfec.red_payload_type = kRedPayloadType; send_config->encoder_settings.encoder = vp8_encoder_.get(); send_config->encoder_settings.payload_name = "VP8"; (*receive_configs)[0].decoders[0].payload_name = "VP8"; - (*receive_configs)[0].rtp.ulpfec.red_payload_type = kRedPayloadType; - (*receive_configs)[0].rtp.ulpfec.ulpfec_payload_type = - kUlpfecPayloadType; + (*receive_configs)[0].rtp.red_payload_type = kRedPayloadType; + (*receive_configs)[0].rtp.ulpfec_payload_type = kUlpfecPayloadType; } // RTX if (use_rtx_) { @@ -2780,6 +2772,12 @@ void EndToEndTest::VerifyHistogramStats(bool use_rtx, (*receive_configs)[0] .rtp.rtx_associated_payload_types[kSendRtxPayloadType] = kFakeVideoSendPayloadType; + if (use_fec_) { + send_config->rtp.ulpfec.red_rtx_payload_type = kRtxRedPayloadType; + (*receive_configs)[0] + .rtp.rtx_associated_payload_types[kRtxRedPayloadType] = + kSendRtxPayloadType; + } } // RTT needed for RemoteNtpTimeEstimator for the receive stream. (*receive_configs)[0].rtp.rtcp_xr.receiver_reference_time_report = true; @@ -2799,14 +2797,14 @@ void EndToEndTest::VerifyHistogramStats(bool use_rtx, rtc::CriticalSection crit_; const bool use_rtx_; - const bool use_red_; + const bool use_fec_; const bool screenshare_; const std::unique_ptr vp8_encoder_; Call* sender_call_; Call* receiver_call_; int64_t start_runtime_ms_; int num_frames_received_ RTC_GUARDED_BY(&crit_); - } test(use_rtx, use_red, screenshare); + } test(use_rtx, use_fec, screenshare); metrics::Reset(); RunBaseTest(&test); @@ -2916,7 +2914,7 @@ void EndToEndTest::VerifyHistogramStats(bool use_rtx, EXPECT_EQ(num_rtx_samples, metrics::NumSamples("WebRTC.Video.RtxBitrateReceivedInKbps")); - int num_red_samples = use_red ? 1 : 0; + int num_red_samples = use_fec ? 1 : 0; EXPECT_EQ(num_red_samples, metrics::NumSamples("WebRTC.Video.FecBitrateSentInKbps")); EXPECT_EQ(num_red_samples, @@ -4838,7 +4836,10 @@ TEST_F(EndToEndTest, VerifyDefaultVideoReceiveConfigParameters) { << "Enabling RTP extensions require negotiation."; VerifyEmptyNackConfig(default_receive_config.rtp.nack); - VerifyEmptyUlpfecConfig(default_receive_config.rtp.ulpfec); + EXPECT_EQ(-1, default_receive_config.rtp.ulpfec_payload_type) + << "Enabling ULPFEC requires rtpmap: ulpfec negotiation."; + EXPECT_EQ(-1, default_receive_config.rtp.red_payload_type) + << "Enabling ULPFEC requires rtpmap: red negotiation."; } TEST_F(EndToEndTest, VerifyDefaultFlexfecReceiveConfigParameters) { diff --git a/video/receive_statistics_proxy.cc b/video/receive_statistics_proxy.cc index b19ec2075d..b7977380ee 100644 --- a/video/receive_statistics_proxy.cc +++ b/video/receive_statistics_proxy.cc @@ -366,7 +366,7 @@ void ReceiveStatisticsProxy::UpdateHistograms() { static_cast(rtx.transmitted.TotalBytes() * 8 / elapsed_sec / 1000)); } - if (config_.rtp.ulpfec.ulpfec_payload_type != -1) { + if (config_.rtp.ulpfec_payload_type != -1) { RTC_HISTOGRAM_COUNTS_10000( "WebRTC.Video.FecBitrateReceivedInKbps", static_cast(rtp_rtx.fec.TotalBytes() * 8 / elapsed_sec / 1000)); diff --git a/video/replay.cc b/video/replay.cc index 82c9de14a6..b9214b8ae6 100644 --- a/video/replay.cc +++ b/video/replay.cc @@ -215,8 +215,8 @@ void RtpReplay() { receive_config.rtp.rtx_ssrc = flags::SsrcRtx(); receive_config.rtp.rtx_associated_payload_types[flags::PayloadTypeRtx()] = flags::PayloadType(); - receive_config.rtp.ulpfec.ulpfec_payload_type = flags::FecPayloadType(); - receive_config.rtp.ulpfec.red_payload_type = flags::RedPayloadType(); + receive_config.rtp.ulpfec_payload_type = flags::FecPayloadType(); + receive_config.rtp.red_payload_type = flags::RedPayloadType(); receive_config.rtp.nack.rtp_history_ms = 1000; if (flags::TransmissionOffsetId() != -1) { receive_config.rtp.extensions.push_back(RtpExtension( diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc index 881ab349bc..c5483e91b4 100644 --- a/video/rtp_video_stream_receiver.cc +++ b/video/rtp_video_stream_receiver.cc @@ -148,7 +148,7 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver( VideoCodec ulpfec_codec = {}; ulpfec_codec.codecType = kVideoCodecULPFEC; strncpy(ulpfec_codec.plName, "ulpfec", sizeof(ulpfec_codec.plName)); - ulpfec_codec.plType = config_.rtp.ulpfec.ulpfec_payload_type; + ulpfec_codec.plType = config_.rtp.ulpfec_payload_type; RTC_CHECK(AddReceiveCodec(ulpfec_codec)); } @@ -156,7 +156,7 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver( VideoCodec red_codec = {}; red_codec.codecType = kVideoCodecRED; strncpy(red_codec.plName, "red", sizeof(red_codec.plName)); - red_codec.plType = config_.rtp.ulpfec.red_payload_type; + red_codec.plType = config_.rtp.red_payload_type; RTC_CHECK(AddReceiveCodec(red_codec)); } @@ -362,11 +362,11 @@ int32_t RtpVideoStreamReceiver::RequestKeyFrame() { } bool RtpVideoStreamReceiver::IsUlpfecEnabled() const { - return config_.rtp.ulpfec.ulpfec_payload_type != -1; + return config_.rtp.ulpfec_payload_type != -1; } bool RtpVideoStreamReceiver::IsRedEnabled() const { - return config_.rtp.ulpfec.red_payload_type != -1; + return config_.rtp.red_payload_type != -1; } bool RtpVideoStreamReceiver::IsRetransmissionsEnabled() const { diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc index ef04725e3c..0bf4587b2f 100644 --- a/video/video_quality_test.cc +++ b/video/video_quality_test.cc @@ -1552,26 +1552,20 @@ void VideoQualityTest::SetupVideo(Transport* send_transport, if (decode_all_receive_streams) { for (auto it = video_receive_configs_.begin(); it != video_receive_configs_.end(); ++it) { - it->rtp.ulpfec.red_payload_type = + it->rtp.red_payload_type = video_send_config_.rtp.ulpfec.red_payload_type; - it->rtp.ulpfec.ulpfec_payload_type = + it->rtp.ulpfec_payload_type = video_send_config_.rtp.ulpfec.ulpfec_payload_type; - it->rtp.ulpfec.red_rtx_payload_type = - video_send_config_.rtp.ulpfec.red_rtx_payload_type; it->rtp.rtx_associated_payload_types[video_send_config_.rtp.ulpfec .red_rtx_payload_type] = video_send_config_.rtp.ulpfec.red_payload_type; } } else { - video_receive_configs_[params_.ss.selected_stream] - .rtp.ulpfec.red_payload_type = + video_receive_configs_[params_.ss.selected_stream].rtp.red_payload_type = video_send_config_.rtp.ulpfec.red_payload_type; video_receive_configs_[params_.ss.selected_stream] - .rtp.ulpfec.ulpfec_payload_type = + .rtp.ulpfec_payload_type = video_send_config_.rtp.ulpfec.ulpfec_payload_type; - video_receive_configs_[params_.ss.selected_stream] - .rtp.ulpfec.red_rtx_payload_type = - video_send_config_.rtp.ulpfec.red_rtx_payload_type; video_receive_configs_[params_.ss.selected_stream] .rtp.rtx_associated_payload_types[video_send_config_.rtp.ulpfec .red_rtx_payload_type] = diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc index 0eeae213b3..faf47c7ffa 100644 --- a/video/video_send_stream_tests.cc +++ b/video/video_send_stream_tests.cc @@ -520,9 +520,9 @@ class UlpfecObserver : public test::EndToEndTest { send_config->rtp.extensions.push_back(RtpExtension( RtpExtension::kAbsSendTimeUri, test::kAbsSendTimeExtensionId)); } - (*receive_configs)[0].rtp.ulpfec.red_payload_type = + (*receive_configs)[0].rtp.red_payload_type = send_config->rtp.ulpfec.red_payload_type; - (*receive_configs)[0].rtp.ulpfec.ulpfec_payload_type = + (*receive_configs)[0].rtp.ulpfec_payload_type = send_config->rtp.ulpfec.ulpfec_payload_type; }