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

Simplify {,Set}UlpfecConfig interface.

Browse Source 
Prior to this change, we signalled that ULPFEC was disabled
through a bool, but that RED was disabled by setting its
payload type to -1. The latter is consistent with how we
disable RED/ULPFEC in the config, so this CL removes the
ULPFEC bool from the {,Set}UlpfecConfig chain of member
functions.

BUG=webrtc:5654

Review-Url: https://codereview.webrtc.org/2460533002
Cr-Commit-Position: refs/heads/master@{#14944}
This commit is contained in:
brandtr 2016-11-07 03:05:06 -08:00 committed by Commit bot
parent 87d7d77700
commit f1bb476050
11 changed files with 74 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
webrtc/modules/rtp_rtcp/include/rtp_rtcp.h
View File

@ -434,9 +434,16 @@ class RtpRtcp : public Module {
// Video
// **************************************************************************
// Turn on/off ULPFEC.
virtual void SetUlpfecConfig(bool enabled,
int red_payload_type,
// Set RED and ULPFEC payload types. A payload type of -1 means that the
// corresponding feature is turned off. Note that we DO NOT support enabling
// ULPFEC without enabling RED. However, we DO support enabling RED without
// enabling ULPFEC. This is due to an RED/RTX workaround, where the receiver
// assumes that RTX packets carry RED if RED has been configured in the SDP,
// regardless of what RTX payload type mapping was negotiated in the SDP.
// TODO(brandtr): Update this comment when we have removed the RED/RTX
// send-side workaround, i.e., when we do not support enabling RED without
// enabling ULPFEC.
virtual void SetUlpfecConfig(int red_payload_type,
int ulpfec_payload_type) = 0;
virtual int32_t SetFecParameters(const FecProtectionParams* delta_params,

6
webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h
View File

@ -186,10 +186,8 @@ class MockRtpRtcp : public RtpRtcp {
int32_t(bool enable, uint8_t id));
MOCK_METHOD1(SetAudioLevel, int32_t(uint8_t level_dbov));
MOCK_METHOD1(SetTargetSendBitrate, void(uint32_t bitrate_bps));
MOCK_METHOD3(SetUlpfecConfig,
void(bool ulpfec_enabled,
int red_payload_type,
int fec_payload_type));
MOCK_METHOD2(SetUlpfecConfig,
void(int red_payload_type, int fec_payload_type));
MOCK_METHOD2(SetFecParameters,
int32_t(const FecProtectionParams* delta_params,
const FecProtectionParams* key_params));

5
webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
View File

@ -789,10 +789,9 @@ int32_t ModuleRtpRtcpImpl::SendRTCPSliceLossIndication(
GetFeedbackState(), kRtcpSli, 0, 0, false, picture_id);
}
void ModuleRtpRtcpImpl::SetUlpfecConfig(bool enabled,
int red_payload_type,
void ModuleRtpRtcpImpl::SetUlpfecConfig(int red_payload_type,
int ulpfec_payload_type) {
rtp_sender_.SetUlpfecConfig(enabled, red_payload_type, ulpfec_payload_type);
rtp_sender_.SetUlpfecConfig(red_payload_type, ulpfec_payload_type);
}
int32_t ModuleRtpRtcpImpl::SetFecParameters(

4
webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h
View File

@ -277,9 +277,7 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp {
// Send a request for a keyframe.
int32_t RequestKeyFrame() override;
void SetUlpfecConfig(bool enabled,
int red_payload_type,
int ulpfec_payload_type) override;
void SetUlpfecConfig(int red_payload_type, int ulpfec_payload_type) override;
int32_t SetFecParameters(const FecProtectionParams* delta_params,
const FecProtectionParams* key_params) override;

10
webrtc/modules/rtp_rtcp/source/rtp_sender.cc
View File

@ -789,10 +789,10 @@ bool RTPSender::IsFecPacket(const RtpPacketToSend& packet) const {
if (!video_) {
return false;
}
bool fec_enabled;
int pt_red;
int pt_fec;
video_->GetUlpfecConfig(&fec_enabled, &pt_red, &pt_fec);
video_->GetUlpfecConfig(&pt_red, &pt_fec);
const bool fec_enabled = (pt_fec != -1);
return fec_enabled && static_cast<int>(packet.PayloadType()) == pt_red &&
static_cast<int>(packet.payload()[0]) == pt_fec;
}
@ -1131,11 +1131,9 @@ RtpVideoCodecTypes RTPSender::VideoCodecType() const {
return video_->VideoCodecType();
}
void RTPSender::SetUlpfecConfig(bool enabled,
int red_payload_type,
int ulpfec_payload_type) {
void RTPSender::SetUlpfecConfig(int red_payload_type, int ulpfec_payload_type) {
RTC_DCHECK(!audio_configured_);
video_->SetUlpfecConfig(enabled, red_payload_type, ulpfec_payload_type);
video_->SetUlpfecConfig(red_payload_type, ulpfec_payload_type);
}
int32_t RTPSender::SetFecParameters(

6
webrtc/modules/rtp_rtcp/source/rtp_sender.h
View File

@ -183,10 +183,8 @@ class RTPSender {
uint32_t MaxConfiguredBitrateVideo() const;
// FEC.
void SetUlpfecConfig(bool enabled,
int red_payload_type,
int ulpfec_payload_type);
// ULPFEC.
void SetUlpfecConfig(int red_payload_type, int ulpfec_payload_type);
int32_t SetFecParameters(const FecProtectionParams *delta_params,
const FecProtectionParams *key_params);

4
webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
View File

@ -1123,8 +1123,8 @@ TEST_F(RtpSenderTestWithoutPacer, StreamDataCountersCallbacks) {
expected.transmitted.packets = 3;
callback.Matches(ssrc, expected);
// Send FEC.
rtp_sender_->SetUlpfecConfig(true, kRedPayloadType, kUlpfecPayloadType);
// Send ULPFEC.
rtp_sender_->SetUlpfecConfig(kRedPayloadType, kUlpfecPayloadType);
FecProtectionParams fec_params;
fec_params.fec_mask_type = kFecMaskRandom;
fec_params.fec_rate = 1;

41
webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc
View File

@ -50,9 +50,8 @@ RTPSenderVideo::RTPSenderVideo(Clock* clock, RTPSender* rtp_sender)
video_type_(kRtpVideoGeneric),
retransmission_settings_(kRetransmitBaseLayer),
last_rotation_(kVideoRotation_0),
fec_enabled_(false),
red_payload_type_(-1),
fec_payload_type_(-1),
ulpfec_payload_type_(-1),
delta_fec_params_{0, 1, kFecMaskRandom},
key_fec_params_{0, 1, kFecMaskRandom},
fec_bitrate_(1000, RateStatistics::kBpsScale),
@ -139,7 +138,7 @@ void RTPSenderVideo::SendVideoPacketAsRed(
uint16_t first_fec_sequence_number =
rtp_sender_->AllocateSequenceNumber(num_fec_packets);
fec_packets = ulpfec_generator_.GetUlpfecPacketsAsRed(
red_payload_type_, fec_payload_type_, first_fec_sequence_number,
red_payload_type_, ulpfec_payload_type_, first_fec_sequence_number,
media_packet->headers_size());
RTC_DCHECK_EQ(num_fec_packets, fec_packets.size());
if (retransmission_settings_ & kRetransmitFECPackets)
@ -179,36 +178,42 @@ void RTPSenderVideo::SendVideoPacketAsRed(
}
}
void RTPSenderVideo::SetUlpfecConfig(bool enabled,
int red_payload_type,
void RTPSenderVideo::SetUlpfecConfig(int red_payload_type,
int ulpfec_payload_type) {
RTC_DCHECK(!enabled || red_payload_type > 0);
// Sanity check. Per the definition of UlpfecConfig (see config.h),
// a payload type of -1 means that the corresponding feature is
// turned off.
RTC_DCHECK_GE(red_payload_type, -1);
RTC_DCHECK_LE(red_payload_type, 127);
RTC_DCHECK_GE(ulpfec_payload_type, -1);
RTC_DCHECK_LE(ulpfec_payload_type, 127);
rtc::CritScope cs(&crit_);
fec_enabled_ = enabled;
red_payload_type_ = red_payload_type;
fec_payload_type_ = ulpfec_payload_type;
ulpfec_payload_type_ = ulpfec_payload_type;
// Must not enable ULPFEC without RED.
// TODO(brandtr): We currently support enabling RED without ULPFEC. Change
// this when we have removed the RED/RTX send-side workaround, so that we
// ensure that RED and ULPFEC are only enabled together.
RTC_DCHECK(red_enabled() || !ulpfec_enabled());
// Reset FEC rates.
delta_fec_params_ = FecProtectionParams{0, 1, kFecMaskRandom};
key_fec_params_ = FecProtectionParams{0, 1, kFecMaskRandom};
}
void RTPSenderVideo::GetUlpfecConfig(bool* enabled,
int* red_payload_type,
void RTPSenderVideo::GetUlpfecConfig(int* red_payload_type,
int* ulpfec_payload_type) const {
rtc::CritScope cs(&crit_);
*enabled = fec_enabled_;
*red_payload_type = red_payload_type_;
*ulpfec_payload_type = fec_payload_type_;
*ulpfec_payload_type = ulpfec_payload_type_;
}
size_t RTPSenderVideo::FecPacketOverhead() const {
rtc::CritScope cs(&crit_);
size_t overhead = 0;
if (red_payload_type_ != -1) {
if (red_enabled()) {
// Overhead is FEC headers plus RED for FEC header plus anything in RTP
// header beyond the 12 bytes base header (CSRC list, extensions...)
// This reason for the header extensions to be included here is that
@ -217,7 +222,7 @@ size_t RTPSenderVideo::FecPacketOverhead() const {
return ulpfec_generator_.MaxPacketOverhead() + kRedForFecHeaderLength +
(rtp_sender_->RtpHeaderLength() - kRtpHeaderSize);
}
if (fec_enabled_)
if (ulpfec_enabled())
overhead += ulpfec_generator_.MaxPacketOverhead();
return overhead;
}
@ -227,7 +232,7 @@ void RTPSenderVideo::SetFecParameters(const FecProtectionParams* delta_params,
rtc::CritScope cs(&crit_);
RTC_DCHECK(delta_params);
RTC_DCHECK(key_params);
if (fec_enabled_) {
if (ulpfec_enabled()) {
delta_fec_params_ = *delta_params;
key_fec_params_ = *key_params;
}
@ -283,7 +288,7 @@ bool RTPSenderVideo::SendVideo(RtpVideoCodecTypes video_type,
video_header ? &(video_header->codecHeader) : nullptr, frame_type));
StorageType storage;
int red_payload_type;
bool red_enabled;
bool first_frame = first_frame_sent_();
{
rtc::CritScope cs(&crit_);
@ -291,7 +296,7 @@ bool RTPSenderVideo::SendVideo(RtpVideoCodecTypes video_type,
frame_type == kVideoFrameKey ? &key_fec_params_ : &delta_fec_params_;
ulpfec_generator_.SetFecParameters(fec_params);
storage = packetizer->GetStorageType(retransmission_settings_);
red_payload_type = red_payload_type_;
red_enabled = this->red_enabled();
}
// TODO(changbin): we currently don't support to configure the codec to
@ -318,7 +323,7 @@ bool RTPSenderVideo::SendVideo(RtpVideoCodecTypes video_type,
if (!rtp_sender_->AssignSequenceNumber(packet.get()))
return false;
if (red_payload_type != -1) {
if (red_enabled) {
SendVideoPacketAsRed(std::move(packet), storage,
packetizer->GetProtectionType() == kProtectedPacket);
} else {

24
webrtc/modules/rtp_rtcp/source/rtp_sender_video.h
View File

@ -59,14 +59,9 @@ class RTPSenderVideo {
void SetVideoCodecType(RtpVideoCodecTypes type);
// FEC
void SetUlpfecConfig(bool enabled,
int red_payload_type,
int ulpfec_payload_type);
void GetUlpfecConfig(bool* enabled,
int* red_payload_type,
int* ulpfec_payload_type) const;
// ULPFEC.
void SetUlpfecConfig(int red_payload_type, int ulpfec_payload_type);
void GetUlpfecConfig(int* red_payload_type, int* ulpfec_payload_type) const;
void SetFecParameters(const FecProtectionParams* delta_params,
const FecProtectionParams* key_params);
@ -85,6 +80,14 @@ class RTPSenderVideo {
StorageType media_packet_storage,
bool protect);
bool red_enabled() const EXCLUSIVE_LOCKS_REQUIRED(crit_) {
return red_payload_type_ >= 0;
}
bool ulpfec_enabled() const EXCLUSIVE_LOCKS_REQUIRED(crit_) {
return ulpfec_payload_type_ >= 0;
}
RTPSender* const rtp_sender_;
Clock* const clock_;
@ -96,10 +99,9 @@ class RTPSenderVideo {
int32_t retransmission_settings_ GUARDED_BY(crit_);
VideoRotation last_rotation_ GUARDED_BY(encoder_checker_);
// FEC
bool fec_enabled_ GUARDED_BY(crit_);
// RED/ULPFEC.
int red_payload_type_ GUARDED_BY(crit_);
int fec_payload_type_ GUARDED_BY(crit_);
int ulpfec_payload_type_ GUARDED_BY(crit_);
FecProtectionParams delta_fec_params_ GUARDED_BY(crit_);
FecProtectionParams key_fec_params_ GUARDED_BY(crit_);
UlpfecGenerator ulpfec_generator_ GUARDED_BY(crit_);

6
webrtc/video/rtp_stream_receiver.cc
View File

@ -178,8 +178,10 @@ RtpStreamReceiver::RtpStreamReceiver(
config_.rtp.ulpfec.red_rtx_payload_type,
config_.rtp.ulpfec.red_payload_type);
}
rtp_rtcp_->SetUlpfecConfig(true, config_.rtp.ulpfec.red_payload_type,
// TODO(brandtr): It doesn't seem that |rtp_rtcp_| is used for sending any
// RTP packets. Investigate if this is the case, and if so, remove this
// call, since there are no RTP packets to protect with RED+ULPFEC.
rtp_rtcp_->SetUlpfecConfig(config_.rtp.ulpfec.red_payload_type,
config_.rtp.ulpfec.ulpfec_payload_type);
}

22
webrtc/video/video_send_stream.cc
View File

@ -934,7 +934,8 @@ void VideoSendStreamImpl::ConfigureProtection() {
RTC_DCHECK_RUN_ON(worker_queue_);
// Enable NACK, FEC or both.
const bool enable_protection_nack = config_->rtp.nack.rtp_history_ms > 0;
bool enable_protection_fec = config_->rtp.ulpfec.ulpfec_payload_type != -1;
const int red_payload_type = config_->rtp.ulpfec.red_payload_type;
int ulpfec_payload_type = config_->rtp.ulpfec.ulpfec_payload_type;
// 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
@ -945,7 +946,7 @@ void VideoSendStreamImpl::ConfigureProtection() {
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;
ulpfec_payload_type = -1;
}
// TODO(brandtr): Remove the workaround described below.
@ -961,13 +962,13 @@ void VideoSendStreamImpl::ConfigureProtection() {
// using the wrong payload type.
// Verify validity of provided payload types.
if (config_->rtp.ulpfec.red_payload_type != -1) {
RTC_DCHECK_GE(config_->rtp.ulpfec.red_payload_type, 0);
RTC_DCHECK_LE(config_->rtp.ulpfec.red_payload_type, 127);
if (red_payload_type != -1) {
RTC_DCHECK_GE(red_payload_type, 0);
RTC_DCHECK_LE(red_payload_type, 127);
}
if (config_->rtp.ulpfec.ulpfec_payload_type != -1) {
RTC_DCHECK_GE(config_->rtp.ulpfec.ulpfec_payload_type, 0);
RTC_DCHECK_LE(config_->rtp.ulpfec.ulpfec_payload_type, 127);
if (ulpfec_payload_type != -1) {
RTC_DCHECK_GE(ulpfec_payload_type, 0);
RTC_DCHECK_LE(ulpfec_payload_type, 127);
}
for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) {
@ -977,12 +978,11 @@ void VideoSendStreamImpl::ConfigureProtection() {
kMinSendSidePacketHistorySize);
// Set FEC.
for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) {
rtp_rtcp->SetUlpfecConfig(enable_protection_fec,
config_->rtp.ulpfec.red_payload_type,
config_->rtp.ulpfec.ulpfec_payload_type);
rtp_rtcp->SetUlpfecConfig(red_payload_type, ulpfec_payload_type);
}
}
const bool enable_protection_fec = (ulpfec_payload_type != -1);
protection_bitrate_calculator_.SetProtectionMethod(enable_protection_fec,
enable_protection_nack);
}