From 244251a9cd283575b27b0b4ab3beddb069e6fa9d Mon Sep 17 00:00:00 2001 From: "phoglund@webrtc.org" Date: Mon, 4 Feb 2013 13:23:07 +0000 Subject: [PATCH] Moved almost all payload-related stuff to the payload registry. The big benefit is we no longer have a circular dependency between the media receiver and the payload registry. The payload registry is starting to take a bit more place on the stage, and now knows how to do different things depending on audio or video. BUG= TESTED=rtp_rtcp_unittests, vie_auto_test, voe_auto_test Review URL: https://webrtc-codereview.appspot.com/1078004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@3465 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../source/mock/mock_rtp_payload_strategy.h | 41 +++ .../rtp_rtcp/source/rtp_payload_registry.cc | 218 +++++++++++----- .../rtp_rtcp/source/rtp_payload_registry.h | 69 +++-- .../source/rtp_payload_registry_unittest.cc | 236 ++++++++++++++++++ .../modules/rtp_rtcp/source/rtp_receiver.cc | 38 ++- webrtc/modules/rtp_rtcp/source/rtp_receiver.h | 10 - .../rtp_rtcp/source/rtp_receiver_audio.cc | 74 +----- .../rtp_rtcp/source/rtp_receiver_audio.h | 20 +- .../rtp_rtcp/source/rtp_receiver_strategy.h | 39 +-- .../rtp_rtcp/source/rtp_receiver_video.cc | 41 +-- .../rtp_rtcp/source/rtp_receiver_video.h | 16 +- .../modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 8 +- .../rtp_rtcp/source/rtp_rtcp_tests.gypi | 9 +- 13 files changed, 523 insertions(+), 296 deletions(-) create mode 100644 webrtc/modules/rtp_rtcp/source/mock/mock_rtp_payload_strategy.h create mode 100644 webrtc/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc 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 000000000..9acec286f --- /dev/null +++ b/webrtc/modules/rtp_rtcp/source/mock/mock_rtp_payload_strategy.h @@ -0,0 +1,41 @@ +/* + * 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_REGISTRY_H_ +#define WEBRTC_MODULES_RTP_RTCP_SOURCE_MOCK_MOCK_RTP_PAYLOAD_REGISTRY_H_ + +#include "gmock/gmock.h" +#include "webrtc/modules/rtp_rtcp/source/rtp_payload_registry.h" + +namespace webrtc { + +class MockRTPPayloadStrategy : public RTPPayloadStrategy { + public: + MOCK_CONST_METHOD0(CodecsMustBeUnique, + bool()); + MOCK_CONST_METHOD4(PayloadIsCompatible, + bool(const ModuleRTPUtility::Payload& payload, + const WebRtc_UWord32 frequency, + const WebRtc_UWord8 channels, + const WebRtc_UWord32 rate)); + MOCK_CONST_METHOD2(UpdatePayloadRate, + void(ModuleRTPUtility::Payload* payload, const WebRtc_UWord32 rate)); + MOCK_CONST_METHOD5(CreatePayloadType, + ModuleRTPUtility::Payload*( + const char payloadName[RTP_PAYLOAD_NAME_SIZE], + const WebRtc_Word8 payloadType, + const WebRtc_UWord32 frequency, + const WebRtc_UWord8 channels, + const WebRtc_UWord32 rate)); +}; + +} // namespace webrtc + +#endif // WEBRTC_MODULES_RTP_RTCP_SOURCE_MOCK_MOCK_RTP_PAYLOAD_REGISTRY_H_ diff --git a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc index 2275d14a1..40d788c06 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012 The WebRTC project authors. All Rights Reserved. + * Copyright (c) 2013 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 @@ -15,9 +15,10 @@ namespace webrtc { RTPPayloadRegistry::RTPPayloadRegistry( - const WebRtc_Word32 id) + const WebRtc_Word32 id, + RTPPayloadStrategy* rtp_payload_strategy) : id_(id), - rtp_media_receiver_(NULL), + rtp_payload_strategy_(rtp_payload_strategy), red_payload_type_(-1), last_received_payload_type_(-1), last_received_media_payload_type_(-1) { @@ -36,9 +37,10 @@ WebRtc_Word32 RTPPayloadRegistry::RegisterReceivePayload( const WebRtc_Word8 payload_type, const WebRtc_UWord32 frequency, const WebRtc_UWord8 channels, - const WebRtc_UWord32 rate) { - assert(rtp_media_receiver_); + const WebRtc_UWord32 rate, + bool* created_new_payload) { assert(payload_name); + *created_new_payload = false; // Sanity check. switch (payload_type) { @@ -67,6 +69,7 @@ WebRtc_Word32 RTPPayloadRegistry::RegisterReceivePayload( if (it != payload_type_map_.end()) { // We already use this payload type. ModuleRTPUtility::Payload* payload = it->second; + assert(payload); size_t name_length = strlen(payload->name); @@ -76,9 +79,9 @@ WebRtc_Word32 RTPPayloadRegistry::RegisterReceivePayload( if (payload_name_length == name_length && ModuleRTPUtility::StringCompare( payload->name, payload_name, payload_name_length)) { - if (rtp_media_receiver_->PayloadIsCompatible(*payload, frequency, - channels, rate)) { - rtp_media_receiver_->UpdatePayloadRate(payload, rate); + if (rtp_payload_strategy_->PayloadIsCompatible(*payload, frequency, + channels, rate)) { + rtp_payload_strategy_->UpdatePayloadRate(payload, rate); return 0; } } @@ -88,9 +91,10 @@ WebRtc_Word32 RTPPayloadRegistry::RegisterReceivePayload( return -1; } - rtp_media_receiver_->PossiblyRemoveExistingPayloadType( - &payload_type_map_, payload_name, payload_name_length, frequency, channels, - rate); + if (rtp_payload_strategy_->CodecsMustBeUnique()) { + DeregisterAudioCodecOrRedTypeRegardlessOfPayloadType( + payload_name, payload_name_length, frequency, channels, rate); + } ModuleRTPUtility::Payload* payload = NULL; @@ -102,15 +106,10 @@ WebRtc_Word32 RTPPayloadRegistry::RegisterReceivePayload( payload->name[RTP_PAYLOAD_NAME_SIZE - 1] = 0; strncpy(payload->name, payload_name, RTP_PAYLOAD_NAME_SIZE - 1); } else { - payload = rtp_media_receiver_->CreatePayloadType( + *created_new_payload = true; + payload = rtp_payload_strategy_->CreatePayloadType( payload_name, payload_type, frequency, channels, rate); } - if (payload == NULL) { - WEBRTC_TRACE(kTraceError, kTraceRtpRtcp, id_, - "%s failed to register payload", - __FUNCTION__); - return -1; - } payload_type_map_[payload_type] = payload; // Successful set of payload type, clear the value of last received payload @@ -122,7 +121,6 @@ WebRtc_Word32 RTPPayloadRegistry::RegisterReceivePayload( WebRtc_Word32 RTPPayloadRegistry::DeRegisterReceivePayload( const WebRtc_Word8 payload_type) { - assert(rtp_media_receiver_); ModuleRTPUtility::PayloadTypeMap::iterator it = payload_type_map_.find(payload_type); @@ -137,6 +135,42 @@ WebRtc_Word32 RTPPayloadRegistry::DeRegisterReceivePayload( return 0; } +// There can't be several codecs with the same rate, frequency and channels +// for audio codecs, but there can for video. +void RTPPayloadRegistry::DeregisterAudioCodecOrRedTypeRegardlessOfPayloadType( + const char payload_name[RTP_PAYLOAD_NAME_SIZE], + const size_t payload_name_length, + const WebRtc_UWord32 frequency, + const WebRtc_UWord8 channels, + const WebRtc_UWord32 rate) { + ModuleRTPUtility::PayloadTypeMap::iterator iterator = + payload_type_map_.begin(); + for (; iterator != payload_type_map_.end(); ++iterator) { + ModuleRTPUtility::Payload* payload = iterator->second; + size_t name_length = strlen(payload->name); + + if (payload_name_length == name_length + && ModuleRTPUtility::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 (ModuleRTPUtility::StringCompare(payload_name, "red", 3)) { + delete payload; + payload_type_map_.erase(iterator); + break; + } + } + } +} + WebRtc_Word32 RTPPayloadRegistry::ReceivePayloadType( const char payload_name[RTP_PAYLOAD_NAME_SIZE], const WebRtc_UWord32 frequency, @@ -153,7 +187,7 @@ WebRtc_Word32 RTPPayloadRegistry::ReceivePayloadType( ModuleRTPUtility::PayloadTypeMap::const_iterator it = payload_type_map_.begin(); - while (it != payload_type_map_.end()) { + for (; it != payload_type_map_.end(); ++it) { ModuleRTPUtility::Payload* payload = it->second; assert(payload); @@ -186,60 +220,13 @@ WebRtc_Word32 RTPPayloadRegistry::ReceivePayloadType( return 0; } } - it++; } return -1; } -WebRtc_Word32 RTPPayloadRegistry::ReceivePayload( - const WebRtc_Word8 payload_type, - char payload_name[RTP_PAYLOAD_NAME_SIZE], - WebRtc_UWord32* frequency, - WebRtc_UWord8* channels, - WebRtc_UWord32* rate) const { - assert(rtp_media_receiver_); - ModuleRTPUtility::PayloadTypeMap::const_iterator it = - payload_type_map_.find(payload_type); - - if (it == payload_type_map_.end()) { - return -1; - } - ModuleRTPUtility::Payload* payload = it->second; - assert(payload); - - if (frequency) { - if (payload->audio) { - *frequency = payload->typeSpecific.Audio.frequency; - } else { - *frequency = kDefaultVideoFrequency; - } - } - if (channels) { - if (payload->audio) { - *channels = payload->typeSpecific.Audio.channels; - } else { - *channels = 1; - } - } - if (rate) { - if (payload->audio) { - *rate = payload->typeSpecific.Audio.rate; - } else { - assert(false); - *rate = 0; - } - } - if (payload_name) { - payload_name[RTP_PAYLOAD_NAME_SIZE - 1] = 0; - strncpy(payload_name, payload->name, RTP_PAYLOAD_NAME_SIZE - 1); - } - return 0; -} - -WebRtc_UWord32 RTPPayloadRegistry::PayloadTypeToPayload( +WebRtc_Word32 RTPPayloadRegistry::PayloadTypeToPayload( const WebRtc_UWord8 payload_type, ModuleRTPUtility::Payload*& payload) const { - assert(rtp_media_receiver_); ModuleRTPUtility::PayloadTypeMap::const_iterator it = payload_type_map_.find(payload_type); @@ -262,4 +249,99 @@ bool RTPPayloadRegistry::ReportMediaPayloadType( return false; } +class RTPPayloadAudioStrategy : public RTPPayloadStrategy { + public: + bool CodecsMustBeUnique() const { return true; } + + bool PayloadIsCompatible( + const ModuleRTPUtility::Payload& payload, + const WebRtc_UWord32 frequency, + const WebRtc_UWord8 channels, + const WebRtc_UWord32 rate) const { + 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( + ModuleRTPUtility::Payload* payload, + const WebRtc_UWord32 rate) const { + payload->typeSpecific.Audio.rate = rate; + } + + ModuleRTPUtility::Payload* CreatePayloadType( + const char payloadName[RTP_PAYLOAD_NAME_SIZE], + const WebRtc_Word8 payloadType, + const WebRtc_UWord32 frequency, + const WebRtc_UWord8 channels, + const WebRtc_UWord32 rate) const { + ModuleRTPUtility::Payload* payload = new ModuleRTPUtility::Payload; + payload->name[RTP_PAYLOAD_NAME_SIZE - 1] = 0; + strncpy(payload->name, payloadName, RTP_PAYLOAD_NAME_SIZE - 1); + payload->typeSpecific.Audio.frequency = frequency; + payload->typeSpecific.Audio.channels = channels; + payload->typeSpecific.Audio.rate = rate; + payload->audio = true; + return payload; + } +}; + +class RTPPayloadVideoStrategy : public RTPPayloadStrategy { + public: + bool CodecsMustBeUnique() const { return false; } + + bool PayloadIsCompatible( + const ModuleRTPUtility::Payload& payload, + const WebRtc_UWord32 frequency, + const WebRtc_UWord8 channels, + const WebRtc_UWord32 rate) const { + return !payload.audio; + } + + void UpdatePayloadRate( + ModuleRTPUtility::Payload* payload, + const WebRtc_UWord32 rate) const { + payload->typeSpecific.Video.maxRate = rate; + } + + ModuleRTPUtility::Payload* CreatePayloadType( + const char payloadName[RTP_PAYLOAD_NAME_SIZE], + const WebRtc_Word8 payloadType, + const WebRtc_UWord32 frequency, + const WebRtc_UWord8 channels, + const WebRtc_UWord32 rate) const { + RtpVideoCodecTypes videoType = kRtpNoVideo; + if (ModuleRTPUtility::StringCompare(payloadName, "VP8", 3)) { + videoType = kRtpVp8Video; + } else if (ModuleRTPUtility::StringCompare(payloadName, "I420", 4)) { + videoType = kRtpNoVideo; + } else if (ModuleRTPUtility::StringCompare(payloadName, "ULPFEC", 6)) { + videoType = kRtpFecVideo; + } else { + assert(false); + return NULL; + } + ModuleRTPUtility::Payload* payload = new ModuleRTPUtility::Payload; + + payload->name[RTP_PAYLOAD_NAME_SIZE - 1] = 0; + strncpy(payload->name, payloadName, RTP_PAYLOAD_NAME_SIZE - 1); + payload->typeSpecific.Video.videoCodecType = videoType; + payload->typeSpecific.Video.maxRate = rate; + payload->audio = false; + return payload; + } +}; + +RTPPayloadStrategy* RTPPayloadStrategy::CreateStrategy( + const bool handling_audio) { + if (handling_audio) { + return new RTPPayloadAudioStrategy(); + } else { + return new RTPPayloadVideoStrategy(); + } +} + } // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.h b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.h index 888de38bc..7153ba251 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012 The WebRTC project authors. All Rights Reserved. + * Copyright (c) 2013 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 @@ -13,29 +13,55 @@ #include "webrtc/modules/rtp_rtcp/source/rtp_receiver_strategy.h" #include "webrtc/modules/rtp_rtcp/source/rtp_utility.h" +#include "webrtc/system_wrappers/interface/scoped_ptr.h" namespace webrtc { +// This strategy deals with the audio/video-specific aspects +// of payload handling. +class RTPPayloadStrategy { + public: + virtual ~RTPPayloadStrategy() {}; + + virtual bool CodecsMustBeUnique() const = 0; + + virtual bool PayloadIsCompatible( + const ModuleRTPUtility::Payload& payload, + const WebRtc_UWord32 frequency, + const WebRtc_UWord8 channels, + const WebRtc_UWord32 rate) const = 0; + + virtual void UpdatePayloadRate( + ModuleRTPUtility::Payload* payload, + const WebRtc_UWord32 rate) const = 0; + + virtual ModuleRTPUtility::Payload* CreatePayloadType( + const char payloadName[RTP_PAYLOAD_NAME_SIZE], + const WebRtc_Word8 payloadType, + const WebRtc_UWord32 frequency, + const WebRtc_UWord8 channels, + const WebRtc_UWord32 rate) const = 0; + + static RTPPayloadStrategy* CreateStrategy(const bool handling_audio); + + protected: + RTPPayloadStrategy() {}; +}; + class RTPPayloadRegistry { public: - explicit RTPPayloadRegistry(const WebRtc_Word32 id); + // The registry takes ownership of the strategy. + RTPPayloadRegistry(const WebRtc_Word32 id, + RTPPayloadStrategy* rtp_payload_strategy); ~RTPPayloadRegistry(); - // Must be called before any other methods are used! - // TODO(phoglund): We shouldn't really have to talk to a media receiver here. - // It would make more sense to talk to some media-specific payload handling - // strategy. Can't do that right now because audio payload type handling is - // too tightly coupled with packet parsing. - void set_rtp_media_receiver(RTPReceiverStrategy* rtp_media_receiver) { - rtp_media_receiver_ = rtp_media_receiver; - } - WebRtc_Word32 RegisterReceivePayload( const char payload_name[RTP_PAYLOAD_NAME_SIZE], const WebRtc_Word8 payload_type, const WebRtc_UWord32 frequency, const WebRtc_UWord8 channels, - const WebRtc_UWord32 rate); + const WebRtc_UWord32 rate, + bool* created_new_payload_type); WebRtc_Word32 DeRegisterReceivePayload( const WebRtc_Word8 payload_type); @@ -47,14 +73,7 @@ class RTPPayloadRegistry { const WebRtc_UWord32 rate, WebRtc_Word8* payload_type) const; - WebRtc_Word32 ReceivePayload( - const WebRtc_Word8 payload_type, - char payload_name[RTP_PAYLOAD_NAME_SIZE], - WebRtc_UWord32* frequency, - WebRtc_UWord8* channels, - WebRtc_UWord32* rate) const; - - WebRtc_UWord32 PayloadTypeToPayload( + WebRtc_Word32 PayloadTypeToPayload( const WebRtc_UWord8 payload_type, ModuleRTPUtility::Payload*& payload) const; @@ -75,9 +94,17 @@ class RTPPayloadRegistry { } private: + // Prunes the payload type map of the specific payload type, if it exists. + void DeregisterAudioCodecOrRedTypeRegardlessOfPayloadType( + const char payload_name[RTP_PAYLOAD_NAME_SIZE], + const size_t payload_name_length, + const WebRtc_UWord32 frequency, + const WebRtc_UWord8 channels, + const WebRtc_UWord32 rate); + ModuleRTPUtility::PayloadTypeMap payload_type_map_; WebRtc_Word32 id_; - RTPReceiverStrategy* rtp_media_receiver_; + scoped_ptr rtp_payload_strategy_; WebRtc_Word8 red_payload_type_; WebRtc_Word8 last_received_payload_type_; WebRtc_Word8 last_received_media_payload_type_; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc new file mode 100644 index 000000000..d9c0961f9 --- /dev/null +++ b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc @@ -0,0 +1,236 @@ +/* + * Copyright (c) 2013 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "webrtc/modules/rtp_rtcp/source/rtp_payload_registry.h" + +#include "gtest/gtest.h" +#include "gmock/gmock.h" +#include "webrtc/modules/rtp_rtcp/source/mock/mock_rtp_payload_strategy.h" +#include "webrtc/modules/rtp_rtcp/source/rtp_utility.h" +#include "webrtc/system_wrappers/interface/scoped_ptr.h" + +namespace webrtc { + +using ::testing::Eq; +using ::testing::Return; +using ::testing::_; + +static const char* kTypicalPayloadName = "name"; +static const uint8_t kTypicalChannels = 1; +static const int kTypicalFrequency = 44000; +static const int kTypicalRate = 32 * 1024; + +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(123, mock_payload_strategy_)); + } + + protected: + ModuleRTPUtility::Payload* ExpectReturnOfTypicalAudioPayload( + uint8_t payload_type, int rate) { + bool audio = true; + ModuleRTPUtility::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. + ModuleRTPUtility::Payload* returned_payload_on_heap = + new ModuleRTPUtility::Payload(returned_payload); + EXPECT_CALL(*mock_payload_strategy_, + CreatePayloadType(kTypicalPayloadName, payload_type, + kTypicalFrequency, + kTypicalChannels, + rate)).WillOnce(Return(returned_payload_on_heap)); + return returned_payload_on_heap; + } + + scoped_ptr rtp_payload_registry_; + testing::NiceMock* mock_payload_strategy_; +}; + +TEST_F(RtpPayloadRegistryTest, RegistersAndRemembersPayloadsUntilDeregistered) { + uint8_t payload_type = 97; + ModuleRTPUtility::Payload* returned_payload_on_heap = + ExpectReturnOfTypicalAudioPayload(payload_type, kTypicalRate); + + bool new_payload_created = false; + EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload( + kTypicalPayloadName, payload_type, kTypicalFrequency, kTypicalChannels, + kTypicalRate, &new_payload_created)); + + EXPECT_TRUE(new_payload_created) << "A new payload WAS created."; + + ModuleRTPUtility::Payload* retrieved_payload = NULL; + EXPECT_EQ(0, rtp_payload_registry_->PayloadTypeToPayload(payload_type, + 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); + + // Now forget about it and verify it's gone. + EXPECT_EQ(0, rtp_payload_registry_->DeRegisterReceivePayload(payload_type)); + EXPECT_EQ(-1, rtp_payload_registry_->PayloadTypeToPayload( + payload_type, retrieved_payload)); +} + +TEST_F(RtpPayloadRegistryTest, DoesNotCreateNewPayloadTypeIfRed) { + EXPECT_CALL(*mock_payload_strategy_, + CreatePayloadType(_, _, _, _, _)).Times(0); + + bool new_payload_created = false; + uint8_t red_type_of_the_day = 104; + EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload( + "red", red_type_of_the_day, kTypicalFrequency, kTypicalChannels, + kTypicalRate, &new_payload_created)); + ASSERT_FALSE(new_payload_created); + + ASSERT_EQ(red_type_of_the_day, rtp_payload_registry_->red_payload_type()); + + ModuleRTPUtility::Payload* retrieved_payload = NULL; + EXPECT_EQ(0, rtp_payload_registry_->PayloadTypeToPayload(red_type_of_the_day, + retrieved_payload)); + EXPECT_FALSE(retrieved_payload->audio); + EXPECT_STRCASEEQ("red", retrieved_payload->name); +} + +TEST_F(RtpPayloadRegistryTest, + DoesNotAcceptSamePayloadTypeTwiceExceptIfPayloadIsCompatible) { + uint8_t payload_type = 97; + + bool ignored = false; + ModuleRTPUtility::Payload* first_payload_on_heap = + ExpectReturnOfTypicalAudioPayload(payload_type, kTypicalRate); + EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload( + kTypicalPayloadName, payload_type, kTypicalFrequency, kTypicalChannels, + kTypicalRate, &ignored)); + + EXPECT_EQ(-1, rtp_payload_registry_->RegisterReceivePayload( + kTypicalPayloadName, payload_type, kTypicalFrequency, kTypicalChannels, + kTypicalRate, &ignored)) << "Adding same codec twice = bad."; + + ModuleRTPUtility::Payload* second_payload_on_heap = + ExpectReturnOfTypicalAudioPayload(payload_type - 1, kTypicalRate); + EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload( + kTypicalPayloadName, payload_type - 1, kTypicalFrequency, + kTypicalChannels, kTypicalRate, &ignored)) << + "With a different payload type is fine though."; + + // Ensure both payloads are preserved. + ModuleRTPUtility::Payload* retrieved_payload = NULL; + EXPECT_EQ(0, rtp_payload_registry_->PayloadTypeToPayload(payload_type, + retrieved_payload)); + EXPECT_EQ(first_payload_on_heap, retrieved_payload); + EXPECT_EQ(0, rtp_payload_registry_->PayloadTypeToPayload(payload_type - 1, + retrieved_payload)); + 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. + 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( + kTypicalPayloadName, payload_type, kTypicalFrequency, kTypicalChannels, + kTypicalRate, &ignored)); +} + +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; + + bool ignored = false; + ExpectReturnOfTypicalAudioPayload(payload_type, kTypicalRate); + EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload( + kTypicalPayloadName, payload_type, kTypicalFrequency, kTypicalChannels, + kTypicalRate, &ignored)); + ExpectReturnOfTypicalAudioPayload(payload_type - 1, kTypicalRate); + EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload( + kTypicalPayloadName, payload_type - 1, kTypicalFrequency, + kTypicalChannels, kTypicalRate, &ignored)); + + ModuleRTPUtility::Payload* retrieved_payload = NULL; + EXPECT_EQ(-1, rtp_payload_registry_->PayloadTypeToPayload( + payload_type, retrieved_payload)) << "The first payload should be " + "deregistered because the only thing that differs is payload type."; + EXPECT_EQ(0, rtp_payload_registry_->PayloadTypeToPayload( + payload_type - 1, retrieved_payload)) << + "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); + EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload( + kTypicalPayloadName, payload_type + 1, kTypicalFrequency, + kTypicalChannels, kTypicalRate, &ignored)); + + EXPECT_EQ(0, rtp_payload_registry_->PayloadTypeToPayload( + payload_type - 1, retrieved_payload)) << + "Not compatible; both payloads should be kept."; + EXPECT_EQ(0, rtp_payload_registry_->PayloadTypeToPayload( + payload_type + 1, retrieved_payload)) << + "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()); + + bool media_type_unchanged = rtp_payload_registry_->ReportMediaPayloadType(18); + EXPECT_FALSE(media_type_unchanged); + media_type_unchanged = rtp_payload_registry_->ReportMediaPayloadType(18); + EXPECT_TRUE(media_type_unchanged); + + bool ignored; + ExpectReturnOfTypicalAudioPayload(34, kTypicalRate); + EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload( + kTypicalPayloadName, 34, kTypicalFrequency, kTypicalChannels, + kTypicalRate, &ignored)); + + 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 { +}; + +TEST_P(ParameterizedRtpPayloadRegistryTest, + FailsToRegisterKnownPayloadsWeAreNotInterestedIn) { + int payload_type = GetParam(); + + bool ignored; + EXPECT_EQ(-1, rtp_payload_registry_->RegisterReceivePayload( + "whatever", static_cast(payload_type), 19, 1, 17, + &ignored)); +} + +INSTANTIATE_TEST_CASE_P(TestKnownBadPayloadTypes, + ParameterizedRtpPayloadRegistryTest, + testing::Values(64, 72, 73, 74, 75, 76, 77, 78, 79)); + +} // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver.cc b/webrtc/modules/rtp_rtcp/source/rtp_receiver.cc index 0c8265702..851eaeb0e 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver.cc @@ -212,8 +212,24 @@ WebRtc_Word32 RTPReceiver::RegisterReceivePayload( const WebRtc_UWord8 channels, const WebRtc_UWord32 rate) { CriticalSectionScoped lock(critical_section_rtp_receiver_); - return rtp_payload_registry_->RegisterReceivePayload( - payload_name, payload_type, frequency, channels, rate); + + // TODO(phoglund): Try to streamline handling of the RED codec and some other + // cases which makes it necessary to keep track of whether we created a + // payload or not. + bool created_new_payload = false; + WebRtc_Word32 result = rtp_payload_registry_->RegisterReceivePayload( + payload_name, payload_type, frequency, channels, rate, + &created_new_payload); + if (created_new_payload) { + if (rtp_media_receiver_->OnNewPayloadTypeCreated(payload_name, payload_type, + frequency) != 0) { + WEBRTC_TRACE(kTraceError, kTraceRtpRtcp, id_, + "%s failed to register payload", + __FUNCTION__); + return -1; + } + } + return result; } WebRtc_Word32 RTPReceiver::DeRegisterReceivePayload( @@ -233,17 +249,6 @@ WebRtc_Word32 RTPReceiver::ReceivePayloadType( payload_name, frequency, channels, rate, payload_type); } -WebRtc_Word32 RTPReceiver::ReceivePayload( - const WebRtc_Word8 payload_type, - char payload_name[RTP_PAYLOAD_NAME_SIZE], - WebRtc_UWord32* frequency, - WebRtc_UWord8* channels, - WebRtc_UWord32* rate) const { - CriticalSectionScoped lock(critical_section_rtp_receiver_); - return rtp_payload_registry_->ReceivePayload( - payload_type, payload_name, frequency, channels, rate); -} - WebRtc_Word32 RTPReceiver::RegisterRtpHeaderExtension( const RTPExtensionType type, const WebRtc_UWord8 id) { @@ -617,13 +622,6 @@ int32_t RTPReceiver::LastReceivedTimeMs() const { return last_received_frame_time_ms_; } -WebRtc_UWord32 RTPReceiver::PayloadTypeToPayload( - const WebRtc_UWord8 payload_type, - Payload*& payload) const { - CriticalSectionScoped lock(critical_section_rtp_receiver_); - return rtp_payload_registry_->PayloadTypeToPayload(payload_type, payload); -} - // Compute time stamp of the last incoming packet that is the first packet of // its frame. WebRtc_Word32 RTPReceiver::EstimatedRemoteTimeStamp( diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver.h b/webrtc/modules/rtp_rtcp/source/rtp_receiver.h index 854c55631..d85929723 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver.h @@ -74,12 +74,6 @@ class RTPReceiver : public Bitrate { const WebRtc_UWord32 rate, WebRtc_Word8* payload_type) const; - WebRtc_Word32 ReceivePayload(const WebRtc_Word8 payload_type, - char payload_name[RTP_PAYLOAD_NAME_SIZE], - WebRtc_UWord32* frequency, - WebRtc_UWord8* channels, - WebRtc_UWord32* rate) const; - WebRtc_Word32 IncomingRTPPacket( WebRtcRTPHeader* rtpheader, const WebRtc_UWord8* incoming_rtp_packet, @@ -148,10 +142,6 @@ class RTPReceiver : public Bitrate { void GetHeaderExtensionMapCopy(RtpHeaderExtensionMap* map) const; - virtual WebRtc_UWord32 PayloadTypeToPayload( - const WebRtc_UWord8 payload_type, - ModuleRTPUtility::Payload*& payload) const; - // RTX. void SetRTXStatus(const bool enable, const WebRtc_UWord32 ssrc); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc b/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc index 595dc95bb..0f91cb80a 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc @@ -188,12 +188,10 @@ bool RTPReceiverAudio::ShouldReportCsrcChanges( G7221 frame N/A */ -ModuleRTPUtility::Payload* RTPReceiverAudio::CreatePayloadType( +WebRtc_Word32 RTPReceiverAudio::OnNewPayloadTypeCreated( const char payloadName[RTP_PAYLOAD_NAME_SIZE], const WebRtc_Word8 payloadType, - const WebRtc_UWord32 frequency, - const WebRtc_UWord8 channels, - const WebRtc_UWord32 rate) { + const WebRtc_UWord32 frequency) { CriticalSectionScoped lock(_criticalSectionRtpReceiverAudio.get()); if (ModuleRTPUtility::StringCompare(payloadName, "telephone-event", 15)) { @@ -211,18 +209,10 @@ ModuleRTPUtility::Payload* RTPReceiverAudio::CreatePayloadType( _cngFBPayloadType = payloadType; } else { assert(false); - return NULL; + return -1; } } - - ModuleRTPUtility::Payload* payload = new ModuleRTPUtility::Payload; - payload->name[RTP_PAYLOAD_NAME_SIZE - 1] = 0; - strncpy(payload->name, payloadName, RTP_PAYLOAD_NAME_SIZE - 1); - payload->typeSpecific.Audio.frequency = frequency; - payload->typeSpecific.Audio.channels = channels; - payload->typeSpecific.Audio.rate = rate; - payload->audio = true; - return payload; + return 0; } void RTPReceiverAudio::SendTelephoneEvents( @@ -290,62 +280,6 @@ RTPAliveType RTPReceiverAudio::ProcessDeadOrAlive( } } -bool RTPReceiverAudio::PayloadIsCompatible( - const ModuleRTPUtility::Payload& payload, - const WebRtc_UWord32 frequency, - const WebRtc_UWord8 channels, - const WebRtc_UWord32 rate) const { - 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 RTPReceiverAudio::UpdatePayloadRate( - ModuleRTPUtility::Payload* payload, - const WebRtc_UWord32 rate) const { - payload->typeSpecific.Audio.rate = rate; -} - -void RTPReceiverAudio::PossiblyRemoveExistingPayloadType( - ModuleRTPUtility::PayloadTypeMap* payloadTypeMap, - const char payloadName[RTP_PAYLOAD_NAME_SIZE], - const size_t payloadNameLength, - const WebRtc_UWord32 frequency, - const WebRtc_UWord8 channels, - const WebRtc_UWord32 rate) const { - ModuleRTPUtility::PayloadTypeMap::iterator audio_it = payloadTypeMap->begin(); - while (audio_it != payloadTypeMap->end()) { - ModuleRTPUtility::Payload* payload = audio_it->second; - size_t nameLength = strlen(payload->name); - - if (payloadNameLength == nameLength && - ModuleRTPUtility::StringCompare(payload->name, - payloadName, payloadNameLength)) { - // we found the payload name in the list - // if audio check frequency and rate - if (payload->audio) { - if (payload->typeSpecific.Audio.frequency == frequency && - (payload->typeSpecific.Audio.rate == rate || - payload->typeSpecific.Audio.rate == 0 || rate == 0) && - payload->typeSpecific.Audio.channels == channels) { - // remove old setting - delete payload; - payloadTypeMap->erase(audio_it); - break; - } - } else if(ModuleRTPUtility::StringCompare(payloadName,"red",3)) { - delete payload; - payloadTypeMap->erase(audio_it); - break; - } - } - audio_it++; - } -} - void RTPReceiverAudio::CheckPayloadChanged( const WebRtc_Word8 payloadType, ModuleRTPUtility::PayloadUnion* specificPayload, diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h b/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h index c66778c7a..64d77f8c7 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h @@ -66,24 +66,12 @@ public: RTPAliveType ProcessDeadOrAlive(WebRtc_UWord16 lastPayloadLength) const; - bool PayloadIsCompatible( - const ModuleRTPUtility::Payload& payload, - const WebRtc_UWord32 frequency, - const WebRtc_UWord8 channels, - const WebRtc_UWord32 rate) const; - - void UpdatePayloadRate( - ModuleRTPUtility::Payload* payload, - const WebRtc_UWord32 rate) const; - bool ShouldReportCsrcChanges(WebRtc_UWord8 payload_type) const; - ModuleRTPUtility::Payload* CreatePayloadType( - const char payloadName[RTP_PAYLOAD_NAME_SIZE], - const WebRtc_Word8 payloadType, - const WebRtc_UWord32 frequency, - const WebRtc_UWord8 channels, - const WebRtc_UWord32 rate); + WebRtc_Word32 OnNewPayloadTypeCreated( + const char payloadName[RTP_PAYLOAD_NAME_SIZE], + const WebRtc_Word8 payloadType, + const WebRtc_UWord32 frequency); WebRtc_Word32 InvokeOnInitializeDecoder( RtpFeedback* callback, diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_strategy.h b/webrtc/modules/rtp_rtcp/source/rtp_receiver_strategy.h index 812bb4caa..19b64888e 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_strategy.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_strategy.h @@ -55,30 +55,16 @@ class RTPReceiverStrategy { virtual RTPAliveType ProcessDeadOrAlive( WebRtc_UWord16 last_payload_length) const = 0; - // Checks if the provided payload can be handled by this strategy and if - // it is compatible with the provided parameters. - virtual bool PayloadIsCompatible( - const ModuleRTPUtility::Payload& payload, - const WebRtc_UWord32 frequency, - const WebRtc_UWord8 channels, - const WebRtc_UWord32 rate) const = 0; - - // Updates the rate in the payload in a media-specific way. - virtual void UpdatePayloadRate( - ModuleRTPUtility::Payload* payload, - const WebRtc_UWord32 rate) const = 0; - // Returns true if we should report CSRC changes for this payload type. // TODO(phoglund): should move out of here along with other payload stuff. virtual bool ShouldReportCsrcChanges(WebRtc_UWord8 payload_type) const = 0; - // Creates a media-specific payload instance from the provided parameters. - virtual ModuleRTPUtility::Payload* CreatePayloadType( - const char payload_name[RTP_PAYLOAD_NAME_SIZE], - const WebRtc_Word8 payload_type, - const WebRtc_UWord32 frequency, - const WebRtc_UWord8 channels, - const WebRtc_UWord32 rate) = 0; + // Notifies the strategy that we have created a new non-RED payload type in + // the payload registry. + virtual WebRtc_Word32 OnNewPayloadTypeCreated( + const char payloadName[RTP_PAYLOAD_NAME_SIZE], + const WebRtc_Word8 payloadType, + const WebRtc_UWord32 frequency) = 0; // Invokes the OnInitializeDecoder callback in a media-specific way. virtual WebRtc_Word32 InvokeOnInitializeDecoder( @@ -88,19 +74,6 @@ class RTPReceiverStrategy { const char payload_name[RTP_PAYLOAD_NAME_SIZE], const ModuleRTPUtility::PayloadUnion& specific_payload) const = 0; - // Prunes the payload type map of the specific payload type, if it exists. - // TODO(phoglund): Move this responsibility into some payload management - // class along with rtp_receiver's payload management. - virtual void PossiblyRemoveExistingPayloadType( - ModuleRTPUtility::PayloadTypeMap* payload_type_map, - const char payload_name[RTP_PAYLOAD_NAME_SIZE], - const size_t payload_name_length, - const WebRtc_UWord32 frequency, - const WebRtc_UWord8 channels, - const WebRtc_UWord32 rate) const { - // Default: do nothing. - } - // Checks if the payload type has changed, and returns whether we should // reset statistics and/or discard this packet. virtual void CheckPayloadChanged( diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc b/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc index 92942fc4c..9af8860fc 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc @@ -51,35 +51,18 @@ bool RTPReceiverVideo::ShouldReportCsrcChanges( return true; } -ModuleRTPUtility::Payload* RTPReceiverVideo::CreatePayloadType( +WebRtc_Word32 RTPReceiverVideo::OnNewPayloadTypeCreated( const char payloadName[RTP_PAYLOAD_NAME_SIZE], const WebRtc_Word8 payloadType, - const WebRtc_UWord32 frequency, - const WebRtc_UWord8 channels, - const WebRtc_UWord32 rate) { - RtpVideoCodecTypes videoType = kRtpNoVideo; - if (ModuleRTPUtility::StringCompare(payloadName, "VP8", 3)) { - videoType = kRtpVp8Video; - } else if (ModuleRTPUtility::StringCompare(payloadName, "I420", 4)) { - videoType = kRtpNoVideo; - } else if (ModuleRTPUtility::StringCompare(payloadName, "ULPFEC", 6)) { - // store this + const WebRtc_UWord32 frequency) { + if (ModuleRTPUtility::StringCompare(payloadName, "ULPFEC", 6)) { + // Enable FEC if not enabled. if (_receiveFEC == NULL) { _receiveFEC = new ReceiverFEC(_id, this); } _receiveFEC->SetPayloadTypeFEC(payloadType); - videoType = kRtpFecVideo; - } else { - return NULL; } - ModuleRTPUtility::Payload* payload = new ModuleRTPUtility::Payload; - - payload->name[RTP_PAYLOAD_NAME_SIZE - 1] = 0; - strncpy(payload->name, payloadName, RTP_PAYLOAD_NAME_SIZE - 1); - payload->typeSpecific.Video.videoCodecType = videoType; - payload->typeSpecific.Video.maxRate = rate; - payload->audio = false; - return payload; + return 0; } WebRtc_Word32 RTPReceiverVideo::ParseRtpPacket( @@ -109,20 +92,6 @@ RTPAliveType RTPReceiverVideo::ProcessDeadOrAlive( return kRtpDead; } -bool RTPReceiverVideo::PayloadIsCompatible( - const ModuleRTPUtility::Payload& payload, - const WebRtc_UWord32 frequency, - const WebRtc_UWord8 channels, - const WebRtc_UWord32 rate) const { - return !payload.audio; -} - -void RTPReceiverVideo::UpdatePayloadRate( - ModuleRTPUtility::Payload* payload, - const WebRtc_UWord32 rate) const { - payload->typeSpecific.Video.maxRate = rate; -} - WebRtc_Word32 RTPReceiverVideo::InvokeOnInitializeDecoder( RtpFeedback* callback, const WebRtc_Word32 id, diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.h b/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.h index 4657757da..38ba186c8 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.h @@ -46,24 +46,12 @@ class RTPReceiverVideo : public RTPReceiverStrategy { RTPAliveType ProcessDeadOrAlive(WebRtc_UWord16 lastPayloadLength) const; - bool PayloadIsCompatible( - const ModuleRTPUtility::Payload& payload, - const WebRtc_UWord32 frequency, - const WebRtc_UWord8 channels, - const WebRtc_UWord32 rate) const; - - void UpdatePayloadRate( - ModuleRTPUtility::Payload* payload, - const WebRtc_UWord32 rate) const; - bool ShouldReportCsrcChanges(WebRtc_UWord8 payload_type) const; - ModuleRTPUtility::Payload* CreatePayloadType( + WebRtc_Word32 OnNewPayloadTypeCreated( const char payloadName[RTP_PAYLOAD_NAME_SIZE], const WebRtc_Word8 payloadType, - const WebRtc_UWord32 frequency, - const WebRtc_UWord8 channels, - const WebRtc_UWord32 rate); + const WebRtc_UWord32 frequency); WebRtc_Word32 InvokeOnInitializeDecoder( RtpFeedback* callback, diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 940836196..c9a72e112 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -85,7 +85,9 @@ RtpRtcp* RtpRtcp::CreateRtpRtcp(const RtpRtcp::Configuration& configuration) { } ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) - : rtp_payload_registry_(configuration.id), + : rtp_payload_registry_( + configuration.id, + RTPPayloadStrategy::CreateStrategy(configuration.audio)), rtp_sender_(configuration.id, configuration.audio, configuration.clock, @@ -138,10 +140,6 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) new RTPReceiverVideo(configuration.id, &rtp_payload_registry_, configuration.incoming_data); } - // TODO(phoglund): Get rid of this silly circular dependency between the - // payload manager and the video RTP receiver. - rtp_payload_registry_.set_rtp_media_receiver(rtp_receiver_strategy); - rtp_receiver_.reset(new RTPReceiver( configuration.id, configuration.clock, this, configuration.audio_messages, configuration.incoming_data, diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_tests.gypi b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_tests.gypi index 5bfb77acc..b73a0521c 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_tests.gypi +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_tests.gypi @@ -28,21 +28,24 @@ '../test/testAPI/test_api_nack.cc', '../test/testAPI/test_api_rtcp.cc', '../test/testAPI/test_api_video.cc', + 'mock/mock_rtp_payload_strategy.h', + 'mock/mock_rtp_receiver_video.h', 'fec_test_helper.cc', 'fec_test_helper.h', 'producer_fec_unittest.cc', 'receiver_fec_unittest.cc', + 'rtcp_format_remb_unittest.cc', + 'rtcp_sender_unittest.cc', + 'rtcp_receiver_unittest.cc', 'rtp_fec_unittest.cc', 'rtp_format_vp8_unittest.cc', 'rtp_format_vp8_test_helper.cc', 'rtp_format_vp8_test_helper.h', - 'rtcp_format_remb_unittest.cc', 'rtp_packet_history_unittest.cc', + 'rtp_payload_registry_unittest.cc', 'rtp_utility_unittest.cc', 'rtp_header_extension_unittest.cc', 'rtp_sender_unittest.cc', - 'rtcp_sender_unittest.cc', - 'rtcp_receiver_unittest.cc', 'vp8_partition_aggregator_unittest.cc', ], # Disable warnings to enable Win64 build, issue 1323.