Delete member VideoReceiveStream::Config::Rtp::ulpfec.
Replaced with scalars ulpfec_payload_type and red_payload_type.
In particular, ulpfec.red_rtx_payload_type, which duplicated info in
rtx_associated_payload_types, is deleted. This is a followup to cl
https://codereview.webrtc.org/3012963002.
BUG=webrtc:7135
Review-Url: https://codereview.webrtc.org/3019453002
Cr-Commit-Position: refs/heads/master@{#19965}
diff --git a/call/call_perf_tests.cc b/call/call_perf_tests.cc
index 58435f0..5dd69bd 100644
--- a/call/call_perf_tests.cc
+++ b/call/call_perf_tests.cc
@@ -238,9 +238,8 @@
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 da7095c..6ce2efd 100644
--- a/call/rampup_tests.cc
+++ b/call/rampup_tests.cc
@@ -202,9 +202,9 @@
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 76892ea..f338805 100644
--- a/call/video_receive_stream.cc
+++ b/call/video_receive_stream.cc
@@ -110,7 +110,8 @@
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 7d996ab..de6f712 100644
--- a/call/video_receive_stream.h
+++ b/call/video_receive_stream.h
@@ -166,13 +166,9 @@
// 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 29fc442..69af25b 100644
--- a/media/engine/webrtcvideoengine.cc
+++ b/media/engine/webrtcvideoengine.cc
@@ -2228,6 +2228,7 @@
void WebRtcVideoChannel::WebRtcVideoReceiveStream::ConfigureCodecs(
const std::vector<VideoCodecSettings>& 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 @@
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 1b3ec72..80d8a07 100644
--- a/media/engine/webrtcvideoengine_unittest.cc
+++ b/media/engine/webrtcvideoengine_unittest.cc
@@ -3326,14 +3326,14 @@
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(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 @@
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 @@
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 06a7118..2ae3cee 100644
--- a/video/end_to_end_tests.cc
+++ b/video/end_to_end_tests.cc
@@ -140,7 +140,7 @@
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 @@
// 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 @@
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 @@
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 @@
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 @@
<< "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 @@
}
void EndToEndTest::VerifyHistogramStats(bool use_rtx,
- bool use_red,
+ bool use_fec,
bool screenshare) {
class StatsObserver : public test::EndToEndTest,
public rtc::VideoSinkInterface<VideoFrame> {
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 @@
(*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 @@
(*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 @@
rtc::CriticalSection crit_;
const bool use_rtx_;
- const bool use_red_;
+ const bool use_fec_;
const bool screenshare_;
const std::unique_ptr<VideoEncoder> 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 @@
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 @@
<< "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 b19ec20..b797738 100644
--- a/video/receive_statistics_proxy.cc
+++ b/video/receive_statistics_proxy.cc
@@ -366,7 +366,7 @@
static_cast<int>(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<int>(rtp_rtx.fec.TotalBytes() * 8 / elapsed_sec / 1000));
diff --git a/video/replay.cc b/video/replay.cc
index 82c9de1..b9214b8 100644
--- a/video/replay.cc
+++ b/video/replay.cc
@@ -215,8 +215,8 @@
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 881ab34..c5483e9 100644
--- a/video/rtp_video_stream_receiver.cc
+++ b/video/rtp_video_stream_receiver.cc
@@ -148,7 +148,7 @@
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 @@
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 @@
}
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 ef04725..0bf4587 100644
--- a/video/video_quality_test.cc
+++ b/video/video_quality_test.cc
@@ -1552,27 +1552,21 @@
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] =
video_send_config_.rtp.ulpfec.red_payload_type;
diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc
index 0eeae21..faf47c7 100644
--- a/video/video_send_stream_tests.cc
+++ b/video/video_send_stream_tests.cc
@@ -520,9 +520,9 @@
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;
}
