Replace RtpStreamReceiver::DeliverRtp with OnRtpPacket.
This avoids redoing RTP header parsing already done in Call, for video.
The next step is to convert other types of receive streams, i.e.,
audio and flexfec, to use a compatible OnRtpPacket method. We can then
introduce a shared base interface, and simplify media-independent
receive processing in Call.
BUG=webrtc:7135
Review-Url: https://codereview.webrtc.org/2681673004
Cr-Original-Commit-Position: refs/heads/master@{#16583}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: 38cc1d6b31eff566651bc1d09b8a4e9093bcb90d
diff --git a/call/BUILD.gn b/call/BUILD.gn
index 382ae2e..680ec6b 100644
--- a/call/BUILD.gn
+++ b/call/BUILD.gn
@@ -61,7 +61,6 @@
"bitrate_estimator_tests.cc",
"call_unittest.cc",
"flexfec_receive_stream_unittest.cc",
- "packet_injection_tests.cc",
]
deps = [
":call",
diff --git a/call/call.cc b/call/call.cc
index e21b076..ec80537 100644
--- a/call/call.cc
+++ b/call/call.cc
@@ -1211,35 +1211,26 @@
if (it != video_receive_ssrcs_.end()) {
received_bytes_per_second_counter_.Add(static_cast<int>(length));
received_video_bytes_per_second_counter_.Add(static_cast<int>(length));
- // TODO(brandtr): Notify the BWE of received media packets here.
- auto status = it->second->DeliverRtp(packet, length, packet_time)
- ? DELIVERY_OK
- : DELIVERY_PACKET_ERROR;
- // Deliver media packets to FlexFEC subsystem. RTP header extensions need
- // not be parsed, as FlexFEC is oblivious to the semantic meaning of the
- // packet contents beyond the 12 byte RTP base header. The BWE is fed
- // information about these media packets from the regular media pipeline.
- if (parsed_packet) {
- auto it_bounds = flexfec_receive_ssrcs_media_.equal_range(ssrc);
- for (auto it = it_bounds.first; it != it_bounds.second; ++it)
- it->second->AddAndProcessReceivedPacket(*parsed_packet);
- }
- if (status == DELIVERY_OK)
- event_log_->LogRtpHeader(kIncomingPacket, media_type, packet, length);
- return status;
+ it->second->OnRtpPacket(*parsed_packet);
+
+ // Deliver media packets to FlexFEC subsystem.
+ auto it_bounds = flexfec_receive_ssrcs_media_.equal_range(ssrc);
+ for (auto it = it_bounds.first; it != it_bounds.second; ++it)
+ it->second->AddAndProcessReceivedPacket(*parsed_packet);
+
+ event_log_->LogRtpHeader(kIncomingPacket, media_type, packet, length);
+ return DELIVERY_OK;
}
}
if (media_type == MediaType::ANY || media_type == MediaType::VIDEO) {
auto it = flexfec_receive_ssrcs_protection_.find(ssrc);
if (it != flexfec_receive_ssrcs_protection_.end()) {
- if (parsed_packet) {
- auto status = it->second->AddAndProcessReceivedPacket(*parsed_packet)
- ? DELIVERY_OK
- : DELIVERY_PACKET_ERROR;
- if (status == DELIVERY_OK)
- event_log_->LogRtpHeader(kIncomingPacket, media_type, packet, length);
- return status;
- }
+ auto status = it->second->AddAndProcessReceivedPacket(*parsed_packet)
+ ? DELIVERY_OK
+ : DELIVERY_PACKET_ERROR;
+ if (status == DELIVERY_OK)
+ event_log_->LogRtpHeader(kIncomingPacket, media_type, packet, length);
+ return status;
}
}
return DELIVERY_UNKNOWN_SSRC;
diff --git a/call/packet_injection_tests.cc b/call/packet_injection_tests.cc
deleted file mode 100644
index 10de8f6..0000000
--- a/call/packet_injection_tests.cc
+++ /dev/null
@@ -1,93 +0,0 @@
-/*
- * Copyright (c) 2015 The WebRTC project authors. All Rights Reserved.
- *
- * Use of this source code is governed by a BSD-style license
- * that can be found in the LICENSE file in the root of the source
- * tree. An additional intellectual property rights grant can be found
- * in the file PATENTS. All contributing project authors may
- * be found in the AUTHORS file in the root of the source tree.
- */
-
-#include <memory>
-
-#include "webrtc/logging/rtc_event_log/rtc_event_log.h"
-#include "webrtc/test/call_test.h"
-#include "webrtc/test/gtest.h"
-#include "webrtc/test/null_transport.h"
-
-namespace webrtc {
-
-class PacketInjectionTest : public test::CallTest {
- protected:
- enum class CodecType {
- kVp8,
- kH264,
- };
-
- PacketInjectionTest() : rtp_header_parser_(RtpHeaderParser::Create()) {}
-
- void InjectIncorrectPacket(CodecType codec_type,
- uint8_t packet_type,
- const uint8_t* packet,
- size_t length);
-
- std::unique_ptr<RtpHeaderParser> rtp_header_parser_;
-};
-
-void PacketInjectionTest::InjectIncorrectPacket(CodecType codec_type,
- uint8_t payload_type,
- const uint8_t* packet,
- size_t length) {
- CreateSenderCall(Call::Config(&event_log_));
- CreateReceiverCall(Call::Config(&event_log_));
-
- test::NullTransport null_transport;
- CreateSendConfig(1, 0, 0, &null_transport);
- CreateMatchingReceiveConfigs(&null_transport);
- video_receive_configs_[0].decoders[0].payload_type = payload_type;
- switch (codec_type) {
- case CodecType::kVp8:
- video_receive_configs_[0].decoders[0].payload_name = "VP8";
- break;
- case CodecType::kH264:
- video_receive_configs_[0].decoders[0].payload_name = "H264";
- break;
- }
- CreateVideoStreams();
-
- RTPHeader header;
- EXPECT_TRUE(rtp_header_parser_->Parse(packet, length, &header));
- EXPECT_EQ(kVideoSendSsrcs[0], header.ssrc)
- << "Packet should have configured SSRC to not be dropped early.";
- EXPECT_EQ(payload_type, header.payloadType);
- Start();
- EXPECT_EQ(PacketReceiver::DELIVERY_PACKET_ERROR,
- receiver_call_->Receiver()->DeliverPacket(MediaType::VIDEO, packet,
- length, PacketTime()));
- Stop();
-
- DestroyStreams();
-}
-
-TEST_F(PacketInjectionTest, StapAPacketWithTruncatedNalUnits) {
- const uint8_t kPacket[] = {0x80,
- 0xE5,
- 0xE6,
- 0x0,
- 0x0,
- 0xED,
- 0x23,
- 0x4,
- 0x00,
- 0xC0,
- 0xFF,
- 0xED,
- 0x58,
- 0xCB,
- 0xED,
- 0xDF};
-
- InjectIncorrectPacket(CodecType::kH264, 101, kPacket, sizeof(kPacket));
-}
-
-} // namespace webrtc
diff --git a/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc b/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc
index f22f108..22f050b 100644
--- a/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc
@@ -867,6 +867,12 @@
EXPECT_FALSE(depacketizer_->Parse(&payload, kPayload, sizeof(kPayload)));
}
+TEST_F(RtpDepacketizerH264Test, TestStapAPacketWithTruncatedNalUnits) {
+ const uint8_t kPayload[] = { 0x58, 0xCB, 0xED, 0xDF};
+ RtpDepacketizer::ParsedPayload payload;
+ EXPECT_FALSE(depacketizer_->Parse(&payload, kPayload, sizeof(kPayload)));
+}
+
TEST_F(RtpDepacketizerH264Test, TestTruncationJustAfterSingleStapANalu) {
const uint8_t kPayload[] = {0x38, 0x27, 0x27};
RtpDepacketizer::ParsedPayload payload;
diff --git a/video/rtp_stream_receiver.cc b/video/rtp_stream_receiver.cc
index d49d095..cb1ffc9 100644
--- a/video/rtp_stream_receiver.cc
+++ b/video/rtp_stream_receiver.cc
@@ -26,6 +26,8 @@
#include "webrtc/modules/rtp_rtcp/include/rtp_receiver.h"
#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h"
#include "webrtc/modules/rtp_rtcp/include/ulpfec_receiver.h"
+#include "webrtc/modules/rtp_rtcp/source/rtp_header_extensions.h"
+#include "webrtc/modules/rtp_rtcp/source/rtp_packet_received.h"
#include "webrtc/modules/video_coding/frame_object.h"
#include "webrtc/modules/video_coding/h264_sprop_parameter_sets.h"
#include "webrtc/modules/video_coding/h264_sps_pps_tracker.h"
@@ -316,57 +318,54 @@
rtp_rtcp_->SetRemoteSSRC(ssrc);
}
-bool RtpStreamReceiver::DeliverRtp(const uint8_t* rtp_packet,
- size_t rtp_packet_length,
- const PacketTime& packet_time) {
+void RtpStreamReceiver::OnRtpPacket(const RtpPacketReceived& packet) {
{
rtc::CritScope lock(&receive_cs_);
if (!receiving_) {
- return false;
+ return;
}
}
- RTPHeader header;
- if (!rtp_header_parser_->Parse(rtp_packet, rtp_packet_length,
- &header)) {
- return false;
- }
- int64_t arrival_time_ms;
int64_t now_ms = clock_->TimeInMilliseconds();
- if (packet_time.timestamp != -1)
- arrival_time_ms = (packet_time.timestamp + 500) / 1000;
- else
- arrival_time_ms = now_ms;
{
// Periodically log the RTP header of incoming packets.
rtc::CritScope lock(&receive_cs_);
if (now_ms - last_packet_log_ms_ > kPacketLogIntervalMs) {
std::stringstream ss;
- ss << "Packet received on SSRC: " << header.ssrc << " with payload type: "
- << static_cast<int>(header.payloadType) << ", timestamp: "
- << header.timestamp << ", sequence number: " << header.sequenceNumber
- << ", arrival time: " << arrival_time_ms;
- if (header.extension.hasTransmissionTimeOffset)
- ss << ", toffset: " << header.extension.transmissionTimeOffset;
- if (header.extension.hasAbsoluteSendTime)
- ss << ", abs send time: " << header.extension.absoluteSendTime;
+ ss << "Packet received on SSRC: " << packet.Ssrc()
+ << " with payload type: " << static_cast<int>(packet.PayloadType())
+ << ", timestamp: " << packet.Timestamp()
+ << ", sequence number: " << packet.SequenceNumber()
+ << ", arrival time: " << packet.arrival_time_ms();
+ int32_t time_offset;
+ if (packet.GetExtension<TransmissionOffset>(&time_offset)) {
+ ss << ", toffset: " << time_offset;
+ }
+ uint32_t send_time;
+ if (packet.GetExtension<AbsoluteSendTime>(&send_time)) {
+ ss << ", abs send time: " << send_time;
+ }
LOG(LS_INFO) << ss.str();
last_packet_log_ms_ = now_ms;
}
}
+ // TODO(nisse): Delete use of GetHeader, but needs refactoring of
+ // ReceivePacket and IncomingPacket methods below.
+ RTPHeader header;
+ packet.GetHeader(&header);
+
header.payload_type_frequency = kVideoPayloadTypeFrequency;
bool in_order = IsPacketInOrder(header);
rtp_payload_registry_.SetIncomingPayloadType(header);
- bool ret = ReceivePacket(rtp_packet, rtp_packet_length, header, in_order);
+ ReceivePacket(packet.data(), packet.size(), header, in_order);
// Update receive statistics after ReceivePacket.
// Receive statistics will be reset if the payload type changes (make sure
// that the first packet is included in the stats).
rtp_receive_statistics_->IncomingPacket(
- header, rtp_packet_length, IsPacketRetransmitted(header, in_order));
- return ret;
+ header, packet.size(), IsPacketRetransmitted(header, in_order));
}
int32_t RtpStreamReceiver::RequestKeyFrame() {
@@ -424,6 +423,7 @@
nack_module_->UpdateRtt(max_rtt_ms);
}
+// TODO(nisse): Drop return value.
bool RtpStreamReceiver::ReceivePacket(const uint8_t* packet,
size_t packet_length,
const RTPHeader& header,
diff --git a/video/rtp_stream_receiver.h b/video/rtp_stream_receiver.h
index dd21789..c6531cc 100644
--- a/video/rtp_stream_receiver.h
+++ b/video/rtp_stream_receiver.h
@@ -44,6 +44,7 @@
class RemoteNtpTimeEstimator;
class RtcpRttStats;
class RtpHeaderParser;
+class RtpPacketReceived;
class RTPPayloadRegistry;
class RtpReceiver;
class Transport;
@@ -89,9 +90,6 @@
void StartReceive();
void StopReceive();
- bool DeliverRtp(const uint8_t* rtp_packet,
- size_t rtp_packet_length,
- const PacketTime& packet_time);
bool DeliverRtcp(const uint8_t* rtcp_packet, size_t rtcp_packet_length);
void FrameContinuous(uint16_t seq_num);
@@ -100,6 +98,9 @@
void SignalNetworkState(NetworkState state);
+ // TODO(nisse): Intended to be part of an RtpPacketReceiver interface.
+ void OnRtpPacket(const RtpPacketReceived& packet);
+
// Implements RtpData.
int32_t OnReceivedPayloadData(const uint8_t* payload_data,
size_t payload_size,
diff --git a/video/video_receive_stream.cc b/video/video_receive_stream.cc
index 50c47e5..7d8cfef 100644
--- a/video/video_receive_stream.cc
+++ b/video/video_receive_stream.cc
@@ -256,10 +256,8 @@
return rtp_stream_receiver_.DeliverRtcp(packet, length);
}
-bool VideoReceiveStream::DeliverRtp(const uint8_t* packet,
- size_t length,
- const PacketTime& packet_time) {
- return rtp_stream_receiver_.DeliverRtp(packet, length, packet_time);
+void VideoReceiveStream::OnRtpPacket(const RtpPacketReceived& packet) {
+ rtp_stream_receiver_.OnRtpPacket(packet);
}
bool VideoReceiveStream::OnRecoveredPacket(const uint8_t* packet,
diff --git a/video/video_receive_stream.h b/video/video_receive_stream.h
index a6ee31e..8c13ccd 100644
--- a/video/video_receive_stream.h
+++ b/video/video_receive_stream.h
@@ -62,9 +62,6 @@
void SignalNetworkState(NetworkState state);
bool DeliverRtcp(const uint8_t* packet, size_t length);
- bool DeliverRtp(const uint8_t* packet,
- size_t length,
- const PacketTime& packet_time);
bool OnRecoveredPacket(const uint8_t* packet, size_t length);
@@ -84,6 +81,9 @@
void EnableEncodedFrameRecording(rtc::PlatformFile file,
size_t byte_limit) override;
+ // TODO(nisse): Intended to be part of an RtpPacketReceiver interface.
+ void OnRtpPacket(const RtpPacketReceived& packet);
+
// Implements rtc::VideoSinkInterface<VideoFrame>.
void OnFrame(const VideoFrame& video_frame) override;
