From 33c81d05613f45f65ee17224ed381c6cdd1c6c6f Mon Sep 17 00:00:00 2001 From: magjed Date: Thu, 24 Nov 2016 11:08:39 -0800 Subject: [PATCH] Revert of Remove RTPPayloadStrategy and simplify RTPPayloadRegistry (patchset #7 id:240001 of https://codereview.webrtc.org/2524923002/ ) Reason for revert: Breaks downstream projects. Original issue's description: > Remove RTPPayloadStrategy and simplify RTPPayloadRegistry > > This CL removes RTPPayloadStrategy that is currently used to handle > audio/video specific aspects of payload handling. Instead, the audio and > video specific aspects will now have different functions, with linear > code flow. > > This CL does not contain any functional changes, and is just a > preparation for future CL:s. > > The main purpose with this CL is to add this function: > bool PayloadIsCompatible(const RtpUtility::Payload& payload, > const webrtc::VideoCodec& video_codec); > that can easily be extended in a future CL to look at video codec > specific information. > > BUG=webrtc:6743 > > Committed: https://crrev.com/b881254dc86d2cc80a52e08155433458be002166 > Cr-Commit-Position: refs/heads/master@{#15232} TBR=danilchap@webrtc.org,solenberg@webrtc.org,mflodman@webrtc.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=webrtc:6743 Review-Url: https://codereview.webrtc.org/2528993002 Cr-Commit-Position: refs/heads/master@{#15234} --- webrtc/modules/BUILD.gn | 1 + webrtc/modules/rtp_rtcp/BUILD.gn | 1 + .../rtp_rtcp/include/rtp_payload_registry.h | 75 +++- .../source/mock/mock_rtp_payload_strategy.h | 43 ++ .../rtp_rtcp/source/nack_rtx_unittest.cc | 3 +- .../rtp_rtcp/source/rtp_payload_registry.cc | 410 +++++++++++------- .../source/rtp_payload_registry_unittest.cc | 277 +++++++----- .../rtp_rtcp/source/rtp_receiver_audio.h | 12 + webrtc/modules/rtp_rtcp/source/rtp_utility.h | 2 + .../modules/rtp_rtcp/test/testAPI/test_api.cc | 3 +- .../rtp_rtcp/test/testAPI/test_api_audio.cc | 6 +- .../rtp_rtcp/test/testAPI/test_api_rtcp.cc | 6 +- .../rtp_rtcp/test/testAPI/test_api_video.cc | 3 +- .../modules/video_coding/test/rtp_player.cc | 3 +- webrtc/video/rtp_stream_receiver.cc | 1 + webrtc/voice_engine/channel.cc | 3 +- 16 files changed, 558 insertions(+), 291 deletions(-) create mode 100644 webrtc/modules/rtp_rtcp/source/mock/mock_rtp_payload_strategy.h diff --git a/webrtc/modules/BUILD.gn b/webrtc/modules/BUILD.gn index 498004fb06..b7db3d019d 100644 --- a/webrtc/modules/BUILD.gn +++ b/webrtc/modules/BUILD.gn @@ -426,6 +426,7 @@ if (rtc_include_tests) { "rtp_rtcp/source/flexfec_header_reader_writer_unittest.cc", "rtp_rtcp/source/flexfec_receiver_unittest.cc", "rtp_rtcp/source/flexfec_sender_unittest.cc", + "rtp_rtcp/source/mock/mock_rtp_payload_strategy.h", "rtp_rtcp/source/nack_rtx_unittest.cc", "rtp_rtcp/source/packet_loss_stats_unittest.cc", "rtp_rtcp/source/playout_delay_oracle_unittest.cc", diff --git a/webrtc/modules/rtp_rtcp/BUILD.gn b/webrtc/modules/rtp_rtcp/BUILD.gn index aa5403dcd8..66183a85a1 100644 --- a/webrtc/modules/rtp_rtcp/BUILD.gn +++ b/webrtc/modules/rtp_rtcp/BUILD.gn @@ -34,6 +34,7 @@ rtc_static_library("rtp_rtcp") { "source/forward_error_correction.h", "source/forward_error_correction_internal.cc", "source/forward_error_correction_internal.h", + "source/mock/mock_rtp_payload_strategy.h", "source/packet_loss_stats.cc", "source/packet_loss_stats.h", "source/playout_delay_oracle.cc", diff --git a/webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h b/webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h index 668fe876f2..8f9d5829f9 100644 --- a/webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h +++ b/webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h @@ -25,24 +25,46 @@ namespace webrtc { struct CodecInst; class VideoCodec; -// TODO(magjed): Remove once external code is updated. +// This strategy deals with the audio/video-specific aspects +// of payload handling. class RTPPayloadStrategy { public: - static RTPPayloadStrategy* CreateStrategy(bool handling_audio) { - return nullptr; - } + virtual ~RTPPayloadStrategy() {} + + virtual bool CodecsMustBeUnique() const = 0; + + virtual bool PayloadIsCompatible(const RtpUtility::Payload& payload, + uint32_t frequency, + size_t channels, + uint32_t rate) const = 0; + + virtual void UpdatePayloadRate(RtpUtility::Payload* payload, + uint32_t rate) const = 0; + + virtual RtpUtility::Payload* CreatePayloadType(const char* payload_name, + int8_t payload_type, + uint32_t frequency, + size_t channels, + uint32_t rate) const = 0; + + virtual int GetPayloadTypeFrequency( + const RtpUtility::Payload& payload) const = 0; + + static RTPPayloadStrategy* CreateStrategy(bool handling_audio); + + protected: + RTPPayloadStrategy() {} }; class RTPPayloadRegistry { public: - RTPPayloadRegistry(); + // The registry takes ownership of the strategy. + explicit RTPPayloadRegistry(RTPPayloadStrategy* rtp_payload_strategy); ~RTPPayloadRegistry(); - // TODO(magjed): Remove once external code is updated. - explicit RTPPayloadRegistry(RTPPayloadStrategy* rtp_payload_strategy) - : RTPPayloadRegistry() {} // TODO(magjed): Split RTPPayloadRegistry into separate Audio and Video class - // and simplify the code. http://crbug/webrtc/6743. + // and remove RTPPayloadStrategy, RTPPayloadVideoStrategy, + // RTPPayloadAudioStrategy, and simplify the code. http://crbug/webrtc/6743. int32_t RegisterReceivePayload(const CodecInst& audio_codec, bool* created_new_payload_type); int32_t RegisterReceivePayload(const VideoCodec& video_codec); @@ -96,9 +118,13 @@ class RTPPayloadRegistry { // Returns true if the new media payload type has not changed. bool ReportMediaPayloadType(uint8_t media_payload_type); - int8_t red_payload_type() const { return GetPayloadTypeWithName("red"); } + int8_t red_payload_type() const { + rtc::CritScope cs(&crit_sect_); + return red_payload_type_; + } int8_t ulpfec_payload_type() const { - return GetPayloadTypeWithName("ulpfec"); + rtc::CritScope cs(&crit_sect_); + return ulpfec_payload_type_; } int8_t last_received_payload_type() const { rtc::CritScope cs(&crit_sect_); @@ -117,17 +143,34 @@ class RTPPayloadRegistry { RTC_DEPRECATED void set_use_rtx_payload_mapping_on_restore(bool val) {} private: + int32_t RegisterReceivePayload(const char* payload_name, + int8_t payload_type, + uint32_t frequency, + size_t channels, + uint32_t rate, + bool* created_new_payload_type); + + int32_t ReceivePayloadType(const char* payload_name, + uint32_t frequency, + size_t channels, + uint32_t rate, + int8_t* payload_type) const; + // Prunes the payload type map of the specific payload type, if it exists. void DeregisterAudioCodecOrRedTypeRegardlessOfPayloadType( - const CodecInst& audio_codec); + const char* payload_name, + size_t payload_name_length, + uint32_t frequency, + size_t channels, + uint32_t rate); bool IsRtxInternal(const RTPHeader& header) const; - // Returns the payload type for the payload with name |payload_name|, or -1 if - // no such payload is registered. - int8_t GetPayloadTypeWithName(const char* payload_name) const; rtc::CriticalSection crit_sect_; - std::map payload_type_map_; + RtpUtility::PayloadTypeMap payload_type_map_; + std::unique_ptr rtp_payload_strategy_; + int8_t red_payload_type_; + int8_t ulpfec_payload_type_; int8_t incoming_payload_type_; int8_t last_received_payload_type_; int8_t last_received_media_payload_type_; diff --git a/webrtc/modules/rtp_rtcp/source/mock/mock_rtp_payload_strategy.h b/webrtc/modules/rtp_rtcp/source/mock/mock_rtp_payload_strategy.h new file mode 100644 index 0000000000..458f5a8945 --- /dev/null +++ b/webrtc/modules/rtp_rtcp/source/mock/mock_rtp_payload_strategy.h @@ -0,0 +1,43 @@ +/* + * Copyright (c) 2012 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. + */ + +#ifndef WEBRTC_MODULES_RTP_RTCP_SOURCE_MOCK_MOCK_RTP_PAYLOAD_STRATEGY_H_ +#define WEBRTC_MODULES_RTP_RTCP_SOURCE_MOCK_MOCK_RTP_PAYLOAD_STRATEGY_H_ + +#include "webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h" +#include "webrtc/test/gmock.h" + +namespace webrtc { + +class MockRTPPayloadStrategy : public RTPPayloadStrategy { + public: + MOCK_CONST_METHOD0(CodecsMustBeUnique, + bool()); + MOCK_CONST_METHOD4(PayloadIsCompatible, + bool(const RtpUtility::Payload& payload, + uint32_t frequency, + size_t channels, + uint32_t rate)); + MOCK_CONST_METHOD2(UpdatePayloadRate, + void(RtpUtility::Payload* payload, uint32_t rate)); + MOCK_CONST_METHOD1(GetPayloadTypeFrequency, + int(const RtpUtility::Payload& payload)); + MOCK_CONST_METHOD5( + CreatePayloadType, + RtpUtility::Payload*(const char payload_name[RTP_PAYLOAD_NAME_SIZE], + int8_t payload_type, + uint32_t frequency, + size_t channels, + uint32_t rate)); +}; + +} // namespace webrtc + +#endif // WEBRTC_MODULES_RTP_RTCP_SOURCE_MOCK_MOCK_RTP_PAYLOAD_STRATEGY_H_ diff --git a/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc b/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc index e52f9d55f7..5b0407c8b2 100644 --- a/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc @@ -169,7 +169,8 @@ class RtxLoopBackTransport : public webrtc::Transport { class RtpRtcpRtxNackTest : public ::testing::Test { protected: RtpRtcpRtxNackTest() - : rtp_rtcp_module_(nullptr), + : rtp_payload_registry_(RTPPayloadStrategy::CreateStrategy(false)), + rtp_rtcp_module_(nullptr), transport_(kTestSsrc + 1), receiver_(), payload_data_length(sizeof(payload_data)), diff --git a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc index 7d4afc6e36..a4354d393e 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc @@ -10,76 +10,55 @@ #include "webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h" -#include - -#include "webrtc/base/checks.h" #include "webrtc/base/logging.h" -#include "webrtc/base/stringutils.h" #include "webrtc/common_types.h" #include "webrtc/modules/rtp_rtcp/source/byte_io.h" namespace webrtc { -namespace { +RTPPayloadRegistry::RTPPayloadRegistry(RTPPayloadStrategy* rtp_payload_strategy) + : rtp_payload_strategy_(rtp_payload_strategy), + red_payload_type_(-1), + ulpfec_payload_type_(-1), + incoming_payload_type_(-1), + last_received_payload_type_(-1), + last_received_media_payload_type_(-1), + rtx_(false), + ssrc_rtx_(0) {} -bool PayloadIsCompatible(const RtpUtility::Payload& payload, - const CodecInst& audio_codec) { - if (!payload.audio) - return false; - if (_stricmp(payload.name, audio_codec.plname) != 0) - return false; - const AudioPayload& audio_payload = payload.typeSpecific.Audio; - const uint32_t rate = std::max(0, audio_codec.rate); - return audio_payload.frequency == static_cast(audio_codec.plfreq) && - audio_payload.channels == audio_codec.channels && - (audio_payload.rate == rate || audio_payload.rate == 0 || rate == 0); -} - -bool PayloadIsCompatible(const RtpUtility::Payload& payload, - const VideoCodec& video_codec) { - return !payload.audio && _stricmp(payload.name, video_codec.plName) == 0; -} - -RtpUtility::Payload CreatePayloadType(const CodecInst& audio_codec) { - RtpUtility::Payload payload; - payload.name[RTP_PAYLOAD_NAME_SIZE - 1] = 0; - strncpy(payload.name, audio_codec.plname, RTP_PAYLOAD_NAME_SIZE - 1); - RTC_DCHECK_GE(audio_codec.plfreq, 1000); - payload.typeSpecific.Audio.frequency = audio_codec.plfreq; - payload.typeSpecific.Audio.channels = audio_codec.channels; - payload.typeSpecific.Audio.rate = std::max(0, audio_codec.rate); - payload.audio = true; - return payload; -} - -RtpVideoCodecTypes ConvertToRtpVideoCodecType(VideoCodecType type) { - switch (type) { - case kVideoCodecVP8: - return kRtpVideoVp8; - case kVideoCodecVP9: - return kRtpVideoVp9; - case kVideoCodecH264: - return kRtpVideoH264; - case kVideoCodecRED: - case kVideoCodecULPFEC: - return kRtpVideoNone; - default: - return kRtpVideoGeneric; +RTPPayloadRegistry::~RTPPayloadRegistry() { + while (!payload_type_map_.empty()) { + RtpUtility::PayloadTypeMap::iterator it = payload_type_map_.begin(); + delete it->second; + payload_type_map_.erase(it); } } -RtpUtility::Payload CreatePayloadType(const VideoCodec& video_codec) { - RtpUtility::Payload payload; - payload.name[RTP_PAYLOAD_NAME_SIZE - 1] = 0; - strncpy(payload.name, video_codec.plName, RTP_PAYLOAD_NAME_SIZE - 1); - payload.typeSpecific.Video.videoCodecType = - ConvertToRtpVideoCodecType(video_codec.codecType); - payload.audio = false; - return payload; +int32_t RTPPayloadRegistry::RegisterReceivePayload(const CodecInst& audio_codec, + bool* created_new_payload) { + return RegisterReceivePayload( + audio_codec.plname, audio_codec.pltype, audio_codec.plfreq, + audio_codec.channels, std::max(0, audio_codec.rate), created_new_payload); } -bool IsPayloadTypeValid(int8_t payload_type) { +int32_t RTPPayloadRegistry::RegisterReceivePayload( + const VideoCodec& video_codec) { + bool dummy_created_new_payload; + return RegisterReceivePayload(video_codec.plName, video_codec.plType, + kVideoPayloadTypeFrequency, 0 /* channels */, + 0 /* rate */, &dummy_created_new_payload); +} + +int32_t RTPPayloadRegistry::RegisterReceivePayload( + const char* const payload_name, + const int8_t payload_type, + const uint32_t frequency, + const size_t channels, + const uint32_t rate, + bool* created_new_payload) { assert(payload_type >= 0); + assert(payload_name); + *created_new_payload = false; // Sanity check. switch (payload_type) { @@ -95,76 +74,59 @@ bool IsPayloadTypeValid(int8_t payload_type) { case 79: // 207 Extended report. LOG(LS_ERROR) << "Can't register invalid receiver payload type: " << payload_type; - return false; + return -1; default: - return true; + break; } -} -} // namespace - -RTPPayloadRegistry::RTPPayloadRegistry() - : incoming_payload_type_(-1), - last_received_payload_type_(-1), - last_received_media_payload_type_(-1), - rtx_(false), - ssrc_rtx_(0) {} - -RTPPayloadRegistry::~RTPPayloadRegistry() = default; - -int32_t RTPPayloadRegistry::RegisterReceivePayload(const CodecInst& audio_codec, - bool* created_new_payload) { - *created_new_payload = false; - if (!IsPayloadTypeValid(audio_codec.pltype)) - return -1; + size_t payload_name_length = strlen(payload_name); rtc::CritScope cs(&crit_sect_); - auto it = payload_type_map_.find(audio_codec.pltype); + RtpUtility::PayloadTypeMap::iterator it = + payload_type_map_.find(payload_type); + if (it != payload_type_map_.end()) { - // We already use this payload type. Check if it's the same as we already - // have. If same, ignore sending an error. - if (PayloadIsCompatible(it->second, audio_codec)) { - it->second.typeSpecific.Audio.rate = std::max(0, audio_codec.rate); - return 0; + // We already use this payload type. + RtpUtility::Payload* payload = it->second; + + assert(payload); + + size_t name_length = strlen(payload->name); + + // Check if it's the same as we already have. + // If same, ignore sending an error. + if (payload_name_length == name_length && + RtpUtility::StringCompare( + payload->name, payload_name, payload_name_length)) { + if (rtp_payload_strategy_->PayloadIsCompatible(*payload, frequency, + channels, rate)) { + rtp_payload_strategy_->UpdatePayloadRate(payload, rate); + return 0; + } } - LOG(LS_ERROR) << "Payload type already registered: " << audio_codec.pltype; + LOG(LS_ERROR) << "Payload type already registered: " + << static_cast(payload_type); return -1; } - // Audio codecs must be unique. - DeregisterAudioCodecOrRedTypeRegardlessOfPayloadType(audio_codec); + if (rtp_payload_strategy_->CodecsMustBeUnique()) { + DeregisterAudioCodecOrRedTypeRegardlessOfPayloadType( + payload_name, payload_name_length, frequency, channels, rate); + } - payload_type_map_[audio_codec.pltype] = CreatePayloadType(audio_codec); + RtpUtility::Payload* payload = rtp_payload_strategy_->CreatePayloadType( + payload_name, payload_type, frequency, channels, rate); + + payload_type_map_[payload_type] = payload; *created_new_payload = true; - // Successful set of payload type, clear the value of last received payload - // type since it might mean something else. - last_received_payload_type_ = -1; - last_received_media_payload_type_ = -1; - return 0; -} - -int32_t RTPPayloadRegistry::RegisterReceivePayload( - const VideoCodec& video_codec) { - if (!IsPayloadTypeValid(video_codec.plType)) - return -1; - - rtc::CritScope cs(&crit_sect_); - - auto it = payload_type_map_.find(video_codec.plType); - if (it != payload_type_map_.end()) { - // We already use this payload type. Check if it's the same as we already - // have. If same, ignore sending an error. - if (PayloadIsCompatible(it->second, video_codec)) - return 0; - LOG(LS_ERROR) << "Payload type already registered: " - << static_cast(video_codec.plType); - return -1; + if (RtpUtility::StringCompare(payload_name, "red", 3)) { + red_payload_type_ = payload_type; + } else if (RtpUtility::StringCompare(payload_name, "ulpfec", 6)) { + ulpfec_payload_type_ = payload_type; } - payload_type_map_[video_codec.plType] = CreatePayloadType(video_codec); - // Successful set of payload type, clear the value of last received payload // type since it might mean something else. last_received_payload_type_ = -1; @@ -175,7 +137,11 @@ int32_t RTPPayloadRegistry::RegisterReceivePayload( int32_t RTPPayloadRegistry::DeRegisterReceivePayload( const int8_t payload_type) { rtc::CritScope cs(&crit_sect_); - payload_type_map_.erase(payload_type); + RtpUtility::PayloadTypeMap::iterator it = + payload_type_map_.find(payload_type); + assert(it != payload_type_map_.end()); + delete it->second; + payload_type_map_.erase(it); return 0; } @@ -183,40 +149,95 @@ int32_t RTPPayloadRegistry::DeRegisterReceivePayload( // for audio codecs, but there can for video. // Always called from within a critical section. void RTPPayloadRegistry::DeregisterAudioCodecOrRedTypeRegardlessOfPayloadType( - const CodecInst& audio_codec) { - for (auto iterator = payload_type_map_.begin(); - iterator != payload_type_map_.end(); ++iterator) { - if (PayloadIsCompatible(iterator->second, audio_codec)) { - // Remove old setting. - payload_type_map_.erase(iterator); - break; + const char* const payload_name, + const size_t payload_name_length, + const uint32_t frequency, + const size_t channels, + const uint32_t rate) { + RtpUtility::PayloadTypeMap::iterator iterator = payload_type_map_.begin(); + for (; iterator != payload_type_map_.end(); ++iterator) { + RtpUtility::Payload* payload = iterator->second; + size_t name_length = strlen(payload->name); + + if (payload_name_length == name_length && + RtpUtility::StringCompare( + payload->name, payload_name, payload_name_length)) { + // We found the payload name in the list. + // If audio, check frequency and rate. + if (payload->audio) { + if (rtp_payload_strategy_->PayloadIsCompatible(*payload, frequency, + channels, rate)) { + // Remove old setting. + delete payload; + payload_type_map_.erase(iterator); + break; + } + } else if (RtpUtility::StringCompare(payload_name, "red", 3)) { + delete payload; + payload_type_map_.erase(iterator); + break; + } } } } int32_t RTPPayloadRegistry::ReceivePayloadType(const CodecInst& audio_codec, int8_t* payload_type) const { - assert(payload_type); - rtc::CritScope cs(&crit_sect_); - - for (const auto& it : payload_type_map_) { - if (PayloadIsCompatible(it.second, audio_codec)) { - *payload_type = it.first; - return 0; - } - } - return -1; + return ReceivePayloadType(audio_codec.plname, audio_codec.plfreq, + audio_codec.channels, std::max(0, audio_codec.rate), + payload_type); } int32_t RTPPayloadRegistry::ReceivePayloadType(const VideoCodec& video_codec, int8_t* payload_type) const { + return ReceivePayloadType(video_codec.plName, kVideoPayloadTypeFrequency, + 0 /* channels */, 0 /* rate */, payload_type); +} + +int32_t RTPPayloadRegistry::ReceivePayloadType(const char* const payload_name, + const uint32_t frequency, + const size_t channels, + const uint32_t rate, + int8_t* payload_type) const { assert(payload_type); + size_t payload_name_length = strlen(payload_name); + rtc::CritScope cs(&crit_sect_); - for (const auto& it : payload_type_map_) { - if (PayloadIsCompatible(it.second, video_codec)) { - *payload_type = it.first; - return 0; + RtpUtility::PayloadTypeMap::const_iterator it = payload_type_map_.begin(); + + for (; it != payload_type_map_.end(); ++it) { + RtpUtility::Payload* payload = it->second; + assert(payload); + + size_t name_length = strlen(payload->name); + if (payload_name_length == name_length && + RtpUtility::StringCompare( + payload->name, payload_name, payload_name_length)) { + // Name matches. + if (payload->audio) { + if (rate == 0) { + // [default] audio, check freq and channels. + if (payload->typeSpecific.Audio.frequency == frequency && + payload->typeSpecific.Audio.channels == channels) { + *payload_type = it->first; + return 0; + } + } else { + // Non-default audio, check freq, channels and rate. + if (payload->typeSpecific.Audio.frequency == frequency && + payload->typeSpecific.Audio.channels == channels && + payload->typeSpecific.Audio.rate == rate) { + // extra rate condition added + *payload_type = it->first; + return 0; + } + } + } else { + // Video. + *payload_type = it->first; + return 0; + } } } return -1; @@ -312,8 +333,7 @@ void RTPPayloadRegistry::SetRtxPayloadType(int payload_type, bool RTPPayloadRegistry::IsRed(const RTPHeader& header) const { rtc::CritScope cs(&crit_sect_); - auto it = payload_type_map_.find(header.payloadType); - return it != payload_type_map_.end() && _stricmp(it->second.name, "red") == 0; + return red_payload_type_ == header.payloadType; } bool RTPPayloadRegistry::IsEncapsulated(const RTPHeader& header) const { @@ -323,13 +343,14 @@ bool RTPPayloadRegistry::IsEncapsulated(const RTPHeader& header) const { bool RTPPayloadRegistry::GetPayloadSpecifics(uint8_t payload_type, PayloadUnion* payload) const { rtc::CritScope cs(&crit_sect_); - auto it = payload_type_map_.find(payload_type); + RtpUtility::PayloadTypeMap::const_iterator 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; + *payload = it->second->typeSpecific; return true; } @@ -340,22 +361,22 @@ int RTPPayloadRegistry::GetPayloadTypeFrequency( return -1; } rtc::CritScope cs(&crit_sect_); - return payload->audio ? payload->typeSpecific.Audio.frequency - : kVideoPayloadTypeFrequency; + return rtp_payload_strategy_->GetPayloadTypeFrequency(*payload); } const RtpUtility::Payload* RTPPayloadRegistry::PayloadTypeToPayload( uint8_t payload_type) const { rtc::CritScope cs(&crit_sect_); - auto it = payload_type_map_.find(payload_type); + RtpUtility::PayloadTypeMap::const_iterator 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; + return it->second; } void RTPPayloadRegistry::SetIncomingPayloadType(const RTPHeader& header) { @@ -374,15 +395,106 @@ bool RTPPayloadRegistry::ReportMediaPayloadType(uint8_t media_payload_type) { return false; } -// Returns -1 if a payload with name |payload_name| is not registered. -int8_t RTPPayloadRegistry::GetPayloadTypeWithName( - const char* payload_name) const { - rtc::CritScope cs(&crit_sect_); - for (const auto& it : payload_type_map_) { - if (_stricmp(it.second.name, payload_name) == 0) - return it.first; +class RTPPayloadAudioStrategy : public RTPPayloadStrategy { + public: + bool CodecsMustBeUnique() const override { return true; } + + bool PayloadIsCompatible(const RtpUtility::Payload& payload, + const uint32_t frequency, + const size_t channels, + const uint32_t rate) const override { + return + payload.audio && + payload.typeSpecific.Audio.frequency == frequency && + payload.typeSpecific.Audio.channels == channels && + (payload.typeSpecific.Audio.rate == rate || + payload.typeSpecific.Audio.rate == 0 || rate == 0); + } + + void UpdatePayloadRate(RtpUtility::Payload* payload, + const uint32_t rate) const override { + payload->typeSpecific.Audio.rate = rate; + } + + RtpUtility::Payload* CreatePayloadType(const char* const payloadName, + const int8_t payloadType, + const uint32_t frequency, + const size_t channels, + const uint32_t rate) const override { + RtpUtility::Payload* payload = new RtpUtility::Payload; + payload->name[RTP_PAYLOAD_NAME_SIZE - 1] = 0; + strncpy(payload->name, payloadName, RTP_PAYLOAD_NAME_SIZE - 1); + assert(frequency >= 1000); + payload->typeSpecific.Audio.frequency = frequency; + payload->typeSpecific.Audio.channels = channels; + payload->typeSpecific.Audio.rate = rate; + payload->audio = true; + return payload; + } + + int GetPayloadTypeFrequency( + const RtpUtility::Payload& payload) const override { + return payload.typeSpecific.Audio.frequency; + } +}; + +class RTPPayloadVideoStrategy : public RTPPayloadStrategy { + public: + bool CodecsMustBeUnique() const override { return false; } + + bool PayloadIsCompatible(const RtpUtility::Payload& payload, + const uint32_t frequency, + const size_t channels, + const uint32_t rate) const override { + return !payload.audio; + } + + void UpdatePayloadRate(RtpUtility::Payload* payload, + const uint32_t rate) const override {} + + RtpUtility::Payload* CreatePayloadType(const char* const payloadName, + const int8_t payloadType, + const uint32_t frequency, + const size_t channels, + const uint32_t rate) const override { + RtpVideoCodecTypes videoType = kRtpVideoGeneric; + + if (RtpUtility::StringCompare(payloadName, "VP8", 3)) { + videoType = kRtpVideoVp8; + } else if (RtpUtility::StringCompare(payloadName, "VP9", 3)) { + videoType = kRtpVideoVp9; + } else if (RtpUtility::StringCompare(payloadName, "H264", 4)) { + videoType = kRtpVideoH264; + } else if (RtpUtility::StringCompare(payloadName, "I420", 4)) { + videoType = kRtpVideoGeneric; + } else if (RtpUtility::StringCompare(payloadName, "ULPFEC", 6) || + RtpUtility::StringCompare(payloadName, "RED", 3)) { + videoType = kRtpVideoNone; + } else { + videoType = kRtpVideoGeneric; + } + RtpUtility::Payload* payload = new RtpUtility::Payload; + + payload->name[RTP_PAYLOAD_NAME_SIZE - 1] = 0; + strncpy(payload->name, payloadName, RTP_PAYLOAD_NAME_SIZE - 1); + payload->typeSpecific.Video.videoCodecType = videoType; + payload->audio = false; + return payload; + } + + int GetPayloadTypeFrequency( + const RtpUtility::Payload& payload) const override { + return kVideoPayloadTypeFrequency; + } +}; + +RTPPayloadStrategy* RTPPayloadStrategy::CreateStrategy( + const bool handling_audio) { + if (handling_audio) { + return new RTPPayloadAudioStrategy(); + } else { + return new RTPPayloadVideoStrategy(); } - return -1; } } // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc index f0776fa60c..5940ad8260 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc @@ -14,6 +14,7 @@ #include "webrtc/modules/rtp_rtcp/include/rtp_header_parser.h" #include "webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h" #include "webrtc/modules/rtp_rtcp/source/byte_io.h" +#include "webrtc/modules/rtp_rtcp/source/mock/mock_rtp_payload_strategy.h" #include "webrtc/modules/rtp_rtcp/source/rtp_utility.h" #include "webrtc/test/gmock.h" #include "webrtc/test/gtest.h" @@ -27,72 +28,116 @@ using ::testing::_; static const char* kTypicalPayloadName = "name"; static const size_t kTypicalChannels = 1; -static const uint32_t kTypicalFrequency = 44000; -static const uint32_t kTypicalRate = 32 * 1024; +static const int kTypicalFrequency = 44000; +static const int kTypicalRate = 32 * 1024; static const CodecInst kTypicalAudioCodec = {-1 /* pltype */, "name", kTypicalFrequency, 0 /* pacsize */, kTypicalChannels, kTypicalRate}; -TEST(RtpPayloadRegistryTest, - RegistersAndRemembersVideoPayloadsUntilDeregistered) { - RTPPayloadRegistry rtp_payload_registry; +class RtpPayloadRegistryTest : public ::testing::Test { + public: + void SetUp() { + // Note: the payload registry takes ownership of the strategy. + mock_payload_strategy_ = new testing::NiceMock(); + rtp_payload_registry_.reset(new RTPPayloadRegistry(mock_payload_strategy_)); + } + + protected: + RtpUtility::Payload* ExpectReturnOfTypicalAudioPayload(uint8_t payload_type, + uint32_t rate) { + bool audio = true; + RtpUtility::Payload returned_payload = { + "name", + audio, + {// Initialize the audio struct in this case. + {kTypicalFrequency, kTypicalChannels, rate}}}; + + // Note: we return a new payload since the payload registry takes ownership + // of the created object. + RtpUtility::Payload* returned_payload_on_heap = + new RtpUtility::Payload(returned_payload); + EXPECT_CALL(*mock_payload_strategy_, + CreatePayloadType(StrEq(kTypicalPayloadName), payload_type, + kTypicalFrequency, kTypicalChannels, rate)) + .WillOnce(Return(returned_payload_on_heap)); + return returned_payload_on_heap; + } + + std::unique_ptr rtp_payload_registry_; + testing::NiceMock* mock_payload_strategy_; +}; + +TEST_F(RtpPayloadRegistryTest, + RegistersAndRemembersVideoPayloadsUntilDeregistered) { const uint8_t payload_type = 97; + RtpUtility::Payload returned_video_payload = { + "VP8", false /* audio */, {{kRtpVideoVp8}}}; + // Note: The payload registry takes ownership of this object in + // RegisterReceivePayload. + RtpUtility::Payload* returned_video_payload_on_heap = + new RtpUtility::Payload(returned_video_payload); + EXPECT_CALL( + *mock_payload_strategy_, + CreatePayloadType(StrEq("VP8"), payload_type, kVideoPayloadTypeFrequency, + 0 /* channels */, 0 /* rate */)) + .WillOnce(Return(returned_video_payload_on_heap)); + VideoCodec video_codec; video_codec.codecType = kVideoCodecVP8; strncpy(video_codec.plName, "VP8", RTP_PAYLOAD_NAME_SIZE); video_codec.plType = payload_type; - EXPECT_EQ(0, rtp_payload_registry.RegisterReceivePayload(video_codec)); + EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload(video_codec)); const RtpUtility::Payload* retrieved_payload = - rtp_payload_registry.PayloadTypeToPayload(payload_type); + rtp_payload_registry_->PayloadTypeToPayload(payload_type); EXPECT_TRUE(retrieved_payload); - // We should get back the corresponding payload that we registered. - EXPECT_STREQ("VP8", retrieved_payload->name); - EXPECT_FALSE(retrieved_payload->audio); - EXPECT_EQ(kRtpVideoVp8, retrieved_payload->typeSpecific.Video.videoCodecType); + // We should get back the exact pointer to the payload returned by the + // payload strategy. + EXPECT_EQ(returned_video_payload_on_heap, retrieved_payload); // Now forget about it and verify it's gone. - EXPECT_EQ(0, rtp_payload_registry.DeRegisterReceivePayload(payload_type)); - EXPECT_FALSE(rtp_payload_registry.PayloadTypeToPayload(payload_type)); + EXPECT_EQ(0, rtp_payload_registry_->DeRegisterReceivePayload(payload_type)); + EXPECT_FALSE(rtp_payload_registry_->PayloadTypeToPayload(payload_type)); } -TEST(RtpPayloadRegistryTest, - RegistersAndRemembersAudioPayloadsUntilDeregistered) { - RTPPayloadRegistry rtp_payload_registry; +TEST_F(RtpPayloadRegistryTest, + RegistersAndRemembersAudioPayloadsUntilDeregistered) { uint8_t payload_type = 97; + RtpUtility::Payload* returned_payload_on_heap = + ExpectReturnOfTypicalAudioPayload(payload_type, kTypicalRate); + bool new_payload_created = false; CodecInst audio_codec = kTypicalAudioCodec; audio_codec.pltype = payload_type; - EXPECT_EQ(0, rtp_payload_registry.RegisterReceivePayload( + EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload( audio_codec, &new_payload_created)); EXPECT_TRUE(new_payload_created) << "A new payload WAS created."; const RtpUtility::Payload* retrieved_payload = - rtp_payload_registry.PayloadTypeToPayload(payload_type); + rtp_payload_registry_->PayloadTypeToPayload(payload_type); EXPECT_TRUE(retrieved_payload); - // We should get back the corresponding payload that we registered. - 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_EQ(kTypicalRate, retrieved_payload->typeSpecific.Audio.rate); + // We should get back the exact pointer to the payload returned by the + // payload strategy. + EXPECT_EQ(returned_payload_on_heap, retrieved_payload); // Now forget about it and verify it's gone. - EXPECT_EQ(0, rtp_payload_registry.DeRegisterReceivePayload(payload_type)); - EXPECT_FALSE(rtp_payload_registry.PayloadTypeToPayload(payload_type)); + EXPECT_EQ(0, rtp_payload_registry_->DeRegisterReceivePayload(payload_type)); + EXPECT_FALSE(rtp_payload_registry_->PayloadTypeToPayload(payload_type)); } -TEST(RtpPayloadRegistryTest, AudioRedWorkProperly) { +TEST_F(RtpPayloadRegistryTest, AudioRedWorkProperly) { const uint8_t kRedPayloadType = 127; const int kRedSampleRate = 8000; const size_t kRedChannels = 1; const int kRedBitRate = 0; - RTPPayloadRegistry rtp_payload_registry; + // This creates an audio RTP payload strategy. + rtp_payload_registry_.reset( + new RTPPayloadRegistry(RTPPayloadStrategy::CreateStrategy(true))); bool new_payload_created = false; CodecInst red_audio_codec; @@ -101,170 +146,170 @@ TEST(RtpPayloadRegistryTest, AudioRedWorkProperly) { red_audio_codec.plfreq = kRedSampleRate; red_audio_codec.channels = kRedChannels; red_audio_codec.rate = kRedBitRate; - EXPECT_EQ(0, rtp_payload_registry.RegisterReceivePayload( + EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload( red_audio_codec, &new_payload_created)); EXPECT_TRUE(new_payload_created); - EXPECT_EQ(kRedPayloadType, rtp_payload_registry.red_payload_type()); + EXPECT_EQ(kRedPayloadType, rtp_payload_registry_->red_payload_type()); const RtpUtility::Payload* retrieved_payload = - rtp_payload_registry.PayloadTypeToPayload(kRedPayloadType); + rtp_payload_registry_->PayloadTypeToPayload(kRedPayloadType); EXPECT_TRUE(retrieved_payload); EXPECT_TRUE(retrieved_payload->audio); EXPECT_STRCASEEQ("red", retrieved_payload->name); // Sample rate is correctly registered. EXPECT_EQ(kRedSampleRate, - rtp_payload_registry.GetPayloadTypeFrequency(kRedPayloadType)); + rtp_payload_registry_->GetPayloadTypeFrequency(kRedPayloadType)); } -TEST(RtpPayloadRegistryTest, - DoesNotAcceptSamePayloadTypeTwiceExceptIfPayloadIsCompatible) { +TEST_F(RtpPayloadRegistryTest, + DoesNotAcceptSamePayloadTypeTwiceExceptIfPayloadIsCompatible) { uint8_t payload_type = 97; - RTPPayloadRegistry rtp_payload_registry; bool ignored = false; + RtpUtility::Payload* first_payload_on_heap = + ExpectReturnOfTypicalAudioPayload(payload_type, kTypicalRate); CodecInst audio_codec = kTypicalAudioCodec; audio_codec.pltype = payload_type; - EXPECT_EQ(0, - rtp_payload_registry.RegisterReceivePayload(audio_codec, &ignored)); - - CodecInst audio_codec_2 = kTypicalAudioCodec; - audio_codec_2.pltype = payload_type; - // Make |audio_codec_2| incompatible with |audio_codec| by changing - // the frequency. - audio_codec_2.plfreq = kTypicalFrequency + 1; EXPECT_EQ( - -1, rtp_payload_registry.RegisterReceivePayload(audio_codec_2, &ignored)) - << "Adding incompatible codec with same payload type = bad."; + 0, rtp_payload_registry_->RegisterReceivePayload(audio_codec, &ignored)); - // Change payload type. + EXPECT_EQ( + -1, rtp_payload_registry_->RegisterReceivePayload(audio_codec, &ignored)) + << "Adding same codec twice = bad."; + + RtpUtility::Payload* second_payload_on_heap = + ExpectReturnOfTypicalAudioPayload(payload_type - 1, kTypicalRate); + CodecInst audio_codec_2 = kTypicalAudioCodec; audio_codec_2.pltype = payload_type - 1; EXPECT_EQ( - 0, rtp_payload_registry.RegisterReceivePayload(audio_codec_2, &ignored)) + 0, rtp_payload_registry_->RegisterReceivePayload(audio_codec_2, &ignored)) << "With a different payload type is fine though."; // Ensure both payloads are preserved. const RtpUtility::Payload* retrieved_payload = - rtp_payload_registry.PayloadTypeToPayload(payload_type); + 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_EQ(kTypicalRate, retrieved_payload->typeSpecific.Audio.rate); - + EXPECT_EQ(first_payload_on_heap, retrieved_payload); retrieved_payload = - rtp_payload_registry.PayloadTypeToPayload(payload_type - 1); + rtp_payload_registry_->PayloadTypeToPayload(payload_type - 1); EXPECT_TRUE(retrieved_payload); - EXPECT_STREQ(kTypicalPayloadName, retrieved_payload->name); - EXPECT_TRUE(retrieved_payload->audio); - EXPECT_EQ(kTypicalFrequency + 1, - retrieved_payload->typeSpecific.Audio.frequency); - EXPECT_EQ(kTypicalChannels, retrieved_payload->typeSpecific.Audio.channels); - EXPECT_EQ(kTypicalRate, retrieved_payload->typeSpecific.Audio.rate); + EXPECT_EQ(second_payload_on_heap, retrieved_payload); // 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 // codec twice, and in that case roughly the following happens. - EXPECT_EQ(0, - rtp_payload_registry.RegisterReceivePayload(audio_codec, &ignored)); + ON_CALL(*mock_payload_strategy_, PayloadIsCompatible(_, _, _, _)) + .WillByDefault(Return(true)); + EXPECT_CALL(*mock_payload_strategy_, + UpdatePayloadRate(first_payload_on_heap, kTypicalRate)); + EXPECT_EQ( + 0, rtp_payload_registry_->RegisterReceivePayload(audio_codec, &ignored)); } -TEST(RtpPayloadRegistryTest, - RemovesCompatibleCodecsOnRegistryIfCodecsMustBeUnique) { +TEST_F(RtpPayloadRegistryTest, + RemovesCompatibleCodecsOnRegistryIfCodecsMustBeUnique) { + ON_CALL(*mock_payload_strategy_, PayloadIsCompatible(_, _, _, _)) + .WillByDefault(Return(true)); + ON_CALL(*mock_payload_strategy_, CodecsMustBeUnique()) + .WillByDefault(Return(true)); + uint8_t payload_type = 97; - RTPPayloadRegistry rtp_payload_registry; bool ignored = false; + ExpectReturnOfTypicalAudioPayload(payload_type, kTypicalRate); CodecInst audio_codec = kTypicalAudioCodec; audio_codec.pltype = payload_type; - EXPECT_EQ(0, - rtp_payload_registry.RegisterReceivePayload(audio_codec, &ignored)); + EXPECT_EQ( + 0, rtp_payload_registry_->RegisterReceivePayload(audio_codec, &ignored)); + ExpectReturnOfTypicalAudioPayload(payload_type - 1, kTypicalRate); CodecInst audio_codec_2 = kTypicalAudioCodec; audio_codec_2.pltype = payload_type - 1; - EXPECT_EQ( - 0, rtp_payload_registry.RegisterReceivePayload(audio_codec_2, &ignored)); + EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload(audio_codec_2, + &ignored)); - EXPECT_FALSE(rtp_payload_registry.PayloadTypeToPayload(payload_type)) + EXPECT_FALSE(rtp_payload_registry_->PayloadTypeToPayload(payload_type)) << "The first payload should be " "deregistered because the only thing that differs is payload type."; - EXPECT_TRUE(rtp_payload_registry.PayloadTypeToPayload(payload_type - 1)) + EXPECT_TRUE(rtp_payload_registry_->PayloadTypeToPayload(payload_type - 1)) << "The second payload should still be registered though."; - // Now ensure non-compatible codecs aren't removed. Make |audio_codec_3| - // incompatible by changing the frequency. + // Now ensure non-compatible codecs aren't removed. + ON_CALL(*mock_payload_strategy_, PayloadIsCompatible(_, _, _, _)) + .WillByDefault(Return(false)); + ExpectReturnOfTypicalAudioPayload(payload_type + 1, kTypicalRate); CodecInst audio_codec_3 = kTypicalAudioCodec; audio_codec_3.pltype = payload_type + 1; - audio_codec_3.plfreq = kTypicalFrequency + 1; - EXPECT_EQ( - 0, rtp_payload_registry.RegisterReceivePayload(audio_codec_3, &ignored)); + EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload(audio_codec_3, + &ignored)); - EXPECT_TRUE(rtp_payload_registry.PayloadTypeToPayload(payload_type - 1)) + EXPECT_TRUE(rtp_payload_registry_->PayloadTypeToPayload(payload_type - 1)) << "Not compatible; both payloads should be kept."; - EXPECT_TRUE(rtp_payload_registry.PayloadTypeToPayload(payload_type + 1)) + EXPECT_TRUE(rtp_payload_registry_->PayloadTypeToPayload(payload_type + 1)) << "Not compatible; both payloads should be kept."; } -TEST(RtpPayloadRegistryTest, - LastReceivedCodecTypesAreResetWhenRegisteringNewPayloadTypes) { - RTPPayloadRegistry rtp_payload_registry; - rtp_payload_registry.set_last_received_payload_type(17); - EXPECT_EQ(17, rtp_payload_registry.last_received_payload_type()); +TEST_F(RtpPayloadRegistryTest, + LastReceivedCodecTypesAreResetWhenRegisteringNewPayloadTypes) { + rtp_payload_registry_->set_last_received_payload_type(17); + EXPECT_EQ(17, rtp_payload_registry_->last_received_payload_type()); - bool media_type_unchanged = rtp_payload_registry.ReportMediaPayloadType(18); + bool media_type_unchanged = rtp_payload_registry_->ReportMediaPayloadType(18); EXPECT_FALSE(media_type_unchanged); - media_type_unchanged = rtp_payload_registry.ReportMediaPayloadType(18); + media_type_unchanged = rtp_payload_registry_->ReportMediaPayloadType(18); EXPECT_TRUE(media_type_unchanged); bool ignored; + ExpectReturnOfTypicalAudioPayload(34, kTypicalRate); CodecInst audio_codec = kTypicalAudioCodec; audio_codec.pltype = 34; - EXPECT_EQ(0, - rtp_payload_registry.RegisterReceivePayload(audio_codec, &ignored)); + EXPECT_EQ( + 0, rtp_payload_registry_->RegisterReceivePayload(audio_codec, &ignored)); - EXPECT_EQ(-1, rtp_payload_registry.last_received_payload_type()); - media_type_unchanged = rtp_payload_registry.ReportMediaPayloadType(18); + EXPECT_EQ(-1, rtp_payload_registry_->last_received_payload_type()); + media_type_unchanged = rtp_payload_registry_->ReportMediaPayloadType(18); EXPECT_FALSE(media_type_unchanged); } class ParameterizedRtpPayloadRegistryTest - : public ::testing::TestWithParam {}; + : public RtpPayloadRegistryTest, + public ::testing::WithParamInterface {}; TEST_P(ParameterizedRtpPayloadRegistryTest, FailsToRegisterKnownPayloadsWeAreNotInterestedIn) { - RTPPayloadRegistry rtp_payload_registry; + int payload_type = GetParam(); bool ignored; CodecInst audio_codec; strncpy(audio_codec.plname, "whatever", RTP_PAYLOAD_NAME_SIZE); - audio_codec.pltype = GetParam(); - audio_codec.plfreq = 1900; + audio_codec.pltype = static_cast(payload_type); + audio_codec.plfreq = 19; audio_codec.channels = 1; audio_codec.rate = 17; - EXPECT_EQ(-1, - rtp_payload_registry.RegisterReceivePayload(audio_codec, &ignored)); + EXPECT_EQ( + -1, rtp_payload_registry_->RegisterReceivePayload(audio_codec, &ignored)); } INSTANTIATE_TEST_CASE_P(TestKnownBadPayloadTypes, ParameterizedRtpPayloadRegistryTest, testing::Values(64, 72, 73, 74, 75, 76, 77, 78, 79)); -class RtpPayloadRegistryGenericTest : public ::testing::TestWithParam {}; +class RtpPayloadRegistryGenericTest + : public RtpPayloadRegistryTest, + public ::testing::WithParamInterface {}; TEST_P(RtpPayloadRegistryGenericTest, RegisterGenericReceivePayloadType) { - RTPPayloadRegistry rtp_payload_registry; - bool ignored; CodecInst audio_codec; // Dummy values, except for payload_type. strncpy(audio_codec.plname, "generic-codec", RTP_PAYLOAD_NAME_SIZE); audio_codec.pltype = GetParam(); - audio_codec.plfreq = 1900; + audio_codec.plfreq = 19; audio_codec.channels = 1; audio_codec.rate = 17; - EXPECT_EQ(0, - rtp_payload_registry.RegisterReceivePayload(audio_codec, &ignored)); + EXPECT_EQ( + 0, rtp_payload_registry_->RegisterReceivePayload(audio_codec, &ignored)); } // Generates an RTX packet for the given length and original sequence number. @@ -339,31 +384,29 @@ void TestRtxPacket(RTPPayloadRegistry* rtp_payload_registry, << "The restored packet should have the correct ssrc."; } -TEST(RtpPayloadRegistryTest, MultipleRtxPayloadTypes) { - RTPPayloadRegistry rtp_payload_registry; +TEST_F(RtpPayloadRegistryTest, MultipleRtxPayloadTypes) { // Set the incoming payload type to 90. RTPHeader header; header.payloadType = 90; header.ssrc = 1; - rtp_payload_registry.SetIncomingPayloadType(header); - rtp_payload_registry.SetRtxSsrc(100); + rtp_payload_registry_->SetIncomingPayloadType(header); + rtp_payload_registry_->SetRtxSsrc(100); // Map two RTX payload types. - rtp_payload_registry.SetRtxPayloadType(105, 95); - rtp_payload_registry.SetRtxPayloadType(106, 96); + rtp_payload_registry_->SetRtxPayloadType(105, 95); + rtp_payload_registry_->SetRtxPayloadType(106, 96); - TestRtxPacket(&rtp_payload_registry, 105, 95, true); - TestRtxPacket(&rtp_payload_registry, 106, 96, true); + TestRtxPacket(rtp_payload_registry_.get(), 105, 95, true); + TestRtxPacket(rtp_payload_registry_.get(), 106, 96, true); } -TEST(RtpPayloadRegistryTest, InvalidRtxConfiguration) { - RTPPayloadRegistry rtp_payload_registry; - rtp_payload_registry.SetRtxSsrc(100); +TEST_F(RtpPayloadRegistryTest, InvalidRtxConfiguration) { + rtp_payload_registry_->SetRtxSsrc(100); // Fails because no mappings exist and the incoming payload type isn't known. - TestRtxPacket(&rtp_payload_registry, 105, 0, false); + TestRtxPacket(rtp_payload_registry_.get(), 105, 0, false); // Succeeds when the mapping is used, but fails for the implicit fallback. - rtp_payload_registry.SetRtxPayloadType(105, 95); - TestRtxPacket(&rtp_payload_registry, 105, 95, true); - TestRtxPacket(&rtp_payload_registry, 106, 0, false); + rtp_payload_registry_->SetRtxPayloadType(105, 95); + TestRtxPacket(rtp_payload_registry_.get(), 105, 95, true); + TestRtxPacket(rtp_payload_registry_.get(), 106, 0, false); } INSTANTIATE_TEST_CASE_P(TestDynamicRange, diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h b/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h index 5fbf738b4b..15d0bdeae6 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h @@ -64,6 +64,16 @@ class RTPReceiverAudio : public RTPReceiverStrategy, const char payload_name[RTP_PAYLOAD_NAME_SIZE], const PayloadUnion& specific_payload) const override; + // We do not allow codecs to have multiple payload types for audio, so we + // need to override the default behavior (which is to do nothing). + void PossiblyRemoveExistingPayloadType( + RtpUtility::PayloadTypeMap* payload_type_map, + const char payload_name[RTP_PAYLOAD_NAME_SIZE], + size_t payload_name_length, + uint32_t frequency, + uint8_t channels, + uint32_t rate) const; + // We need to look out for special payload types here and sometimes reset // statistics. In addition we sometimes need to tweak the frequency. void CheckPayloadChanged(int8_t payload_type, @@ -79,6 +89,8 @@ class RTPReceiverAudio : public RTPReceiverStrategy, const AudioPayload& audio_specific, bool is_red); + uint32_t last_received_frequency_; + bool telephone_event_forward_to_decoder_; int8_t telephone_event_payload_type_; std::set telephone_event_reported_; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_utility.h b/webrtc/modules/rtp_rtcp/source/rtp_utility.h index 0e1d6610b9..6cab7cbcf5 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_utility.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_utility.h @@ -36,6 +36,8 @@ struct Payload { PayloadUnion typeSpecific; }; +typedef std::map PayloadTypeMap; + bool StringCompare(const char* str1, const char* str2, const uint32_t length); // Round up to the nearest size that is a multiple of 4. diff --git a/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc b/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc index 2a30043b9f..576557f32e 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc @@ -101,7 +101,8 @@ class RtpRtcpAPITest : public ::testing::Test { configuration.outgoing_transport = &null_transport_; configuration.retransmission_rate_limiter = &retransmission_rate_limiter_; module_.reset(RtpRtcp::CreateRtpRtcp(configuration)); - rtp_payload_registry_.reset(new RTPPayloadRegistry()); + rtp_payload_registry_.reset(new RTPPayloadRegistry( + RTPPayloadStrategy::CreateStrategy(true))); rtp_receiver_.reset(RtpReceiver::CreateAudioReceiver( &fake_clock_, NULL, NULL, rtp_payload_registry_.get())); } diff --git a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_audio.cc b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_audio.cc index a88ffc81da..175771755e 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_audio.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_audio.cc @@ -101,8 +101,10 @@ class RtpRtcpAudioTest : public ::testing::Test { receive_statistics1_.reset(ReceiveStatistics::Create(&fake_clock)); receive_statistics2_.reset(ReceiveStatistics::Create(&fake_clock)); - rtp_payload_registry1_.reset(new RTPPayloadRegistry()); - rtp_payload_registry2_.reset(new RTPPayloadRegistry()); + rtp_payload_registry1_.reset(new RTPPayloadRegistry( + RTPPayloadStrategy::CreateStrategy(true))); + rtp_payload_registry2_.reset(new RTPPayloadRegistry( + RTPPayloadStrategy::CreateStrategy(true))); RtpRtcp::Configuration configuration; configuration.audio = true; diff --git a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_rtcp.cc b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_rtcp.cc index 25ff98d7f2..a4715d2a1f 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_rtcp.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_rtcp.cc @@ -94,8 +94,10 @@ class RtpRtcpRtcpTest : public ::testing::Test { configuration.intra_frame_callback = myRTCPFeedback1; configuration.retransmission_rate_limiter = &retransmission_rate_limiter_; - rtp_payload_registry1_.reset(new RTPPayloadRegistry()); - rtp_payload_registry2_.reset(new RTPPayloadRegistry()); + rtp_payload_registry1_.reset(new RTPPayloadRegistry( + RTPPayloadStrategy::CreateStrategy(true))); + rtp_payload_registry2_.reset(new RTPPayloadRegistry( + RTPPayloadStrategy::CreateStrategy(true))); module1 = RtpRtcp::CreateRtpRtcp(configuration); diff --git a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_video.cc b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_video.cc index 447b4b7ac1..102753782d 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_video.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_video.cc @@ -35,7 +35,8 @@ namespace webrtc { class RtpRtcpVideoTest : public ::testing::Test { protected: RtpRtcpVideoTest() - : test_ssrc_(3456), + : rtp_payload_registry_(RTPPayloadStrategy::CreateStrategy(false)), + test_ssrc_(3456), test_timestamp_(4567), test_sequence_number_(2345), fake_clock(123456), diff --git a/webrtc/modules/video_coding/test/rtp_player.cc b/webrtc/modules/video_coding/test/rtp_player.cc index 42cc61f782..d5fa9ae936 100644 --- a/webrtc/modules/video_coding/test/rtp_player.cc +++ b/webrtc/modules/video_coding/test/rtp_player.cc @@ -271,7 +271,8 @@ class SsrcHandlers { const PayloadTypes& payload_types, LostPackets* lost_packets) : rtp_header_parser_(RtpHeaderParser::Create()), - rtp_payload_registry_(new RTPPayloadRegistry()), + rtp_payload_registry_(new RTPPayloadRegistry( + RTPPayloadStrategy::CreateStrategy(false))), rtp_module_(), payload_sink_(), ssrc_(ssrc), diff --git a/webrtc/video/rtp_stream_receiver.cc b/webrtc/video/rtp_stream_receiver.cc index bc4aa37366..f02cf3fa30 100644 --- a/webrtc/video/rtp_stream_receiver.cc +++ b/webrtc/video/rtp_stream_receiver.cc @@ -106,6 +106,7 @@ RtpStreamReceiver::RtpStreamReceiver( remb_(remb), process_thread_(process_thread), ntp_estimator_(clock_), + rtp_payload_registry_(RTPPayloadStrategy::CreateStrategy(false)), rtp_header_parser_(RtpHeaderParser::Create()), rtp_receiver_(RtpReceiver::CreateVideoReceiver(clock_, this, diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc index b2fade8743..bdf6fb5387 100644 --- a/webrtc/voice_engine/channel.cc +++ b/webrtc/voice_engine/channel.cc @@ -834,7 +834,8 @@ Channel::Channel(int32_t channelId, _channelId(channelId), event_log_proxy_(new RtcEventLogProxy()), rtp_header_parser_(RtpHeaderParser::Create()), - rtp_payload_registry_(new RTPPayloadRegistry()), + rtp_payload_registry_( + new RTPPayloadRegistry(RTPPayloadStrategy::CreateStrategy(true))), rtp_receive_statistics_( ReceiveStatistics::Create(Clock::GetRealTimeClock())), rtp_receiver_(