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

Allow VideoTimingExtension to be used with FEC

Browse Source 
This CL allows for FEC protection of packets with VideoTimingExtension by
zero-ing out data, which is changed after FEC protection is generated (i.e
in the pacer or by the SFU).

Actual FEC protection of these packets would be enabled later, when all
modern receivers have this change.

Bug: webrtc:10750
Change-Id: If4785392204d68cb8527629727b5c062f9fb6600
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/143760
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28396}
This commit is contained in:
Ilya Nikolaevskiy 2019-06-26 14:39:36 +02:00 committed by Commit Bot
parent e4ac723bdc
commit 2d821c3cbc
12 changed files with 100 additions and 131 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
modules/rtp_rtcp/include/ulpfec_receiver.h
View File

@ -11,6 +11,10 @@
#ifndef MODULES_RTP_RTCP_INCLUDE_ULPFEC_RECEIVER_H_
#define MODULES_RTP_RTCP_INCLUDE_ULPFEC_RECEIVER_H_
#include <memory>
#include "api/array_view.h"
#include "api/rtp_parameters.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
namespace webrtc {
@ -30,8 +34,10 @@ struct FecPacketCounter {
class UlpfecReceiver {
public:
static UlpfecReceiver* Create(uint32_t ssrc,
RecoveredPacketReceiver* callback);
static std::unique_ptr<UlpfecReceiver> Create(
uint32_t ssrc,
RecoveredPacketReceiver* callback,
rtc::ArrayView<const RtpExtension> extensions);
virtual ~UlpfecReceiver() {}

4
modules/rtp_rtcp/source/flexfec_receiver.cc
View File

@ -123,10 +123,10 @@ FlexfecReceiver::AddReceivedPacket(const RtpPacketReceived& packet) {
received_packet->is_fec = false;
// Insert entire packet into erasure code.
// TODO(brandtr): Remove this memcpy too.
received_packet->pkt = rtc::scoped_refptr<ForwardErrorCorrection::Packet>(
new ForwardErrorCorrection::Packet());
memcpy(received_packet->pkt->data, packet.data(), packet.size());
// Create a copy and fill with zeros all mutable extensions.
packet.CopyAndZeroMutableExtensions(received_packet->pkt->data);
received_packet->pkt->length = packet.size();
}

45
modules/rtp_rtcp/source/rtp_packet.cc
View File

@ -156,6 +156,51 @@ void RtpPacket::SetSsrc(uint32_t ssrc) {
ByteWriter<uint32_t>::WriteBigEndian(WriteAt(8), ssrc);
}
void RtpPacket::CopyAndZeroMutableExtensions(
rtc::ArrayView<uint8_t> buffer) const {
RTC_CHECK_GE(buffer.size(), buffer_.size());
memcpy(buffer.data(), buffer_.cdata(), buffer_.size());
for (const ExtensionInfo& extension : extension_entries_) {
switch (extensions_.GetType(extension.id)) {
case RTPExtensionType::kRtpExtensionNone: {
RTC_LOG(LS_WARNING) << "Unidentified extension in the packet.";
break;
}
case RTPExtensionType::kRtpExtensionVideoTiming: {
// Nullify 3 last entries: packetization delay and 2 network timestamps.
// Each of them is 2 bytes.
memset(buffer.data() + extension.offset +
VideoSendTiming::kPacerExitDeltaOffset,
0, 6);
break;
}
case RTPExtensionType::kRtpExtensionTransportSequenceNumber:
case RTPExtensionType::kRtpExtensionTransportSequenceNumber02:
case RTPExtensionType::kRtpExtensionTransmissionTimeOffset:
case RTPExtensionType::kRtpExtensionAbsoluteSendTime: {
// Nullify whole extension, as it's filled in the pacer.
memset(buffer.data() + extension.offset, 0, extension.length);
break;
}
case RTPExtensionType::kRtpExtensionAudioLevel:
case RTPExtensionType::kRtpExtensionColorSpace:
case RTPExtensionType::kRtpExtensionFrameMarking:
case RTPExtensionType::kRtpExtensionGenericFrameDescriptor00:
case RTPExtensionType::kRtpExtensionGenericFrameDescriptor01:
case RTPExtensionType::kRtpExtensionMid:
case RTPExtensionType::kRtpExtensionNumberOfExtensions:
case RTPExtensionType::kRtpExtensionPlayoutDelay:
case RTPExtensionType::kRtpExtensionRepairedRtpStreamId:
case RTPExtensionType::kRtpExtensionRtpStreamId:
case RTPExtensionType::kRtpExtensionVideoContentType:
case RTPExtensionType::kRtpExtensionVideoRotation: {
// Non-mutable extension. Don't change it.
break;
}
}
}
}
void RtpPacket::SetCsrcs(rtc::ArrayView<const uint32_t> csrcs) {
RTC_DCHECK_EQ(extensions_size_, 0);
RTC_DCHECK_EQ(payload_size_, 0);

4
modules/rtp_rtcp/source/rtp_packet.h
View File

@ -85,6 +85,10 @@ class RtpPacket {
void SetTimestamp(uint32_t timestamp);
void SetSsrc(uint32_t ssrc);
// Copies the buffer with zero-ed mutable extensions,
// which are modified after FEC protection is generated.
void CopyAndZeroMutableExtensions(rtc::ArrayView<uint8_t> buffer) const;
// Writes csrc list. Assumes:
// a) There is enough room left in buffer.
// b) Extension headers, payload or padding data has not already been added.

108
modules/rtp_rtcp/source/rtp_sender_unittest.cc
View File

@ -1324,114 +1324,6 @@ TEST_P(RtpSenderTest, SendFlexfecPackets) {
EXPECT_EQ(kFlexfecSsrc, flexfec_packet.Ssrc());
}
// TODO(ilnik): because of webrtc:7859. Once FEC moved below pacer, this test
// should be removed.
TEST_P(RtpSenderTest, NoFlexfecForTimingFrames) {
constexpr uint32_t kTimestamp = 1234;
const int64_t kCaptureTimeMs = fake_clock_.TimeInMilliseconds();
constexpr int kMediaPayloadType = 127;
constexpr int kFlexfecPayloadType = 118;
constexpr uint32_t kMediaSsrc = 1234;
constexpr uint32_t kFlexfecSsrc = 5678;
const std::vector<RtpExtension> kNoRtpExtensions;
const std::vector<RtpExtensionSize> kNoRtpExtensionSizes;
FlexfecSender flexfec_sender(kFlexfecPayloadType, kFlexfecSsrc, kMediaSsrc,
kNoMid, kNoRtpExtensions, kNoRtpExtensionSizes,
nullptr /* rtp_state */, &fake_clock_);
// Reset |rtp_sender_| to use FlexFEC.
rtp_sender_.reset(new RTPSender(
false, &fake_clock_, &transport_, &mock_paced_sender_,
flexfec_sender.ssrc(), &seq_num_allocator_, nullptr, nullptr, nullptr,
&mock_rtc_event_log_, &send_packet_observer_,
&retransmission_rate_limiter_, nullptr, false, nullptr, false, false,
FieldTrialBasedConfig()));
rtp_sender_->SetSSRC(kMediaSsrc);
rtp_sender_->SetSequenceNumber(kSeqNum);
rtp_sender_->SetStorePacketsStatus(true, 10);
PlayoutDelayOracle playout_delay_oracle;
RTPSenderVideo rtp_sender_video(
&fake_clock_, rtp_sender_.get(), &flexfec_sender, &playout_delay_oracle,
nullptr, false, false, FieldTrialBasedConfig());
rtp_sender_video.RegisterPayloadType(kMediaPayloadType, "GENERIC",
/*raw_payload=*/false);
// Need extension to be registered for timing frames to be sent.
ASSERT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension(
kRtpExtensionVideoTiming, kVideoTimingExtensionId));
// Parameters selected to generate a single FEC packet per media packet.
FecProtectionParams params;
params.fec_rate = 15;
params.max_fec_frames = 1;
params.fec_mask_type = kFecMaskRandom;
rtp_sender_video.SetFecParameters(params, params);
EXPECT_CALL(mock_paced_sender_,
InsertPacket(RtpPacketSender::kLowPriority, kMediaSsrc, kSeqNum,
_, _, false));
EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kLowPriority,
kFlexfecSsrc, _, _, _, false))
.Times(0); // Not called because packet should not be protected.
RTPVideoHeader video_header;
video_header.video_timing.flags = VideoSendTiming::kTriggeredByTimer;
EXPECT_TRUE(rtp_sender_video.SendVideo(
VideoFrameType::kVideoFrameKey, kMediaPayloadType, kTimestamp,
kCaptureTimeMs, kPayloadData, sizeof(kPayloadData), nullptr,
&video_header, kDefaultExpectedRetransmissionTimeMs));
EXPECT_CALL(mock_rtc_event_log_,
LogProxy(SameRtcEventTypeAs(RtcEvent::Type::RtpPacketOutgoing)))
.Times(1);
EXPECT_EQ(RtpPacketSendResult::kSuccess,
rtp_sender_->TimeToSendPacket(kMediaSsrc, kSeqNum,
fake_clock_.TimeInMilliseconds(),
false, PacedPacketInfo()));
ASSERT_EQ(1, transport_.packets_sent());
const RtpPacketReceived& media_packet = transport_.sent_packets_[0];
EXPECT_EQ(kMediaPayloadType, media_packet.PayloadType());
EXPECT_EQ(kSeqNum, media_packet.SequenceNumber());
EXPECT_EQ(kMediaSsrc, media_packet.Ssrc());
// Now try to send not a timing frame.
uint16_t flexfec_seq_num;
EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kLowPriority,
kFlexfecSsrc, _, _, _, false))
.WillOnce(SaveArg<2>(&flexfec_seq_num));
EXPECT_CALL(mock_paced_sender_,
InsertPacket(RtpPacketSender::kLowPriority, kMediaSsrc,
kSeqNum + 1, _, _, false));
video_header.video_timing.flags = VideoSendTiming::kInvalid;
EXPECT_TRUE(rtp_sender_video.SendVideo(
VideoFrameType::kVideoFrameKey, kMediaPayloadType, kTimestamp + 1,
kCaptureTimeMs + 1, kPayloadData, sizeof(kPayloadData), nullptr,
&video_header, kDefaultExpectedRetransmissionTimeMs));
EXPECT_CALL(mock_rtc_event_log_,
LogProxy(SameRtcEventTypeAs(RtcEvent::Type::RtpPacketOutgoing)))
.Times(2);
EXPECT_EQ(RtpPacketSendResult::kSuccess,
rtp_sender_->TimeToSendPacket(kMediaSsrc, kSeqNum + 1,
fake_clock_.TimeInMilliseconds(),
false, PacedPacketInfo()));
EXPECT_EQ(RtpPacketSendResult::kSuccess,
rtp_sender_->TimeToSendPacket(kFlexfecSsrc, flexfec_seq_num,
fake_clock_.TimeInMilliseconds(),
false, PacedPacketInfo()));
ASSERT_EQ(3, transport_.packets_sent());
const RtpPacketReceived& media_packet2 = transport_.sent_packets_[1];
EXPECT_EQ(kMediaPayloadType, media_packet2.PayloadType());
EXPECT_EQ(kSeqNum + 1, media_packet2.SequenceNumber());
EXPECT_EQ(kMediaSsrc, media_packet2.Ssrc());
const RtpPacketReceived& flexfec_packet = transport_.sent_packets_[2];
EXPECT_EQ(kFlexfecPayloadType, flexfec_packet.PayloadType());
EXPECT_EQ(flexfec_seq_num, flexfec_packet.SequenceNumber());
EXPECT_EQ(kFlexfecSsrc, flexfec_packet.Ssrc());
}
TEST_P(RtpSenderTestWithoutPacer, SendFlexfecPackets) {
constexpr uint32_t kTimestamp = 1234;
constexpr int kMediaPayloadType = 127;

12
modules/rtp_rtcp/source/rtp_sender_video.cc
View File

@ -692,12 +692,12 @@ bool RTPSenderVideo::SendVideo(
// Put packetization finish timestamp into extension.
if (packet->HasExtension<VideoTimingExtension>()) {
packet->set_packetization_finish_time_ms(clock_->TimeInMilliseconds());
// TODO(ilnik): Due to webrtc:7859, packets with timing extensions are not
// protected by FEC. It reduces FEC efficiency a bit. When FEC is moved
// below the pacer, it can be re-enabled for these packets.
// NOTE: Any RTP stream processor in the network, modifying 'network'
// timestamps in the timing frames extension have to be an end-point for
// FEC, otherwise recovered by FEC packets will be corrupted.
// TODO(webrtc:10750): wait a couple of months and remove the statement
// below. For now we can't use packets with VideoTimingFrame extensions in
// Fec because the extension is modified after FEC is calculated by pacer
// and network. This may cause corruptions in video payload and header.
// The fix in receive code is implemented, but until all the receivers
// are updated, senders can't send potentially breaking packets.
protect_packet = false;
}

24
modules/rtp_rtcp/source/ulpfec_receiver_impl.cc
View File

@ -14,21 +14,28 @@
#include <memory>
#include <utility>
#include "absl/memory/memory.h"
#include "api/scoped_refptr.h"
#include "modules/rtp_rtcp/source/byte_io.h"
#include "modules/rtp_rtcp/source/rtp_packet_received.h"
#include "rtc_base/logging.h"
#include "rtc_base/time_utils.h"
namespace webrtc {
UlpfecReceiver* UlpfecReceiver::Create(uint32_t ssrc,
RecoveredPacketReceiver* callback) {
return new UlpfecReceiverImpl(ssrc, callback);
std::unique_ptr<UlpfecReceiver> UlpfecReceiver::Create(
uint32_t ssrc,
RecoveredPacketReceiver* callback,
rtc::ArrayView<const RtpExtension> extensions) {
return absl::make_unique<UlpfecReceiverImpl>(ssrc, callback, extensions);
}
UlpfecReceiverImpl::UlpfecReceiverImpl(uint32_t ssrc,
RecoveredPacketReceiver* callback)
UlpfecReceiverImpl::UlpfecReceiverImpl(
uint32_t ssrc,
RecoveredPacketReceiver* callback,
rtc::ArrayView<const RtpExtension> extensions)
: ssrc_(ssrc),
extensions_(extensions),
recovered_packet_callback_(callback),
fec_(ForwardErrorCorrection::CreateUlpfec(ssrc_)) {}
@ -243,6 +250,13 @@ int32_t UlpfecReceiverImpl::ProcessReceivedFec() {
recovered_packet_callback_->OnRecoveredPacket(packet->data,
packet->length);
crit_sect_.Enter();
RtpPacketReceived rtp_packet;
// TODO(ilnik): move extension nullifying out of RtpPacket, so there's no
// need to create one here, and avoid two memcpy calls below.
rtp_packet.Parse(packet->data, packet->length); // Does memcopy.
rtp_packet.IdentifyExtensions(extensions_);
rtp_packet.CopyAndZeroMutableExtensions( // Does memcopy.
rtc::MakeArrayView(packet->data, packet->length));
}
fec_->DecodeFec(*received_packet, &recovered_packets_);
}

6
modules/rtp_rtcp/source/ulpfec_receiver_impl.h
View File

@ -17,6 +17,7 @@
#include <vector>
#include "api/rtp_headers.h"
#include "modules/rtp_rtcp/include/rtp_header_extension_map.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "modules/rtp_rtcp/include/ulpfec_receiver.h"
#include "modules/rtp_rtcp/source/forward_error_correction.h"
@ -26,7 +27,9 @@ namespace webrtc {
class UlpfecReceiverImpl : public UlpfecReceiver {
public:
explicit UlpfecReceiverImpl(uint32_t ssrc, RecoveredPacketReceiver* callback);
explicit UlpfecReceiverImpl(uint32_t ssrc,
RecoveredPacketReceiver* callback,
rtc::ArrayView<const RtpExtension> extensions);
~UlpfecReceiverImpl() override;
int32_t AddReceivedRedPacket(const RTPHeader& rtp_header,
@ -40,6 +43,7 @@ class UlpfecReceiverImpl : public UlpfecReceiver {
private:
const uint32_t ssrc_;
const RtpHeaderExtensionMap extensions_;
rtc::CriticalSection crit_sect_;
RecoveredPacketReceiver* recovered_packet_callback_;

7
modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc
View File

@ -49,8 +49,9 @@ class UlpfecReceiverTest : public ::testing::Test {
protected:
UlpfecReceiverTest()
: fec_(ForwardErrorCorrection::CreateUlpfec(kMediaSsrc)),
receiver_fec_(
UlpfecReceiver::Create(kMediaSsrc, &recovered_packet_receiver_)),
receiver_fec_(UlpfecReceiver::Create(kMediaSsrc,
&recovered_packet_receiver_,
{})),
packet_generator_(kMediaSsrc) {}
// Generates |num_fec_packets| FEC packets, given |media_packets|.
@ -180,7 +181,7 @@ void UlpfecReceiverTest::SurvivesMaliciousPacket(const uint8_t* data,
NullRecoveredPacketReceiver null_callback;
std::unique_ptr<UlpfecReceiver> receiver_fec(
UlpfecReceiver::Create(kMediaSsrc, &null_callback));
UlpfecReceiver::Create(kMediaSsrc, &null_callback, {}));
receiver_fec->AddReceivedRedPacket(header, data, length, ulpfec_payload_type);
}

2
test/fuzzers/ulpfec_receiver_fuzzer.cc
View File

@ -36,7 +36,7 @@ void FuzzOneInput(const uint8_t* data, size_t size) {
DummyCallback callback;
std::unique_ptr<UlpfecReceiver> receiver(
UlpfecReceiver::Create(ulpfec_ssrc, &callback));
UlpfecReceiver::Create(ulpfec_ssrc, &callback, {}));
std::unique_ptr<uint8_t[]> packet;
size_t packet_length;

4
video/rtp_video_stream_receiver.cc
View File

@ -179,7 +179,9 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver(
ntp_estimator_(clock),
rtp_header_extensions_(config_.rtp.extensions),
rtp_receive_statistics_(rtp_receive_statistics),
ulpfec_receiver_(UlpfecReceiver::Create(config->rtp.remote_ssrc, this)),
ulpfec_receiver_(UlpfecReceiver::Create(config->rtp.remote_ssrc,
this,
config->rtp.extensions)),
receiving_(false),
last_packet_log_ms_(-1),
rtp_rtcp_(CreateRtpRtcpModule(clock,

5
video/video_send_stream_tests.cc
View File

@ -591,6 +591,7 @@ class UlpfecObserver : public test::EndToEndTest {
send_config->rtp.extensions.push_back(
RtpExtension(RtpExtension::kAbsSendTimeUri, kAbsSendTimeExtensionId));
}
(*receive_configs)[0].rtp.extensions = send_config->rtp.extensions;
encoder_config->codec_type = PayloadStringToCodecType(payload_name_);
(*receive_configs)[0].rtp.red_payload_type =
send_config->rtp.ulpfec.red_payload_type;
@ -787,6 +788,7 @@ class FlexfecObserver : public test::EndToEndTest {
} else {
send_config->rtp.extensions.clear();
}
(*receive_configs)[0].rtp.extensions = send_config->rtp.extensions;
encoder_config->codec_type = PayloadStringToCodecType(payload_name_);
}
@ -2033,8 +2035,7 @@ TEST_F(VideoSendStreamTest, CanReconfigureToUseStartBitrateAbovePreviousMax) {
class StartBitrateObserver : public test::FakeEncoder {
public:
StartBitrateObserver()
: FakeEncoder(Clock::GetRealTimeClock()),
start_bitrate_kbps_(0) {}
: FakeEncoder(Clock::GetRealTimeClock()), start_bitrate_kbps_(0) {}
int32_t InitEncode(const VideoCodec* config,
const Settings& settings) override {
rtc::CritScope lock(&crit_);