Remove the redundant method GetPayloadSpecifics

It's in the way of a refactoring.

Also change PayloadTypeToPayload---the method all callers can use instead---to return Optional<Payload> instead of const Payload* (for thread safety reasons: an object that protects itself with a mutex shouldn't be handing out pointers to parts of itself). 

BUG=webrtc:8159

Change-Id: I7ef0d545077ffdea016b309f2165e3c4955a2928
Reviewed-on: https://webrtc-review.googlesource.com/2360
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#19917}
This commit is contained in:
Karl Wiberg 2017-09-21 15:00:58 +02:00 committed by Commit Bot
parent 736a98ae5d
commit 73b60b82ee
8 changed files with 49 additions and 68 deletions

View File

@ -15,6 +15,7 @@
#include <set>
#include "api/audio_codecs/audio_format.h"
#include "api/optional.h"
#include "modules/rtp_rtcp/source/rtp_utility.h"
#include "rtc_base/criticalsection.h"
@ -55,11 +56,10 @@ class RTPPayloadRegistry {
bool IsRed(const RTPHeader& header) const;
bool GetPayloadSpecifics(uint8_t payload_type, PayloadUnion* payload) const;
int GetPayloadTypeFrequency(uint8_t payload_type) const;
const RtpUtility::Payload* PayloadTypeToPayload(uint8_t payload_type) const;
rtc::Optional<RtpUtility::Payload> PayloadTypeToPayload(
uint8_t payload_type) const;
void ResetLastReceivedPayloadTypes() {
rtc::CritScope cs(&crit_sect_);

View File

@ -303,22 +303,9 @@ bool RTPPayloadRegistry::IsRed(const RTPHeader& header) const {
return it != payload_type_map_.end() && _stricmp(it->second.name, "red") == 0;
}
bool RTPPayloadRegistry::GetPayloadSpecifics(uint8_t payload_type,
PayloadUnion* payload) const {
rtc::CritScope cs(&crit_sect_);
auto it = payload_type_map_.find(payload_type);
// Check that this is a registered payload type.
if (it == payload_type_map_.end()) {
return false;
}
*payload = it->second.typeSpecific;
return true;
}
int RTPPayloadRegistry::GetPayloadTypeFrequency(
uint8_t payload_type) const {
const RtpUtility::Payload* payload = PayloadTypeToPayload(payload_type);
const auto payload = PayloadTypeToPayload(payload_type);
if (!payload) {
return -1;
}
@ -327,18 +314,13 @@ int RTPPayloadRegistry::GetPayloadTypeFrequency(
: kVideoPayloadTypeFrequency;
}
const RtpUtility::Payload* RTPPayloadRegistry::PayloadTypeToPayload(
rtc::Optional<RtpUtility::Payload> RTPPayloadRegistry::PayloadTypeToPayload(
uint8_t payload_type) const {
rtc::CritScope cs(&crit_sect_);
auto it = payload_type_map_.find(payload_type);
// Check that this is a registered payload type.
if (it == payload_type_map_.end()) {
return nullptr;
}
return &it->second;
const auto it = payload_type_map_.find(payload_type);
return it == payload_type_map_.end()
? rtc::Optional<RtpUtility::Payload>()
: rtc::Optional<RtpUtility::Payload>(it->second);
}
void RTPPayloadRegistry::SetIncomingPayloadType(const RTPHeader& header) {

View File

@ -42,7 +42,7 @@ TEST(RtpPayloadRegistryTest,
EXPECT_EQ(0, rtp_payload_registry.RegisterReceivePayload(video_codec));
const RtpUtility::Payload* retrieved_payload =
const auto retrieved_payload =
rtp_payload_registry.PayloadTypeToPayload(payload_type);
EXPECT_TRUE(retrieved_payload);
@ -68,7 +68,7 @@ TEST(RtpPayloadRegistryTest,
EXPECT_TRUE(new_payload_created) << "A new payload WAS created.";
const RtpUtility::Payload* retrieved_payload =
const auto retrieved_payload =
rtp_payload_registry.PayloadTypeToPayload(payload_type);
EXPECT_TRUE(retrieved_payload);
@ -102,7 +102,7 @@ TEST(RtpPayloadRegistryTest, AudioRedWorkProperly) {
EXPECT_EQ(kRedPayloadType, rtp_payload_registry.red_payload_type());
const RtpUtility::Payload* retrieved_payload =
const auto retrieved_payload =
rtp_payload_registry.PayloadTypeToPayload(kRedPayloadType);
EXPECT_TRUE(retrieved_payload);
EXPECT_TRUE(retrieved_payload->audio);
@ -140,22 +140,23 @@ TEST(RtpPayloadRegistryTest,
<< "With a different payload type is fine though.";
// Ensure both payloads are preserved.
const RtpUtility::Payload* retrieved_payload =
const auto retrieved_payload1 =
rtp_payload_registry.PayloadTypeToPayload(payload_type);
EXPECT_TRUE(retrieved_payload);
EXPECT_STREQ(kTypicalPayloadName, retrieved_payload->name);
EXPECT_TRUE(retrieved_payload->audio);
EXPECT_EQ(kTypicalFrequency, retrieved_payload->typeSpecific.Audio.frequency);
EXPECT_EQ(kTypicalChannels, retrieved_payload->typeSpecific.Audio.channels);
EXPECT_TRUE(retrieved_payload1);
EXPECT_STREQ(kTypicalPayloadName, retrieved_payload1->name);
EXPECT_TRUE(retrieved_payload1->audio);
EXPECT_EQ(kTypicalFrequency,
retrieved_payload1->typeSpecific.Audio.frequency);
EXPECT_EQ(kTypicalChannels, retrieved_payload1->typeSpecific.Audio.channels);
retrieved_payload =
const auto retrieved_payload2 =
rtp_payload_registry.PayloadTypeToPayload(payload_type - 1);
EXPECT_TRUE(retrieved_payload);
EXPECT_STREQ(kTypicalPayloadName, retrieved_payload->name);
EXPECT_TRUE(retrieved_payload->audio);
EXPECT_TRUE(retrieved_payload2);
EXPECT_STREQ(kTypicalPayloadName, retrieved_payload2->name);
EXPECT_TRUE(retrieved_payload2->audio);
EXPECT_EQ(kTypicalFrequency + 1,
retrieved_payload->typeSpecific.Audio.frequency);
EXPECT_EQ(kTypicalChannels, retrieved_payload->typeSpecific.Audio.channels);
retrieved_payload2->typeSpecific.Audio.frequency);
EXPECT_EQ(kTypicalChannels, retrieved_payload2->typeSpecific.Audio.channels);
// Ok, update the rate for one of the codecs. If either the incoming rate or
// the stored rate is zero it's not really an error to register the same

View File

@ -294,7 +294,7 @@ void RtpReceiverImpl::CheckSSRCChanged(const RTPHeader& rtp_header) {
if (rtp_header.payloadType == last_received_payload_type) {
re_initialize_decoder = true;
const Payload* payload = rtp_payload_registry_->PayloadTypeToPayload(
const auto payload = rtp_payload_registry_->PayloadTypeToPayload(
rtp_header.payloadType);
if (!payload) {
return;
@ -382,7 +382,7 @@ int32_t RtpReceiverImpl::CheckPayloadChanged(const RTPHeader& rtp_header,
return 0;
}
const Payload* payload =
const auto payload =
rtp_payload_registry_->PayloadTypeToPayload(payload_type);
if (!payload) {
// Not a registered payload type.

View File

@ -48,9 +48,9 @@ bool LoopBackTransport::SendRtp(const uint8_t* data,
if (!parser->Parse(data, len, &header)) {
return false;
}
PayloadUnion payload_specific;
if (!rtp_payload_registry_->GetPayloadSpecifics(header.payloadType,
&payload_specific)) {
const auto pl =
rtp_payload_registry_->PayloadTypeToPayload(header.payloadType);
if (!pl) {
return false;
}
const uint8_t* payload = data + header.headerLength;
@ -58,7 +58,7 @@ bool LoopBackTransport::SendRtp(const uint8_t* data,
const size_t payload_length = len - header.headerLength;
receive_statistics_->IncomingPacket(header, len, false);
return rtp_receiver_->IncomingRtpPacket(header, payload, payload_length,
payload_specific, true);
pl->typeSpecific, true);
}
bool LoopBackTransport::SendRtcp(const uint8_t* data, size_t len) {

View File

@ -165,14 +165,13 @@ TEST_F(RtpRtcpVideoTest, PaddingOnlyFrames) {
RTPHeader header;
std::unique_ptr<RtpHeaderParser> parser(RtpHeaderParser::Create());
EXPECT_TRUE(parser->Parse(padding_packet, packet_size, &header));
PayloadUnion payload_specific;
EXPECT_TRUE(rtp_payload_registry_.GetPayloadSpecifics(header.payloadType,
&payload_specific));
const auto pl =
rtp_payload_registry_.PayloadTypeToPayload(header.payloadType);
EXPECT_TRUE(pl);
const uint8_t* payload = padding_packet + header.headerLength;
const size_t payload_length = packet_size - header.headerLength;
EXPECT_TRUE(rtp_receiver_->IncomingRtpPacket(header, payload,
payload_length,
payload_specific, true));
EXPECT_TRUE(rtp_receiver_->IncomingRtpPacket(
header, payload, payload_length, pl->typeSpecific, true));
EXPECT_EQ(0u, receiver_->payload_size());
EXPECT_EQ(payload_length, receiver_->rtp_header().header.paddingLength);
}

View File

@ -460,13 +460,12 @@ void RtpVideoStreamReceiver::ReceivePacket(const uint8_t* packet,
const uint8_t* payload = packet + header.headerLength;
assert(packet_length >= header.headerLength);
size_t payload_length = packet_length - header.headerLength;
PayloadUnion payload_specific;
if (!rtp_payload_registry_.GetPayloadSpecifics(header.payloadType,
&payload_specific)) {
return;
const auto pl =
rtp_payload_registry_.PayloadTypeToPayload(header.payloadType);
if (pl) {
rtp_receiver_->IncomingRtpPacket(header, payload, payload_length,
pl->typeSpecific, in_order);
}
rtp_receiver_->IncomingRtpPacket(header, payload, payload_length,
payload_specific, in_order);
}
void RtpVideoStreamReceiver::ParseAndHandleEncapsulatingHeader(
@ -501,13 +500,13 @@ void RtpVideoStreamReceiver::NotifyReceiverOfFecPacket(
rtp_header.header = header;
rtp_header.header.payloadType = last_media_payload_type;
rtp_header.header.paddingLength = 0;
PayloadUnion payload_specific;
if (!rtp_payload_registry_.GetPayloadSpecifics(last_media_payload_type,
&payload_specific)) {
const auto pl =
rtp_payload_registry_.PayloadTypeToPayload(last_media_payload_type);
if (!pl) {
LOG(LS_WARNING) << "Failed to get payload specifics.";
return;
}
rtp_header.type.Video.codec = payload_specific.Video.videoCodecType;
rtp_header.type.Video.codec = pl->typeSpecific.Video.videoCodecType;
rtp_header.type.Video.rotation = kVideoRotation_0;
if (header.extension.hasVideoRotation) {
rtp_header.type.Video.rotation = header.extension.videoRotation;

View File

@ -1391,13 +1391,13 @@ bool Channel::ReceivePacket(const uint8_t* packet,
const uint8_t* payload = packet + header.headerLength;
assert(packet_length >= header.headerLength);
size_t payload_length = packet_length - header.headerLength;
PayloadUnion payload_specific;
if (!rtp_payload_registry_->GetPayloadSpecifics(header.payloadType,
&payload_specific)) {
const auto pl =
rtp_payload_registry_->PayloadTypeToPayload(header.payloadType);
if (!pl) {
return false;
}
return rtp_receiver_->IncomingRtpPacket(header, payload, payload_length,
payload_specific, in_order);
pl->typeSpecific, in_order);
}
bool Channel::IsPacketInOrder(const RTPHeader& header) const {