admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Delete member VideoReceiveStream::Config::Rtp::ulpfec.

Browse Source 
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}
This commit is contained in:
nisse 2017-09-26 02:49:21 -07:00 committed by Commit Bot
parent daea5bf2de
commit 3b3622fafc
12 changed files with 64 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
call/call_perf_tests.cc
View File

@ -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;

4
call/rampup_tests.cc
View File

@ -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

3
call/video_receive_stream.cc
View File

@ -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) {

10
call/video_receive_stream.h
View File

@ -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;

14
media/engine/webrtcvideoengine.cc
View File

@ -2228,6 +2228,7 @@ CricketDecoderFactoryAdapter::CreateVideoDecoder(
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 @@ 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;
}
}

10
media/engine/webrtcvideoengine_unittest.cc
View File

@ -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.";
}

57
video/end_to_end_tests.cc
View File

@ -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<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 @@ 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<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 @@ 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) {

2
video/receive_statistics_proxy.cc
View File

@ -366,7 +366,7 @@ void ReceiveStatisticsProxy::UpdateHistograms() {
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));

4
video/replay.cc
View File

@ -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(

8
video/rtp_video_stream_receiver.cc
View File

@ -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 {

14
video/video_quality_test.cc
View File

@ -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] =

4
video/video_send_stream_tests.cc
View File

@ -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;
}