Delete RtpUtility::Payload, and refactor RTPSender to not use it

Replaced by a payload type --> video codec map in RTPSenderVideo,
where it is used to select the right packetizer.

Bug: webrtc:6883
Change-Id: I43a635d5135c5d519df860a2f4287a4478870b0f
Reviewed-on: https://webrtc-review.googlesource.com/c/119263
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26380}
This commit is contained in:
Niels Möller
2019-01-24 09:44:14 +01:00
committed by Commit Bot
parent 2820d174b5
commit 171df93262
10 changed files with 57 additions and 214 deletions

View File

@ -53,15 +53,6 @@ void StreamId::Set(const char* data, size_t size) {
// and thus assume trivial destructibility.
static_assert(std::is_trivially_destructible<StreamId>::value, "");
PayloadUnion::PayloadUnion(const AudioPayload& payload) : payload_(payload) {}
PayloadUnion::PayloadUnion(const VideoPayload& payload) : payload_(payload) {}
PayloadUnion::PayloadUnion(const PayloadUnion&) = default;
PayloadUnion::PayloadUnion(PayloadUnion&&) = default;
PayloadUnion::~PayloadUnion() = default;
PayloadUnion& PayloadUnion::operator=(const PayloadUnion&) = default;
PayloadUnion& PayloadUnion::operator=(PayloadUnion&&) = default;
PacketFeedback::PacketFeedback(int64_t arrival_time_ms,
uint16_t sequence_number)
: PacketFeedback(-1,

View File

@ -41,47 +41,6 @@ const int kBogusRtpRateForAudioRtcp = 8000;
// Minimum RTP header size in bytes.
const uint8_t kRtpHeaderSize = 12;
struct AudioPayload {
SdpAudioFormat format;
uint32_t rate;
};
struct VideoPayload {
VideoCodecType videoCodecType;
// The H264 profile only matters if videoCodecType == kVideoCodecH264.
H264::Profile h264_profile;
};
class PayloadUnion {
public:
explicit PayloadUnion(const AudioPayload& payload);
explicit PayloadUnion(const VideoPayload& payload);
PayloadUnion(const PayloadUnion&);
PayloadUnion(PayloadUnion&&);
~PayloadUnion();
PayloadUnion& operator=(const PayloadUnion&);
PayloadUnion& operator=(PayloadUnion&&);
bool is_audio() const {
return absl::holds_alternative<AudioPayload>(payload_);
}
bool is_video() const {
return absl::holds_alternative<VideoPayload>(payload_);
}
const AudioPayload& audio_payload() const {
return absl::get<AudioPayload>(payload_);
}
const VideoPayload& video_payload() const {
return absl::get<VideoPayload>(payload_);
}
AudioPayload& audio_payload() { return absl::get<AudioPayload>(payload_); }
VideoPayload& video_payload() { return absl::get<VideoPayload>(payload_); }
private:
absl::variant<AudioPayload, VideoPayload> payload_;
};
enum ProtectionType { kUnprotectedPacket, kProtectedPacket };
enum StorageType { kDontRetransmit, kAllowRetransmission };

View File

@ -144,7 +144,6 @@ RTPSender::RTPSender(
force_part_of_allocation_(false),
max_packet_size_(IP_PACKET_SIZE - 28), // Default is IP-v4/UDP.
last_payload_type_(-1),
payload_type_map_(),
rtp_header_extension_map_(extmap_allow_mixed),
packet_history_(clock),
flexfec_packet_history_(clock),
@ -202,12 +201,6 @@ RTPSender::~RTPSender() {
// variables but we grab them in all other methods. (what's the design?)
// Start documenting what thread we're on in what method so that it's easier
// to understand performance attributes and possibly remove locks.
while (!payload_type_map_.empty()) {
std::map<int8_t, RtpUtility::Payload*>::iterator it =
payload_type_map_.begin();
delete it->second;
payload_type_map_.erase(it);
}
}
rtc::ArrayView<const RtpExtensionSize> RTPSender::FecExtensionSizes() {
@ -284,58 +277,19 @@ int32_t RTPSender::RegisterPayload(absl::string_view payload_name,
RTC_DCHECK_LT(payload_name.size(), RTP_PAYLOAD_NAME_SIZE);
rtc::CritScope lock(&send_critsect_);
std::map<int8_t, RtpUtility::Payload*>::iterator it =
payload_type_map_.find(payload_number);
if (payload_type_map_.end() != it) {
// We already use this payload type.
RtpUtility::Payload* payload = it->second;
RTC_DCHECK(payload);
// Check if it's the same as we already have.
if (absl::EqualsIgnoreCase(payload->name, payload_name)) {
if (audio_configured_ && payload->typeSpecific.is_audio()) {
auto& p = payload->typeSpecific.audio_payload();
if (rtc::SafeEq(p.format.clockrate_hz, frequency) &&
(p.rate == rate || p.rate == 0 || rate == 0)) {
p.rate = rate;
// Ensure that we update the rate if new or old is zero.
return 0;
}
}
if (!audio_configured_ && !payload->typeSpecific.is_audio()) {
return 0;
}
}
return -1;
}
int32_t ret_val = 0;
RtpUtility::Payload* payload = nullptr;
if (audio_configured_) {
// TODO(mflodman): Change to CreateAudioPayload and make static.
ret_val = audio_->RegisterAudioPayload(payload_name, payload_number,
frequency, channels, rate, &payload);
frequency, channels, rate);
} else {
payload = video_->CreateVideoPayload(payload_name, payload_number);
}
if (payload) {
payload_type_map_[payload_number] = payload;
video_->RegisterPayloadType(payload_number, payload_name);
}
return ret_val;
}
int32_t RTPSender::DeRegisterSendPayload(int8_t payload_type) {
rtc::CritScope lock(&send_critsect_);
std::map<int8_t, RtpUtility::Payload*>::iterator it =
payload_type_map_.find(payload_type);
if (payload_type_map_.end() == it) {
return -1;
}
RtpUtility::Payload* payload = it->second;
delete payload;
payload_type_map_.erase(it);
int32_t RTPSender::DeRegisterSendPayload(int8_t /* payload_type */) {
return 0;
}
@ -384,37 +338,6 @@ void RTPSender::SetRtxPayloadType(int payload_type,
rtx_payload_type_map_[associated_payload_type] = payload_type;
}
int32_t RTPSender::CheckPayloadType(int8_t payload_type,
VideoCodecType* video_type) {
rtc::CritScope lock(&send_critsect_);
if (payload_type < 0) {
RTC_LOG(LS_ERROR) << "Invalid payload_type " << payload_type << ".";
return -1;
}
if (last_payload_type_ == payload_type) {
if (!audio_configured_) {
*video_type = video_->VideoCodecType();
}
return 0;
}
std::map<int8_t, RtpUtility::Payload*>::iterator it =
payload_type_map_.find(payload_type);
if (it == payload_type_map_.end()) {
RTC_LOG(LS_WARNING) << "Payload type " << static_cast<int>(payload_type)
<< " not registered.";
return -1;
}
RtpUtility::Payload* payload = it->second;
RTC_DCHECK(payload);
if (payload->typeSpecific.is_video() && !audio_configured_) {
video_->SetVideoCodecType(
payload->typeSpecific.video_payload().videoCodecType);
*video_type = payload->typeSpecific.video_payload().videoCodecType;
}
return 0;
}
bool RTPSender::SendOutgoingData(FrameType frame_type,
int8_t payload_type,
uint32_t capture_timestamp,
@ -441,13 +364,6 @@ bool RTPSender::SendOutgoingData(FrameType frame_type,
if (!sending_media_)
return true;
}
VideoCodecType video_type = kVideoCodecGeneric;
if (CheckPayloadType(payload_type, &video_type) != 0) {
RTC_LOG(LS_ERROR) << "Don't send data with unknown payload type: "
<< static_cast<int>(payload_type) << ".";
return false;
}
switch (frame_type) {
case kAudioFrameSpeech:
case kAudioFrameCN:
@ -481,9 +397,9 @@ bool RTPSender::SendOutgoingData(FrameType frame_type,
sequence_number);
}
result = video_->SendVideo(video_type, frame_type, payload_type,
rtp_timestamp, capture_time_ms, payload_data,
payload_size, fragmentation, rtp_header,
result = video_->SendVideo(frame_type, payload_type, rtp_timestamp,
capture_time_ms, payload_data, payload_size,
fragmentation, rtp_header,
expected_retransmission_time_ms);
}

View File

@ -28,7 +28,6 @@
#include "modules/rtp_rtcp/source/playout_delay_oracle.h"
#include "modules/rtp_rtcp/source/rtp_packet_history.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_config.h"
#include "modules/rtp_rtcp/source/rtp_utility.h"
#include "rtc_base/constructor_magic.h"
#include "rtc_base/critical_section.h"
#include "rtc_base/deprecation.h"
@ -224,9 +223,6 @@ class RTPSender {
void SetRtt(int64_t rtt_ms);
protected:
int32_t CheckPayloadType(int8_t payload_type, VideoCodecType* video_type);
private:
// Maps capture time in milliseconds to send-side delay in milliseconds.
// Send-side delay is the difference between transmission time and capture
@ -291,7 +287,6 @@ class RTPSender {
size_t max_packet_size_;
int8_t last_payload_type_ RTC_GUARDED_BY(send_critsect_);
std::map<int8_t, RtpUtility::Payload*> payload_type_map_;
RtpHeaderExtensionMap rtp_header_extension_map_
RTC_GUARDED_BY(send_critsect_);

View File

@ -36,8 +36,7 @@ int32_t RTPSenderAudio::RegisterAudioPayload(absl::string_view payload_name,
const int8_t payload_type,
const uint32_t frequency,
const size_t channels,
const uint32_t rate,
RtpUtility::Payload** payload) {
const uint32_t rate) {
if (absl::EqualsIgnoreCase(payload_name, "cn")) {
rtc::CritScope cs(&send_audio_critsect_);
// we can have multiple CNG payload types
@ -65,10 +64,6 @@ int32_t RTPSenderAudio::RegisterAudioPayload(absl::string_view payload_name,
dtmf_payload_freq_ = frequency;
return 0;
}
*payload = new RtpUtility::Payload(
payload_name,
PayloadUnion(AudioPayload{
SdpAudioFormat(payload_name, frequency, channels), rate}));
return 0;
}

View File

@ -18,7 +18,6 @@
#include "common_types.h" // NOLINT(build/include)
#include "modules/rtp_rtcp/source/dtmf_queue.h"
#include "modules/rtp_rtcp/source/rtp_sender.h"
#include "modules/rtp_rtcp/source/rtp_utility.h"
#include "rtc_base/constructor_magic.h"
#include "rtc_base/critical_section.h"
#include "rtc_base/one_time_event.h"
@ -36,8 +35,7 @@ class RTPSenderAudio {
int8_t payload_type,
uint32_t frequency,
size_t channels,
uint32_t rate,
RtpUtility::Payload** payload);
uint32_t rate);
bool SendAudio(FrameType frame_type,
int8_t payload_type,

View File

@ -304,6 +304,7 @@ class RtpSenderVideoTest : public RtpSenderTest {
SetUpRtpSender(false, false);
rtp_sender_video_.reset(
new TestRtpSenderVideo(&fake_clock_, rtp_sender_.get(), nullptr));
rtp_sender_video_->RegisterPayloadType(kPayload, "generic");
}
std::unique_ptr<TestRtpSenderVideo> rtp_sender_video_;
};
@ -1894,9 +1895,9 @@ TEST_P(RtpSenderVideoTest, KeyFrameHasCVO) {
RTPVideoHeader hdr;
hdr.rotation = kVideoRotation_0;
rtp_sender_video_->SendVideo(kVideoCodecGeneric, kVideoFrameKey, kPayload,
kTimestamp, 0, kFrame, sizeof(kFrame), nullptr,
&hdr, kDefaultExpectedRetransmissionTimeMs);
rtp_sender_video_->SendVideo(kVideoFrameKey, kPayload, kTimestamp, 0, kFrame,
sizeof(kFrame), nullptr, &hdr,
kDefaultExpectedRetransmissionTimeMs);
VideoRotation rotation;
EXPECT_TRUE(
@ -1920,10 +1921,9 @@ TEST_P(RtpSenderVideoTest, TimingFrameHasPacketizationTimstampSet) {
hdr.video_timing.encode_finish_delta_ms = kEncodeFinishDeltaMs;
fake_clock_.AdvanceTimeMilliseconds(kPacketizationTimeMs);
rtp_sender_video_->SendVideo(kVideoCodecGeneric, kVideoFrameKey, kPayload,
kTimestamp, kCaptureTimestamp, kFrame,
sizeof(kFrame), nullptr, &hdr,
kDefaultExpectedRetransmissionTimeMs);
rtp_sender_video_->SendVideo(
kVideoFrameKey, kPayload, kTimestamp, kCaptureTimestamp, kFrame,
sizeof(kFrame), nullptr, &hdr, kDefaultExpectedRetransmissionTimeMs);
VideoSendTiming timing;
EXPECT_TRUE(transport_.last_sent_packet().GetExtension<VideoTimingExtension>(
&timing));
@ -1940,13 +1940,13 @@ TEST_P(RtpSenderVideoTest, DeltaFrameHasCVOWhenChanged) {
RTPVideoHeader hdr;
hdr.rotation = kVideoRotation_90;
EXPECT_TRUE(rtp_sender_video_->SendVideo(
kVideoCodecGeneric, kVideoFrameKey, kPayload, kTimestamp, 0, kFrame,
sizeof(kFrame), nullptr, &hdr, kDefaultExpectedRetransmissionTimeMs));
kVideoFrameKey, kPayload, kTimestamp, 0, kFrame, sizeof(kFrame), nullptr,
&hdr, kDefaultExpectedRetransmissionTimeMs));
hdr.rotation = kVideoRotation_0;
EXPECT_TRUE(rtp_sender_video_->SendVideo(
kVideoCodecGeneric, kVideoFrameDelta, kPayload, kTimestamp + 1, 0, kFrame,
sizeof(kFrame), nullptr, &hdr, kDefaultExpectedRetransmissionTimeMs));
kVideoFrameDelta, kPayload, kTimestamp + 1, 0, kFrame, sizeof(kFrame),
nullptr, &hdr, kDefaultExpectedRetransmissionTimeMs));
VideoRotation rotation;
EXPECT_TRUE(
@ -1962,12 +1962,12 @@ TEST_P(RtpSenderVideoTest, DeltaFrameHasCVOWhenNonZero) {
RTPVideoHeader hdr;
hdr.rotation = kVideoRotation_90;
EXPECT_TRUE(rtp_sender_video_->SendVideo(
kVideoCodecGeneric, kVideoFrameKey, kPayload, kTimestamp, 0, kFrame,
sizeof(kFrame), nullptr, &hdr, kDefaultExpectedRetransmissionTimeMs));
kVideoFrameKey, kPayload, kTimestamp, 0, kFrame, sizeof(kFrame), nullptr,
&hdr, kDefaultExpectedRetransmissionTimeMs));
EXPECT_TRUE(rtp_sender_video_->SendVideo(
kVideoCodecGeneric, kVideoFrameDelta, kPayload, kTimestamp + 1, 0, kFrame,
sizeof(kFrame), nullptr, &hdr, kDefaultExpectedRetransmissionTimeMs));
kVideoFrameDelta, kPayload, kTimestamp + 1, 0, kFrame, sizeof(kFrame),
nullptr, &hdr, kDefaultExpectedRetransmissionTimeMs));
VideoRotation rotation;
EXPECT_TRUE(
@ -2233,9 +2233,9 @@ TEST_P(RtpSenderVideoTest, PopulateGenericFrameDescriptor) {
generic.higher_spatial_layers.push_back(4);
generic.dependencies.push_back(kFrameId - 1);
generic.dependencies.push_back(kFrameId - 500);
rtp_sender_video_->SendVideo(kVideoCodecGeneric, kVideoFrameDelta, kPayload,
kTimestamp, 0, kFrame, sizeof(kFrame), nullptr,
&hdr, kDefaultExpectedRetransmissionTimeMs);
rtp_sender_video_->SendVideo(kVideoFrameDelta, kPayload, kTimestamp, 0,
kFrame, sizeof(kFrame), nullptr, &hdr,
kDefaultExpectedRetransmissionTimeMs);
RtpGenericFrameDescriptor descriptor_wire;
EXPECT_EQ(1U, transport_.sent_packets_.size());
@ -2266,9 +2266,10 @@ TEST_P(RtpSenderVideoTest,
vp8.keyIdx = 2;
RTPVideoHeader::GenericDescriptorInfo& generic = hdr.generic.emplace();
generic.frame_id = kFrameId;
rtp_sender_video_->SendVideo(kVideoCodecVP8, kVideoFrameDelta, kPayload,
kTimestamp, 0, kFrame, sizeof(kFrame), nullptr,
&hdr, kDefaultExpectedRetransmissionTimeMs);
rtp_sender_video_->RegisterPayloadType(kPayload, "vp8");
rtp_sender_video_->SendVideo(kVideoFrameDelta, kPayload, kTimestamp, 0,
kFrame, sizeof(kFrame), nullptr, &hdr,
kDefaultExpectedRetransmissionTimeMs);
ASSERT_THAT(transport_.sent_packets_, SizeIs(1));
// Expect only minimal 1-byte vp8 descriptor was generated.

View File

@ -156,7 +156,6 @@ RTPSenderVideo::RTPSenderVideo(Clock* clock,
bool require_frame_encryption)
: rtp_sender_(rtp_sender),
clock_(clock),
video_type_(kVideoCodecGeneric),
retransmission_settings_(kRetransmitBaseLayer |
kConditionallyRetransmitHigherLayers),
last_rotation_(kVideoRotation_0),
@ -176,19 +175,10 @@ RTPSenderVideo::RTPSenderVideo(Clock* clock,
RTPSenderVideo::~RTPSenderVideo() {}
void RTPSenderVideo::SetVideoCodecType(enum VideoCodecType video_type) {
video_type_ = video_type;
}
void RTPSenderVideo::RegisterPayloadType(int8_t payload_type,
absl::string_view payload_name) {
VideoCodecType video_type;
VideoCodecType RTPSenderVideo::VideoCodecType() const {
return video_type_;
}
// Static.
RtpUtility::Payload* RTPSenderVideo::CreateVideoPayload(
absl::string_view payload_name,
int8_t payload_type) {
enum VideoCodecType video_type = kVideoCodecGeneric;
if (absl::EqualsIgnoreCase(payload_name, "VP8")) {
video_type = kVideoCodecVP8;
} else if (absl::EqualsIgnoreCase(payload_name, "VP9")) {
@ -202,9 +192,9 @@ RtpUtility::Payload* RTPSenderVideo::CreateVideoPayload(
} else {
video_type = kVideoCodecGeneric;
}
VideoPayload vp;
vp.videoCodecType = video_type;
return new RtpUtility::Payload(payload_name, PayloadUnion(vp));
rtc::CritScope cs(&payload_type_crit_);
payload_type_map_[payload_type] = video_type;
}
void RTPSenderVideo::SendVideoPacket(std::unique_ptr<RtpPacketToSend> packet,
@ -376,8 +366,7 @@ absl::optional<uint32_t> RTPSenderVideo::FlexfecSsrc() const {
return absl::nullopt;
}
bool RTPSenderVideo::SendVideo(enum VideoCodecType video_type,
FrameType frame_type,
bool RTPSenderVideo::SendVideo(FrameType frame_type,
int8_t payload_type,
uint32_t rtp_timestamp,
int64_t capture_time_ms,
@ -536,6 +525,17 @@ bool RTPSenderVideo::SendVideo(enum VideoCodecType video_type,
<< "one is required since require_frame_encryptor is set";
}
VideoCodecType video_type;
{
rtc::CritScope cs(&payload_type_crit_);
const auto it = payload_type_map_.find(payload_type);
if (it == payload_type_map_.end()) {
RTC_LOG(LS_ERROR) << "Payload type " << static_cast<int>(payload_type)
<< " not registered.";
return false;
}
video_type = it->second;
}
std::unique_ptr<RtpPacketizer> packetizer = RtpPacketizer::Create(
video_type, rtc::MakeArrayView(payload_data, payload_size), limits,
*packetize_video_header, frame_type, fragmentation);

View File

@ -21,7 +21,6 @@
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_config.h"
#include "modules/rtp_rtcp/source/rtp_sender.h"
#include "modules/rtp_rtcp/source/rtp_utility.h"
#include "modules/rtp_rtcp/source/ulpfec_generator.h"
#include "rtc_base/critical_section.h"
#include "rtc_base/one_time_event.h"
@ -46,13 +45,7 @@ class RTPSenderVideo {
bool require_frame_encryption);
virtual ~RTPSenderVideo();
virtual enum VideoCodecType VideoCodecType() const;
static RtpUtility::Payload* CreateVideoPayload(absl::string_view payload_name,
int8_t payload_type);
bool SendVideo(enum VideoCodecType video_type,
FrameType frame_type,
bool SendVideo(FrameType frame_type,
int8_t payload_type,
uint32_t capture_timestamp,
int64_t capture_time_ms,
@ -62,7 +55,7 @@ class RTPSenderVideo {
const RTPVideoHeader* video_header,
int64_t expected_retransmission_time_ms);
void SetVideoCodecType(enum VideoCodecType type);
void RegisterPayloadType(int8_t payload_type, absl::string_view payload_name);
// ULPFEC.
void SetUlpfecConfig(int red_payload_type, int ulpfec_payload_type);
@ -133,10 +126,15 @@ class RTPSenderVideo {
RTPSender* const rtp_sender_;
Clock* const clock_;
// Maps payload type to codec type, for packetization.
// TODO(nisse): Set on construction, to avoid lock.
rtc::CriticalSection payload_type_crit_;
std::map<int8_t, VideoCodecType> payload_type_map_
RTC_GUARDED_BY(payload_type_crit_);
// Should never be held when calling out of this class.
rtc::CriticalSection crit_;
enum VideoCodecType video_type_;
int32_t retransmission_settings_ RTC_GUARDED_BY(crit_);
VideoRotation last_rotation_ RTC_GUARDED_BY(crit_);
absl::optional<ColorSpace> last_color_space_ RTC_GUARDED_BY(crit_);

View File

@ -26,16 +26,6 @@ const uint8_t kRtpMarkerBitMask = 0x80;
namespace RtpUtility {
struct Payload {
Payload(absl::string_view payload_name, const PayloadUnion& pu)
: typeSpecific(pu) {
size_t clipped_size = payload_name.copy(name, sizeof(name) - 1);
name[clipped_size] = '\0';
}
char name[RTP_PAYLOAD_NAME_SIZE];
PayloadUnion typeSpecific;
};
// Round up to the nearest size that is a multiple of 4.
size_t Word32Align(size_t size);