From f3feeffe03af4b86fde8c30d3a07f82cef57d876 Mon Sep 17 00:00:00 2001 From: magjed Date: Fri, 25 Nov 2016 06:40:25 -0800 Subject: [PATCH] Reland of move RTPPayloadStrategy and simplify RTPPayloadRegistry (patchset #1 id:1 of https://codereview.webrtc.org/2528993002/ ) Reason for revert: Downstream code has been updated. Original issue's description: > 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 > > Committed: https://crrev.com/33c81d05613f45f65ee17224ed381c6cdd1c6c6f > Cr-Commit-Position: refs/heads/master@{#15234} 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/2531043002 Cr-Commit-Position: refs/heads/master@{#15245} --- webrtc/modules/BUILD.gn | 1 - webrtc/modules/rtp_rtcp/BUILD.gn | 1 - .../rtp_rtcp/include/rtp_payload_registry.h | 75 +--- .../modules/rtp_rtcp/include/rtp_receiver.h | 7 - .../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 - .../rtp_rtcp/source/rtp_receiver_impl.cc | 16 - .../rtp_rtcp/source/rtp_receiver_impl.h | 6 - 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 +- 19 files changed, 291 insertions(+), 587 deletions(-) delete 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 1cd97f3b97..bab3b8ee80 100644 --- a/webrtc/modules/BUILD.gn +++ b/webrtc/modules/BUILD.gn @@ -426,7 +426,6 @@ 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 66183a85a1..aa5403dcd8 100644 --- a/webrtc/modules/rtp_rtcp/BUILD.gn +++ b/webrtc/modules/rtp_rtcp/BUILD.gn @@ -34,7 +34,6 @@ 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 8f9d5829f9..668fe876f2 100644 --- a/webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h +++ b/webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h @@ -25,46 +25,24 @@ namespace webrtc { struct CodecInst; class VideoCodec; -// This strategy deals with the audio/video-specific aspects -// of payload handling. +// TODO(magjed): Remove once external code is updated. class RTPPayloadStrategy { public: - 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() {} + static RTPPayloadStrategy* CreateStrategy(bool handling_audio) { + return nullptr; + } }; class RTPPayloadRegistry { public: - // The registry takes ownership of the strategy. - explicit RTPPayloadRegistry(RTPPayloadStrategy* rtp_payload_strategy); + RTPPayloadRegistry(); ~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 remove RTPPayloadStrategy, RTPPayloadVideoStrategy, - // RTPPayloadAudioStrategy, and simplify the code. http://crbug/webrtc/6743. + // 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); @@ -118,13 +96,9 @@ 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 { - rtc::CritScope cs(&crit_sect_); - return red_payload_type_; - } + int8_t red_payload_type() const { return GetPayloadTypeWithName("red"); } int8_t ulpfec_payload_type() const { - rtc::CritScope cs(&crit_sect_); - return ulpfec_payload_type_; + return GetPayloadTypeWithName("ulpfec"); } int8_t last_received_payload_type() const { rtc::CritScope cs(&crit_sect_); @@ -143,34 +117,17 @@ 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 char* payload_name, - size_t payload_name_length, - uint32_t frequency, - size_t channels, - uint32_t rate); + const CodecInst& audio_codec); 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_; - RtpUtility::PayloadTypeMap payload_type_map_; - std::unique_ptr rtp_payload_strategy_; - int8_t red_payload_type_; - int8_t ulpfec_payload_type_; + std::map payload_type_map_; 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/include/rtp_receiver.h b/webrtc/modules/rtp_rtcp/include/rtp_receiver.h index a69300841d..54a99c93a1 100644 --- a/webrtc/modules/rtp_rtcp/include/rtp_receiver.h +++ b/webrtc/modules/rtp_rtcp/include/rtp_receiver.h @@ -61,13 +61,6 @@ class RtpReceiver { virtual int32_t RegisterReceivePayload(const CodecInst& audio_codec) = 0; // Registers a receive payload in the payload registry. virtual int32_t RegisterReceivePayload(const VideoCodec& video_codec) = 0; - // TODO(magjed): Remove once external code is updated. - virtual int32_t RegisterReceivePayload( - const char payload_name[RTP_PAYLOAD_NAME_SIZE], - const int8_t payload_type, - const uint32_t frequency, - const size_t channels, - const uint32_t rate) = 0; // De-registers |payload_type| from the payload registry. virtual int32_t DeRegisterReceivePayload(const int8_t payload_type) = 0; 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 deleted file mode 100644 index 458f5a8945..0000000000 --- a/webrtc/modules/rtp_rtcp/source/mock/mock_rtp_payload_strategy.h +++ /dev/null @@ -1,43 +0,0 @@ -/* - * 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 5b0407c8b2..e52f9d55f7 100644 --- a/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc @@ -169,8 +169,7 @@ class RtxLoopBackTransport : public webrtc::Transport { class RtpRtcpRtxNackTest : public ::testing::Test { protected: RtpRtcpRtxNackTest() - : rtp_payload_registry_(RTPPayloadStrategy::CreateStrategy(false)), - rtp_rtcp_module_(nullptr), + : 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 a4354d393e..7d4afc6e36 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc @@ -10,55 +10,76 @@ #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 { -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) {} +namespace { -RTPPayloadRegistry::~RTPPayloadRegistry() { - while (!payload_type_map_.empty()) { - RtpUtility::PayloadTypeMap::iterator it = payload_type_map_.begin(); - delete it->second; - payload_type_map_.erase(it); +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; } } -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); +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 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) { +bool IsPayloadTypeValid(int8_t payload_type) { assert(payload_type >= 0); - assert(payload_name); - *created_new_payload = false; // Sanity check. switch (payload_type) { @@ -74,59 +95,76 @@ int32_t RTPPayloadRegistry::RegisterReceivePayload( case 79: // 207 Extended report. LOG(LS_ERROR) << "Can't register invalid receiver payload type: " << payload_type; - return -1; + return false; default: - break; + return true; } +} - size_t payload_name_length = strlen(payload_name); +} // 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; rtc::CritScope cs(&crit_sect_); - RtpUtility::PayloadTypeMap::iterator it = - payload_type_map_.find(payload_type); - + auto it = payload_type_map_.find(audio_codec.pltype); if (it != payload_type_map_.end()) { - // 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; - } + // 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; } - LOG(LS_ERROR) << "Payload type already registered: " - << static_cast(payload_type); + LOG(LS_ERROR) << "Payload type already registered: " << audio_codec.pltype; return -1; } - if (rtp_payload_strategy_->CodecsMustBeUnique()) { - DeregisterAudioCodecOrRedTypeRegardlessOfPayloadType( - payload_name, payload_name_length, frequency, channels, rate); - } + // Audio codecs must be unique. + DeregisterAudioCodecOrRedTypeRegardlessOfPayloadType(audio_codec); - RtpUtility::Payload* payload = rtp_payload_strategy_->CreatePayloadType( - payload_name, payload_type, frequency, channels, rate); - - payload_type_map_[payload_type] = payload; + payload_type_map_[audio_codec.pltype] = CreatePayloadType(audio_codec); *created_new_payload = true; - 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; + // 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; } + 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; @@ -137,11 +175,7 @@ int32_t RTPPayloadRegistry::RegisterReceivePayload( int32_t RTPPayloadRegistry::DeRegisterReceivePayload( const int8_t payload_type) { rtc::CritScope cs(&crit_sect_); - RtpUtility::PayloadTypeMap::iterator it = - payload_type_map_.find(payload_type); - assert(it != payload_type_map_.end()); - delete it->second; - payload_type_map_.erase(it); + payload_type_map_.erase(payload_type); return 0; } @@ -149,95 +183,40 @@ int32_t RTPPayloadRegistry::DeRegisterReceivePayload( // for audio codecs, but there can for video. // Always called from within a critical section. void RTPPayloadRegistry::DeregisterAudioCodecOrRedTypeRegardlessOfPayloadType( - 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; - } + 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; } } } int32_t RTPPayloadRegistry::ReceivePayloadType(const CodecInst& audio_codec, int8_t* payload_type) const { - return ReceivePayloadType(audio_codec.plname, audio_codec.plfreq, - audio_codec.channels, std::max(0, audio_codec.rate), - payload_type); + 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; } 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_); - 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; - } + for (const auto& it : payload_type_map_) { + if (PayloadIsCompatible(it.second, video_codec)) { + *payload_type = it.first; + return 0; } } return -1; @@ -333,7 +312,8 @@ void RTPPayloadRegistry::SetRtxPayloadType(int payload_type, bool RTPPayloadRegistry::IsRed(const RTPHeader& header) const { rtc::CritScope cs(&crit_sect_); - return red_payload_type_ == header.payloadType; + auto it = payload_type_map_.find(header.payloadType); + return it != payload_type_map_.end() && _stricmp(it->second.name, "red") == 0; } bool RTPPayloadRegistry::IsEncapsulated(const RTPHeader& header) const { @@ -343,14 +323,13 @@ bool RTPPayloadRegistry::IsEncapsulated(const RTPHeader& header) const { bool RTPPayloadRegistry::GetPayloadSpecifics(uint8_t payload_type, PayloadUnion* payload) const { rtc::CritScope cs(&crit_sect_); - RtpUtility::PayloadTypeMap::const_iterator it = - payload_type_map_.find(payload_type); + 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; + *payload = it->second.typeSpecific; return true; } @@ -361,22 +340,22 @@ int RTPPayloadRegistry::GetPayloadTypeFrequency( return -1; } rtc::CritScope cs(&crit_sect_); - return rtp_payload_strategy_->GetPayloadTypeFrequency(*payload); + return payload->audio ? payload->typeSpecific.Audio.frequency + : kVideoPayloadTypeFrequency; } const RtpUtility::Payload* RTPPayloadRegistry::PayloadTypeToPayload( uint8_t payload_type) const { rtc::CritScope cs(&crit_sect_); - RtpUtility::PayloadTypeMap::const_iterator it = - payload_type_map_.find(payload_type); + 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; + return &it->second; } void RTPPayloadRegistry::SetIncomingPayloadType(const RTPHeader& header) { @@ -395,106 +374,15 @@ bool RTPPayloadRegistry::ReportMediaPayloadType(uint8_t media_payload_type) { return false; } -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(); +// 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; } + 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 5940ad8260..f0776fa60c 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc @@ -14,7 +14,6 @@ #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" @@ -28,116 +27,72 @@ using ::testing::_; static const char* kTypicalPayloadName = "name"; static const size_t kTypicalChannels = 1; -static const int kTypicalFrequency = 44000; -static const int kTypicalRate = 32 * 1024; +static const uint32_t kTypicalFrequency = 44000; +static const uint32_t kTypicalRate = 32 * 1024; static const CodecInst kTypicalAudioCodec = {-1 /* pltype */, "name", kTypicalFrequency, 0 /* pacsize */, kTypicalChannels, kTypicalRate}; -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) { +TEST(RtpPayloadRegistryTest, + RegistersAndRemembersVideoPayloadsUntilDeregistered) { + RTPPayloadRegistry rtp_payload_registry; 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 exact pointer to the payload returned by the - // payload strategy. - EXPECT_EQ(returned_video_payload_on_heap, 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); // 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_F(RtpPayloadRegistryTest, - RegistersAndRemembersAudioPayloadsUntilDeregistered) { +TEST(RtpPayloadRegistryTest, + RegistersAndRemembersAudioPayloadsUntilDeregistered) { + RTPPayloadRegistry rtp_payload_registry; 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 exact pointer to the payload returned by the - // payload strategy. - EXPECT_EQ(returned_payload_on_heap, 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); // 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_F(RtpPayloadRegistryTest, AudioRedWorkProperly) { +TEST(RtpPayloadRegistryTest, AudioRedWorkProperly) { const uint8_t kRedPayloadType = 127; const int kRedSampleRate = 8000; const size_t kRedChannels = 1; const int kRedBitRate = 0; - // This creates an audio RTP payload strategy. - rtp_payload_registry_.reset( - new RTPPayloadRegistry(RTPPayloadStrategy::CreateStrategy(true))); + RTPPayloadRegistry rtp_payload_registry; bool new_payload_created = false; CodecInst red_audio_codec; @@ -146,170 +101,170 @@ TEST_F(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_F(RtpPayloadRegistryTest, - DoesNotAcceptSamePayloadTypeTwiceExceptIfPayloadIsCompatible) { +TEST(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)); + EXPECT_EQ(0, + rtp_payload_registry.RegisterReceivePayload(audio_codec, &ignored)); - 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; + // 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."; + + // Change payload type. 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_EQ(first_payload_on_heap, 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); + retrieved_payload = - rtp_payload_registry_->PayloadTypeToPayload(payload_type - 1); + rtp_payload_registry.PayloadTypeToPayload(payload_type - 1); EXPECT_TRUE(retrieved_payload); - EXPECT_EQ(second_payload_on_heap, 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); // 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. - 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)); + EXPECT_EQ(0, + rtp_payload_registry.RegisterReceivePayload(audio_codec, &ignored)); } -TEST_F(RtpPayloadRegistryTest, - RemovesCompatibleCodecsOnRegistryIfCodecsMustBeUnique) { - ON_CALL(*mock_payload_strategy_, PayloadIsCompatible(_, _, _, _)) - .WillByDefault(Return(true)); - ON_CALL(*mock_payload_strategy_, CodecsMustBeUnique()) - .WillByDefault(Return(true)); - +TEST(RtpPayloadRegistryTest, + RemovesCompatibleCodecsOnRegistryIfCodecsMustBeUnique) { 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)); - ExpectReturnOfTypicalAudioPayload(payload_type - 1, kTypicalRate); + EXPECT_EQ(0, + rtp_payload_registry.RegisterReceivePayload(audio_codec, &ignored)); 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. - ON_CALL(*mock_payload_strategy_, PayloadIsCompatible(_, _, _, _)) - .WillByDefault(Return(false)); - ExpectReturnOfTypicalAudioPayload(payload_type + 1, kTypicalRate); + // Now ensure non-compatible codecs aren't removed. Make |audio_codec_3| + // incompatible by changing the frequency. CodecInst audio_codec_3 = kTypicalAudioCodec; audio_codec_3.pltype = payload_type + 1; - EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload(audio_codec_3, - &ignored)); + audio_codec_3.plfreq = kTypicalFrequency + 1; + 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_F(RtpPayloadRegistryTest, - LastReceivedCodecTypesAreResetWhenRegisteringNewPayloadTypes) { - rtp_payload_registry_->set_last_received_payload_type(17); - EXPECT_EQ(17, rtp_payload_registry_->last_received_payload_type()); +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()); - 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 RtpPayloadRegistryTest, - public ::testing::WithParamInterface {}; + : public ::testing::TestWithParam {}; TEST_P(ParameterizedRtpPayloadRegistryTest, FailsToRegisterKnownPayloadsWeAreNotInterestedIn) { - int payload_type = GetParam(); + RTPPayloadRegistry rtp_payload_registry; bool ignored; CodecInst audio_codec; strncpy(audio_codec.plname, "whatever", RTP_PAYLOAD_NAME_SIZE); - audio_codec.pltype = static_cast(payload_type); - audio_codec.plfreq = 19; + audio_codec.pltype = GetParam(); + audio_codec.plfreq = 1900; 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 RtpPayloadRegistryTest, - public ::testing::WithParamInterface {}; +class RtpPayloadRegistryGenericTest : public ::testing::TestWithParam {}; 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 = 19; + audio_codec.plfreq = 1900; 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. @@ -384,29 +339,31 @@ void TestRtxPacket(RTPPayloadRegistry* rtp_payload_registry, << "The restored packet should have the correct ssrc."; } -TEST_F(RtpPayloadRegistryTest, MultipleRtxPayloadTypes) { +TEST(RtpPayloadRegistryTest, MultipleRtxPayloadTypes) { + RTPPayloadRegistry rtp_payload_registry; // 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_.get(), 105, 95, true); - TestRtxPacket(rtp_payload_registry_.get(), 106, 96, true); + TestRtxPacket(&rtp_payload_registry, 105, 95, true); + TestRtxPacket(&rtp_payload_registry, 106, 96, true); } -TEST_F(RtpPayloadRegistryTest, InvalidRtxConfiguration) { - rtp_payload_registry_->SetRtxSsrc(100); +TEST(RtpPayloadRegistryTest, InvalidRtxConfiguration) { + RTPPayloadRegistry rtp_payload_registry; + rtp_payload_registry.SetRtxSsrc(100); // Fails because no mappings exist and the incoming payload type isn't known. - TestRtxPacket(rtp_payload_registry_.get(), 105, 0, false); + TestRtxPacket(&rtp_payload_registry, 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_.get(), 105, 95, true); - TestRtxPacket(rtp_payload_registry_.get(), 106, 0, false); + rtp_payload_registry.SetRtxPayloadType(105, 95); + TestRtxPacket(&rtp_payload_registry, 105, 95, true); + TestRtxPacket(&rtp_payload_registry, 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 15d0bdeae6..5fbf738b4b 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h @@ -64,16 +64,6 @@ 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, @@ -89,8 +79,6 @@ 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_receiver_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc index 386336544b..79e43ef073 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc @@ -105,22 +105,6 @@ int32_t RtpReceiverImpl::RegisterReceivePayload(const VideoCodec& video_codec) { return rtp_payload_registry_->RegisterReceivePayload(video_codec); } -// TODO(magjed): Remove once external code is updated. -int32_t RtpReceiverImpl::RegisterReceivePayload( - const char payload_name[RTP_PAYLOAD_NAME_SIZE], - const int8_t payload_type, - const uint32_t frequency, - const size_t channels, - const uint32_t rate) { - CodecInst codec; - codec.pltype = payload_type; - strncpy(codec.plname, payload_name, RTP_PAYLOAD_NAME_SIZE); - codec.plfreq = frequency; - codec.channels = channels; - codec.rate = rate; - return RegisterReceivePayload(codec); -} - int32_t RtpReceiverImpl::DeRegisterReceivePayload( const int8_t payload_type) { rtc::CritScope lock(&critical_section_rtp_receiver_); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.h b/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.h index a1d60ade8e..4b5524877c 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.h @@ -35,12 +35,6 @@ class RtpReceiverImpl : public RtpReceiver { int32_t RegisterReceivePayload(const CodecInst& audio_codec) override; int32_t RegisterReceivePayload(const VideoCodec& video_codec) override; - // TODO(magjed): Remove once external code is updated. - int32_t RegisterReceivePayload(const char payload_name[RTP_PAYLOAD_NAME_SIZE], - const int8_t payload_type, - const uint32_t frequency, - const size_t channels, - const uint32_t rate) override; int32_t DeRegisterReceivePayload(const int8_t payload_type) override; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_utility.h b/webrtc/modules/rtp_rtcp/source/rtp_utility.h index 6cab7cbcf5..0e1d6610b9 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_utility.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_utility.h @@ -36,8 +36,6 @@ 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 576557f32e..2a30043b9f 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc @@ -101,8 +101,7 @@ 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( - RTPPayloadStrategy::CreateStrategy(true))); + rtp_payload_registry_.reset(new RTPPayloadRegistry()); 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 175771755e..a88ffc81da 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_audio.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_audio.cc @@ -101,10 +101,8 @@ 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( - RTPPayloadStrategy::CreateStrategy(true))); - rtp_payload_registry2_.reset(new RTPPayloadRegistry( - RTPPayloadStrategy::CreateStrategy(true))); + rtp_payload_registry1_.reset(new RTPPayloadRegistry()); + rtp_payload_registry2_.reset(new RTPPayloadRegistry()); 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 a4715d2a1f..25ff98d7f2 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_rtcp.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_rtcp.cc @@ -94,10 +94,8 @@ class RtpRtcpRtcpTest : public ::testing::Test { configuration.intra_frame_callback = myRTCPFeedback1; configuration.retransmission_rate_limiter = &retransmission_rate_limiter_; - rtp_payload_registry1_.reset(new RTPPayloadRegistry( - RTPPayloadStrategy::CreateStrategy(true))); - rtp_payload_registry2_.reset(new RTPPayloadRegistry( - RTPPayloadStrategy::CreateStrategy(true))); + rtp_payload_registry1_.reset(new RTPPayloadRegistry()); + rtp_payload_registry2_.reset(new RTPPayloadRegistry()); 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 102753782d..447b4b7ac1 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_video.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_video.cc @@ -35,8 +35,7 @@ namespace webrtc { class RtpRtcpVideoTest : public ::testing::Test { protected: RtpRtcpVideoTest() - : rtp_payload_registry_(RTPPayloadStrategy::CreateStrategy(false)), - test_ssrc_(3456), + : 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 d5fa9ae936..42cc61f782 100644 --- a/webrtc/modules/video_coding/test/rtp_player.cc +++ b/webrtc/modules/video_coding/test/rtp_player.cc @@ -271,8 +271,7 @@ class SsrcHandlers { const PayloadTypes& payload_types, LostPackets* lost_packets) : rtp_header_parser_(RtpHeaderParser::Create()), - rtp_payload_registry_(new RTPPayloadRegistry( - RTPPayloadStrategy::CreateStrategy(false))), + rtp_payload_registry_(new RTPPayloadRegistry()), rtp_module_(), payload_sink_(), ssrc_(ssrc), diff --git a/webrtc/video/rtp_stream_receiver.cc b/webrtc/video/rtp_stream_receiver.cc index f02cf3fa30..bc4aa37366 100644 --- a/webrtc/video/rtp_stream_receiver.cc +++ b/webrtc/video/rtp_stream_receiver.cc @@ -106,7 +106,6 @@ 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 bdf6fb5387..b2fade8743 100644 --- a/webrtc/voice_engine/channel.cc +++ b/webrtc/voice_engine/channel.cc @@ -834,8 +834,7 @@ Channel::Channel(int32_t channelId, _channelId(channelId), event_log_proxy_(new RtcEventLogProxy()), rtp_header_parser_(RtpHeaderParser::Create()), - rtp_payload_registry_( - new RTPPayloadRegistry(RTPPayloadStrategy::CreateStrategy(true))), + rtp_payload_registry_(new RTPPayloadRegistry()), rtp_receive_statistics_( ReceiveStatistics::Create(Clock::GetRealTimeClock())), rtp_receiver_(