Don't send FEC for H.264 with NACK enabled.
The H.264 does not contain picture IDs and are not sufficient to determine that a packet may be skipped. This causes retransmission requests for FEC that are currently dropped by the sender (since they should be redundant). The receiver is then unable to continue without having the packet gap filled (unlike VP8/VP9 which moves on since it has a consecutive stream of picture IDs). Even if FEC retransmission did work it's a huge waste of bandwidth, since it just adds additional overhead that has to be unconditionally transmitted. This bandwidth is better used to send higher-quality frames. BUG=webrtc:5264 R=stefan@webrtc.org Review URL: https://codereview.webrtc.org/1687303002 . Cr-Commit-Position: refs/heads/master@{#11601}
This commit is contained in:
parent
efce73e4ed
commit
25558ad819
@ -76,7 +76,7 @@ class EndToEndTest : public test::CallTest {
|
||||
}
|
||||
};
|
||||
|
||||
void DecodesRetransmittedFrame(bool use_rtx, bool use_red);
|
||||
void DecodesRetransmittedFrame(bool enable_rtx, bool enable_red);
|
||||
void ReceivesPliAndRecovers(int rtp_history_ms);
|
||||
void RespectsRtcpMode(RtcpMode rtcp_mode);
|
||||
void TestXrReceiverReferenceTimeReport(bool enable_rrtr);
|
||||
@ -701,18 +701,20 @@ TEST_F(EndToEndTest, DISABLED_ReceivedFecPacketsNotNacked) {
|
||||
|
||||
// This test drops second RTP packet with a marker bit set, makes sure it's
|
||||
// retransmitted and renders. Retransmission SSRCs are also checked.
|
||||
void EndToEndTest::DecodesRetransmittedFrame(bool use_rtx, bool use_red) {
|
||||
void EndToEndTest::DecodesRetransmittedFrame(bool enable_rtx, bool enable_red) {
|
||||
// Must be set high enough to allow the bitrate probing to finish.
|
||||
static const int kMinProbePackets = 30;
|
||||
static const int kDroppedFrameNumber = kMinProbePackets + 1;
|
||||
class RetransmissionObserver : public test::EndToEndTest,
|
||||
public I420FrameCallback {
|
||||
public:
|
||||
explicit RetransmissionObserver(bool use_rtx, bool use_red)
|
||||
RetransmissionObserver(bool enable_rtx, bool enable_red)
|
||||
: EndToEndTest(kDefaultTimeoutMs),
|
||||
payload_type_(GetPayloadType(false, use_red)),
|
||||
retransmission_ssrc_(use_rtx ? kSendRtxSsrcs[0] : kVideoSendSsrcs[0]),
|
||||
retransmission_payload_type_(GetPayloadType(use_rtx, use_red)),
|
||||
payload_type_(GetPayloadType(false, enable_red)),
|
||||
retransmission_ssrc_(enable_rtx ? kSendRtxSsrcs[0]
|
||||
: kVideoSendSsrcs[0]),
|
||||
retransmission_payload_type_(GetPayloadType(enable_rtx, enable_red)),
|
||||
encoder_(VideoEncoder::Create(VideoEncoder::EncoderType::kVp8)),
|
||||
marker_bits_observed_(0),
|
||||
num_packets_observed_(0),
|
||||
retransmitted_timestamp_(0),
|
||||
@ -790,6 +792,12 @@ void EndToEndTest::DecodesRetransmittedFrame(bool use_rtx, bool use_red) {
|
||||
(*receive_configs)[0].rtp.rtx[payload_type_].payload_type =
|
||||
kSendRtxPayloadType;
|
||||
}
|
||||
// Configure encoding and decoding with VP8, since generic packetization
|
||||
// doesn't support FEC with NACK.
|
||||
RTC_DCHECK_EQ(1u, (*receive_configs)[0].decoders.size());
|
||||
send_config->encoder_settings.encoder = encoder_.get();
|
||||
send_config->encoder_settings.payload_name = "VP8";
|
||||
(*receive_configs)[0].decoders[0].payload_name = "VP8";
|
||||
}
|
||||
|
||||
void PerformTest() override {
|
||||
@ -812,11 +820,13 @@ void EndToEndTest::DecodesRetransmittedFrame(bool use_rtx, bool use_red) {
|
||||
const int payload_type_;
|
||||
const uint32_t retransmission_ssrc_;
|
||||
const int retransmission_payload_type_;
|
||||
rtc::scoped_ptr<VideoEncoder> encoder_;
|
||||
const std::string payload_name_;
|
||||
int marker_bits_observed_;
|
||||
int num_packets_observed_;
|
||||
uint32_t retransmitted_timestamp_ GUARDED_BY(&crit_);
|
||||
bool frame_retransmitted_;
|
||||
} test(use_rtx, use_red);
|
||||
} test(enable_rtx, enable_red);
|
||||
|
||||
RunBaseTest(&test);
|
||||
}
|
||||
@ -2088,6 +2098,10 @@ void EndToEndTest::VerifyHistogramStats(bool use_rtx,
|
||||
use_rtx_(use_rtx),
|
||||
use_red_(use_red),
|
||||
screenshare_(screenshare),
|
||||
// This test uses NACK, so to send FEC we can't use a fake encoder.
|
||||
vp8_encoder_(
|
||||
use_red ? VideoEncoder::Create(VideoEncoder::EncoderType::kVp8)
|
||||
: nullptr),
|
||||
sender_call_(nullptr),
|
||||
receiver_call_(nullptr),
|
||||
start_runtime_ms_(-1) {}
|
||||
@ -2127,6 +2141,9 @@ void EndToEndTest::VerifyHistogramStats(bool use_rtx,
|
||||
if (use_red_) {
|
||||
send_config->rtp.fec.ulpfec_payload_type = kUlpfecPayloadType;
|
||||
send_config->rtp.fec.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.fec.red_payload_type = kRedPayloadType;
|
||||
(*receive_configs)[0].rtp.fec.ulpfec_payload_type = kUlpfecPayloadType;
|
||||
}
|
||||
@ -2156,6 +2173,7 @@ void EndToEndTest::VerifyHistogramStats(bool use_rtx,
|
||||
const bool use_rtx_;
|
||||
const bool use_red_;
|
||||
const bool screenshare_;
|
||||
const rtc::scoped_ptr<VideoEncoder> vp8_encoder_;
|
||||
Call* sender_call_;
|
||||
Call* receiver_call_;
|
||||
int64_t start_runtime_ms_;
|
||||
|
||||
@ -107,6 +107,35 @@ std::string VideoSendStream::Config::ToString() const {
|
||||
|
||||
namespace {
|
||||
|
||||
VideoCodecType PayloadNameToCodecType(const std::string& payload_name) {
|
||||
if (payload_name == "VP8")
|
||||
return kVideoCodecVP8;
|
||||
if (payload_name == "VP9")
|
||||
return kVideoCodecVP9;
|
||||
if (payload_name == "H264")
|
||||
return kVideoCodecH264;
|
||||
return kVideoCodecGeneric;
|
||||
}
|
||||
|
||||
bool PayloadTypeSupportsSkippingFecPackets(const std::string& payload_name) {
|
||||
switch (PayloadNameToCodecType(payload_name)) {
|
||||
case kVideoCodecVP8:
|
||||
case kVideoCodecVP9:
|
||||
return true;
|
||||
case kVideoCodecH264:
|
||||
case kVideoCodecGeneric:
|
||||
return false;
|
||||
case kVideoCodecI420:
|
||||
case kVideoCodecRED:
|
||||
case kVideoCodecULPFEC:
|
||||
case kVideoCodecUnknown:
|
||||
RTC_NOTREACHED();
|
||||
return false;
|
||||
}
|
||||
RTC_NOTREACHED();
|
||||
return false;
|
||||
}
|
||||
|
||||
CpuOveruseOptions GetCpuOveruseOptions(bool full_overuse_time) {
|
||||
CpuOveruseOptions options;
|
||||
if (full_overuse_time) {
|
||||
@ -215,7 +244,19 @@ VideoSendStream::VideoSendStream(
|
||||
|
||||
// Enable NACK, FEC or both.
|
||||
const bool enable_protection_nack = config_.rtp.nack.rtp_history_ms > 0;
|
||||
const bool enable_protection_fec = config_.rtp.fec.red_payload_type != -1;
|
||||
bool enable_protection_fec = config_.rtp.fec.red_payload_type != -1;
|
||||
// Payload types without picture ID cannot determine that a stream is complete
|
||||
// without retransmitting FEC, so using FEC + NACK for H.264 (for instance) is
|
||||
// a waste of bandwidth since FEC packets still have to be transmitted. Note
|
||||
// that this is not the case with FLEXFEC.
|
||||
if (enable_protection_nack &&
|
||||
!PayloadTypeSupportsSkippingFecPackets(
|
||||
config_.encoder_settings.payload_name)) {
|
||||
LOG(LS_WARNING) << "Transmitting payload type without picture ID using"
|
||||
"NACK+FEC is a waste of bandwidth since FEC packets "
|
||||
"also have to be retransmitted. Disabling FEC.";
|
||||
enable_protection_fec = false;
|
||||
}
|
||||
// TODO(changbin): Should set RTX for RED mapping in RTP sender in future.
|
||||
vie_channel_.SetProtectionMode(enable_protection_nack, enable_protection_fec,
|
||||
config_.rtp.fec.red_payload_type,
|
||||
@ -324,15 +365,8 @@ bool VideoSendStream::ReconfigureVideoEncoder(
|
||||
|
||||
VideoCodec video_codec;
|
||||
memset(&video_codec, 0, sizeof(video_codec));
|
||||
if (config_.encoder_settings.payload_name == "VP8") {
|
||||
video_codec.codecType = kVideoCodecVP8;
|
||||
} else if (config_.encoder_settings.payload_name == "VP9") {
|
||||
video_codec.codecType = kVideoCodecVP9;
|
||||
} else if (config_.encoder_settings.payload_name == "H264") {
|
||||
video_codec.codecType = kVideoCodecH264;
|
||||
} else {
|
||||
video_codec.codecType = kVideoCodecGeneric;
|
||||
}
|
||||
video_codec.codecType =
|
||||
PayloadNameToCodecType(config_.encoder_settings.payload_name);
|
||||
|
||||
switch (config.content_type) {
|
||||
case VideoEncoderConfig::ContentType::kRealtimeVideo:
|
||||
|
||||
@ -308,48 +308,55 @@ class FakeReceiveStatistics : public NullReceiveStatistics {
|
||||
StatisticianMap stats_map_;
|
||||
};
|
||||
|
||||
class FecObserver : public test::SendTest {
|
||||
class FecObserver : public test::EndToEndTest {
|
||||
public:
|
||||
explicit FecObserver(bool header_extensions_enabled)
|
||||
: SendTest(VideoSendStreamTest::kDefaultTimeoutMs),
|
||||
FecObserver(bool header_extensions_enabled,
|
||||
bool use_nack,
|
||||
bool expect_red,
|
||||
const std::string& codec)
|
||||
: EndToEndTest(VideoSendStreamTest::kDefaultTimeoutMs),
|
||||
payload_name_(codec),
|
||||
use_nack_(use_nack),
|
||||
expect_red_(expect_red),
|
||||
send_count_(0),
|
||||
received_media_(false),
|
||||
received_fec_(false),
|
||||
header_extensions_enabled_(header_extensions_enabled) {}
|
||||
header_extensions_enabled_(header_extensions_enabled) {
|
||||
if (codec == "H264") {
|
||||
encoder_.reset(new test::FakeH264Encoder(Clock::GetRealTimeClock()));
|
||||
} else if (codec == "VP8") {
|
||||
encoder_.reset(VideoEncoder::Create(VideoEncoder::EncoderType::kVp8));
|
||||
} else if (codec == "VP9") {
|
||||
encoder_.reset(VideoEncoder::Create(VideoEncoder::EncoderType::kVp9));
|
||||
} else {
|
||||
RTC_NOTREACHED();
|
||||
}
|
||||
}
|
||||
|
||||
private:
|
||||
Action OnSendRtp(const uint8_t* packet, size_t length) override {
|
||||
RTPHeader header;
|
||||
EXPECT_TRUE(parser_->Parse(packet, length, &header));
|
||||
|
||||
// Send lossy receive reports to trigger FEC enabling.
|
||||
if (send_count_++ % 2 != 0) {
|
||||
// Receive statistics reporting having lost 50% of the packets.
|
||||
FakeReceiveStatistics lossy_receive_stats(
|
||||
VideoSendStreamTest::kVideoSendSsrcs[0], header.sequenceNumber,
|
||||
send_count_ / 2, 127);
|
||||
RTCPSender rtcp_sender(false, Clock::GetRealTimeClock(),
|
||||
&lossy_receive_stats, nullptr, nullptr,
|
||||
transport_adapter_.get());
|
||||
|
||||
rtcp_sender.SetRTCPStatus(RtcpMode::kReducedSize);
|
||||
rtcp_sender.SetRemoteSSRC(VideoSendStreamTest::kVideoSendSsrcs[0]);
|
||||
|
||||
RTCPSender::FeedbackState feedback_state;
|
||||
|
||||
EXPECT_EQ(0, rtcp_sender.SendRTCP(feedback_state, kRtcpRr));
|
||||
}
|
||||
|
||||
++send_count_;
|
||||
int encapsulated_payload_type = -1;
|
||||
if (header.payloadType == VideoSendStreamTest::kRedPayloadType) {
|
||||
EXPECT_TRUE(expect_red_);
|
||||
encapsulated_payload_type = static_cast<int>(packet[header.headerLength]);
|
||||
if (encapsulated_payload_type !=
|
||||
VideoSendStreamTest::kFakeVideoSendPayloadType)
|
||||
VideoSendStreamTest::kFakeVideoSendPayloadType) {
|
||||
EXPECT_EQ(VideoSendStreamTest::kUlpfecPayloadType,
|
||||
encapsulated_payload_type);
|
||||
}
|
||||
} else {
|
||||
EXPECT_EQ(VideoSendStreamTest::kFakeVideoSendPayloadType,
|
||||
header.payloadType);
|
||||
if (static_cast<size_t>(header.headerLength + header.paddingLength) <
|
||||
length) {
|
||||
// Not padding-only, media received outside of RED.
|
||||
EXPECT_FALSE(expect_red_);
|
||||
received_media_ = true;
|
||||
}
|
||||
}
|
||||
|
||||
if (header_extensions_enabled_) {
|
||||
@ -379,14 +386,27 @@ class FecObserver : public test::SendTest {
|
||||
}
|
||||
}
|
||||
|
||||
if (received_media_ && received_fec_ && send_count_ > 100)
|
||||
observation_complete_.Set();
|
||||
if (send_count_ > 100 && received_media_) {
|
||||
if (received_fec_ || !expect_red_)
|
||||
observation_complete_.Set();
|
||||
}
|
||||
|
||||
prev_header_ = header;
|
||||
|
||||
return SEND_PACKET;
|
||||
}
|
||||
|
||||
test::PacketTransport* CreateSendTransport(Call* sender_call) override {
|
||||
// At low RTT (< kLowRttNackMs) -> NACK only, no FEC.
|
||||
// Configure some network delay.
|
||||
const int kNetworkDelayMs = 100;
|
||||
FakeNetworkPipe::Config config;
|
||||
config.loss_percent = 50;
|
||||
config.queue_delay_ms = kNetworkDelayMs;
|
||||
return new test::PacketTransport(sender_call, this,
|
||||
test::PacketTransport::kSender, config);
|
||||
}
|
||||
|
||||
void ModifyVideoConfigs(
|
||||
VideoSendStream::Config* send_config,
|
||||
std::vector<VideoReceiveStream::Config>* receive_configs,
|
||||
@ -394,10 +414,17 @@ class FecObserver : public test::SendTest {
|
||||
transport_adapter_.reset(
|
||||
new internal::TransportAdapter(send_config->send_transport));
|
||||
transport_adapter_->Enable();
|
||||
if (use_nack_) {
|
||||
send_config->rtp.nack.rtp_history_ms =
|
||||
(*receive_configs)[0].rtp.nack.rtp_history_ms =
|
||||
VideoSendStreamTest::kNackRtpHistoryMs;
|
||||
}
|
||||
send_config->encoder_settings.encoder = encoder_.get();
|
||||
send_config->encoder_settings.payload_name = payload_name_;
|
||||
send_config->rtp.fec.red_payload_type =
|
||||
VideoSendStreamTest::kRedPayloadType;
|
||||
VideoSendStreamTest::kRedPayloadType;
|
||||
send_config->rtp.fec.ulpfec_payload_type =
|
||||
VideoSendStreamTest::kUlpfecPayloadType;
|
||||
VideoSendStreamTest::kUlpfecPayloadType;
|
||||
if (header_extensions_enabled_) {
|
||||
send_config->rtp.extensions.push_back(RtpExtension(
|
||||
RtpExtension::kAbsSendTime, test::kAbsSendTimeExtensionId));
|
||||
@ -405,6 +432,10 @@ class FecObserver : public test::SendTest {
|
||||
RtpExtension(RtpExtension::kTransportSequenceNumber,
|
||||
test::kTransportSequenceNumberExtensionId));
|
||||
}
|
||||
(*receive_configs)[0].rtp.fec.red_payload_type =
|
||||
send_config->rtp.fec.red_payload_type;
|
||||
(*receive_configs)[0].rtp.fec.ulpfec_payload_type =
|
||||
send_config->rtp.fec.ulpfec_payload_type;
|
||||
}
|
||||
|
||||
void PerformTest() override {
|
||||
@ -412,6 +443,10 @@ class FecObserver : public test::SendTest {
|
||||
}
|
||||
|
||||
rtc::scoped_ptr<internal::TransportAdapter> transport_adapter_;
|
||||
rtc::scoped_ptr<VideoEncoder> encoder_;
|
||||
const std::string payload_name_;
|
||||
const bool use_nack_;
|
||||
const bool expect_red_;
|
||||
int send_count_;
|
||||
bool received_media_;
|
||||
bool received_fec_;
|
||||
@ -420,14 +455,37 @@ class FecObserver : public test::SendTest {
|
||||
};
|
||||
|
||||
TEST_F(VideoSendStreamTest, SupportsFecWithExtensions) {
|
||||
FecObserver test(true);
|
||||
|
||||
FecObserver test(true, false, true, "VP8");
|
||||
RunBaseTest(&test);
|
||||
}
|
||||
|
||||
TEST_F(VideoSendStreamTest, SupportsFecWithoutExtensions) {
|
||||
FecObserver test(false);
|
||||
FecObserver test(false, false, true, "VP8");
|
||||
RunBaseTest(&test);
|
||||
}
|
||||
|
||||
// The FEC scheme used is not efficient for H264, so we should not use RED/FEC
|
||||
// since we'll still have to re-request FEC packets, effectively wasting
|
||||
// bandwidth since the receiver has to wait for FEC retransmissions to determine
|
||||
// that the received state is actually decodable.
|
||||
TEST_F(VideoSendStreamTest, DoesNotUtilizeRedForH264WithNackEnabled) {
|
||||
FecObserver test(false, true, false, "H264");
|
||||
RunBaseTest(&test);
|
||||
}
|
||||
|
||||
// Without retransmissions FEC for H264 is fine.
|
||||
TEST_F(VideoSendStreamTest, DoesUtilizeRedForH264WithoutNackEnabled) {
|
||||
FecObserver test(false, false, true, "H264");
|
||||
RunBaseTest(&test);
|
||||
}
|
||||
|
||||
TEST_F(VideoSendStreamTest, DoesUtilizeRedForVp8WithNackEnabled) {
|
||||
FecObserver test(false, true, true, "VP8");
|
||||
RunBaseTest(&test);
|
||||
}
|
||||
|
||||
TEST_F(VideoSendStreamTest, DoesUtilizeRedForVp9WithNackEnabled) {
|
||||
FecObserver test(false, true, true, "VP9");
|
||||
RunBaseTest(&test);
|
||||
}
|
||||
|
||||
|
||||
@ -414,13 +414,15 @@ void ViEChannel::SetProtectionMode(bool enable_nack,
|
||||
bool enable_fec,
|
||||
int payload_type_red,
|
||||
int payload_type_fec) {
|
||||
// Validate payload types.
|
||||
if (enable_fec) {
|
||||
// Validate payload types. If either RED or FEC payload types are set then
|
||||
// both should be. If FEC is enabled then they both have to be set.
|
||||
if (enable_fec || payload_type_red != -1 || payload_type_fec != -1) {
|
||||
RTC_DCHECK_GE(payload_type_red, 0);
|
||||
RTC_DCHECK_GE(payload_type_fec, 0);
|
||||
RTC_DCHECK_LE(payload_type_red, 127);
|
||||
RTC_DCHECK_LE(payload_type_fec, 127);
|
||||
} else {
|
||||
// Payload types unset.
|
||||
RTC_DCHECK_EQ(payload_type_red, -1);
|
||||
RTC_DCHECK_EQ(payload_type_fec, -1);
|
||||
// Set to valid uint8_ts to be castable later without signed overflows.
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user