From 56124bd158f371e39aeae3dcff176cbbad75dbcd Mon Sep 17 00:00:00 2001 From: magjed Date: Thu, 24 Nov 2016 09:34:46 -0800 Subject: [PATCH] Send audio and video codecs to RTPPayloadRegistry The purpose with this CL is to be able to send video codec specific information down to RTPPayloadRegistry. We already do this for audio with explicit arguments for e.g. number of channels. Instead of extracting the arguments from webrtc::CodecInst (audio) and webrtc::VideoCodec, this CL sends the types unmodified all the way down to RTPPayloadRegistry. This CL does not contain any functional changes, and is just a preparation for future CL:s. In the dependent CL https://codereview.webrtc.org/2524923002/, RTPPayloadStrategy is removed. RTPPayloadStrategy previously handled audio/video specific aspects of payload handling. After this CL, we will know if we get audio or video codecs without any dependency injection, since we have different functions with different signatures for audio vs video. BUG=webrtc:6743 TBR=mflodman Review-Url: https://codereview.webrtc.org/2523843002 Cr-Commit-Position: refs/heads/master@{#15231} --- .../rtp_rtcp/include/rtp_payload_registry.h | 33 +++-- .../modules/rtp_rtcp/include/rtp_receiver.h | 4 + .../rtp_rtcp/source/nack_rtx_unittest.cc | 4 +- .../rtp_rtcp/source/rtp_payload_registry.cc | 29 ++++ .../source/rtp_payload_registry_unittest.cc | 137 +++++++++++++----- .../rtp_rtcp/source/rtp_receiver_audio.cc | 27 ++-- .../rtp_rtcp/source/rtp_receiver_audio.h | 5 +- .../rtp_rtcp/source/rtp_receiver_impl.cc | 34 +++-- .../rtp_rtcp/source/rtp_receiver_impl.h | 2 + .../rtp_rtcp/source/rtp_receiver_strategy.h | 11 +- .../rtp_rtcp/source/rtp_receiver_video.cc | 5 +- .../rtp_rtcp/source/rtp_receiver_video.h | 5 +- .../rtp_rtcp/test/testAPI/test_api_audio.cc | 21 +-- .../rtp_rtcp/test/testAPI/test_api_rtcp.cc | 14 +- .../rtp_rtcp/test/testAPI/test_api_video.cc | 12 +- webrtc/video/rtp_stream_receiver.cc | 9 +- webrtc/voice_engine/channel.cc | 24 +-- 17 files changed, 218 insertions(+), 158 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h b/webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h index fd228e1f16..8f9d5829f9 100644 --- a/webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h +++ b/webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h @@ -22,6 +22,9 @@ namespace webrtc { +struct CodecInst; +class VideoCodec; + // This strategy deals with the audio/video-specific aspects // of payload handling. class RTPPayloadStrategy { @@ -59,19 +62,18 @@ class RTPPayloadRegistry { explicit RTPPayloadRegistry(RTPPayloadStrategy* rtp_payload_strategy); ~RTPPayloadRegistry(); - int32_t RegisterReceivePayload(const char* payload_name, - int8_t payload_type, - uint32_t frequency, - size_t channels, - uint32_t rate, + // TODO(magjed): Split RTPPayloadRegistry into separate Audio and Video class + // and remove RTPPayloadStrategy, RTPPayloadVideoStrategy, + // RTPPayloadAudioStrategy, and simplify the code. http://crbug/webrtc/6743. + int32_t RegisterReceivePayload(const CodecInst& audio_codec, bool* created_new_payload_type); + int32_t RegisterReceivePayload(const VideoCodec& video_codec); int32_t DeRegisterReceivePayload(int8_t payload_type); - int32_t ReceivePayloadType(const char* payload_name, - uint32_t frequency, - size_t channels, - uint32_t rate, + int32_t ReceivePayloadType(const CodecInst& audio_codec, + int8_t* payload_type) const; + int32_t ReceivePayloadType(const VideoCodec& video_codec, int8_t* payload_type) const; bool RtxEnabled() const; @@ -141,6 +143,19 @@ 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, diff --git a/webrtc/modules/rtp_rtcp/include/rtp_receiver.h b/webrtc/modules/rtp_rtcp/include/rtp_receiver.h index 9db1c63da7..799d6b64c3 100644 --- a/webrtc/modules/rtp_rtcp/include/rtp_receiver.h +++ b/webrtc/modules/rtp_rtcp/include/rtp_receiver.h @@ -16,7 +16,9 @@ namespace webrtc { +struct CodecInst; class RTPPayloadRegistry; +class VideoCodec; class TelephoneEventHandler { public: @@ -56,6 +58,8 @@ class RtpReceiver { // Registers a receive payload in the payload registry and notifies the media // receiver strategy. + virtual int32_t RegisterReceivePayload(const CodecInst& audio_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, diff --git a/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc b/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc index 2a0fce5a06..5b0407c8b2 100644 --- a/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc @@ -210,9 +210,7 @@ class RtpRtcpRtxNackTest : public ::testing::Test { EXPECT_EQ(0, rtp_rtcp_module_->RegisterSendPayload(video_codec)); rtp_rtcp_module_->SetRtxSendPayloadType(kRtxPayloadType, kPayloadType); - EXPECT_EQ(0, rtp_receiver_->RegisterReceivePayload( - video_codec.plName, video_codec.plType, 90000, 0, - video_codec.maxBitrate)); + EXPECT_EQ(0, rtp_payload_registry_.RegisterReceivePayload(video_codec)); rtp_payload_registry_.SetRtxPayloadType(kRtxPayloadType, kPayloadType); for (size_t n = 0; n < payload_data_length; n++) { diff --git a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc index 4188f97b53..a4354d393e 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc @@ -11,6 +11,7 @@ #include "webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h" #include "webrtc/base/logging.h" +#include "webrtc/common_types.h" #include "webrtc/modules/rtp_rtcp/source/byte_io.h" namespace webrtc { @@ -33,6 +34,21 @@ RTPPayloadRegistry::~RTPPayloadRegistry() { } } +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); +} + +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, @@ -165,6 +181,19 @@ void RTPPayloadRegistry::DeregisterAudioCodecOrRedTypeRegardlessOfPayloadType( } } +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); +} + +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, 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 abb2ae91c4..5940ad8260 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc @@ -10,6 +10,7 @@ #include +#include "webrtc/common_types.h" #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" @@ -22,12 +23,16 @@ namespace webrtc { using ::testing::Eq; using ::testing::Return; +using ::testing::StrEq; 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 CodecInst kTypicalAudioCodec = {-1 /* pltype */, "name", + kTypicalFrequency, 0 /* pacsize */, + kTypicalChannels, kTypicalRate}; class RtpPayloadRegistryTest : public ::testing::Test { public: @@ -52,7 +57,7 @@ class RtpPayloadRegistryTest : public ::testing::Test { RtpUtility::Payload* returned_payload_on_heap = new RtpUtility::Payload(returned_payload); EXPECT_CALL(*mock_payload_strategy_, - CreatePayloadType(kTypicalPayloadName, payload_type, + CreatePayloadType(StrEq(kTypicalPayloadName), payload_type, kTypicalFrequency, kTypicalChannels, rate)) .WillOnce(Return(returned_payload_on_heap)); return returned_payload_on_heap; @@ -62,15 +67,52 @@ class RtpPayloadRegistryTest : public ::testing::Test { testing::NiceMock* mock_payload_strategy_; }; -TEST_F(RtpPayloadRegistryTest, RegistersAndRemembersPayloadsUntilDeregistered) { +TEST_F(RtpPayloadRegistryTest, + RegistersAndRemembersVideoPayloadsUntilDeregistered) { + const uint8_t payload_type = 97; + RtpUtility::Payload returned_video_payload = { + "VP8", false /* audio */, {{kRtpVideoVp8}}}; + // Note: The payload registry takes ownership of this object in + // RegisterReceivePayload. + RtpUtility::Payload* returned_video_payload_on_heap = + new RtpUtility::Payload(returned_video_payload); + EXPECT_CALL( + *mock_payload_strategy_, + CreatePayloadType(StrEq("VP8"), payload_type, kVideoPayloadTypeFrequency, + 0 /* channels */, 0 /* rate */)) + .WillOnce(Return(returned_video_payload_on_heap)); + + VideoCodec video_codec; + video_codec.codecType = kVideoCodecVP8; + strncpy(video_codec.plName, "VP8", RTP_PAYLOAD_NAME_SIZE); + video_codec.plType = payload_type; + + EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload(video_codec)); + + const RtpUtility::Payload* retrieved_payload = + 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); + + // 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)); +} + +TEST_F(RtpPayloadRegistryTest, + RegistersAndRemembersAudioPayloadsUntilDeregistered) { uint8_t payload_type = 97; RtpUtility::Payload* returned_payload_on_heap = ExpectReturnOfTypicalAudioPayload(payload_type, kTypicalRate); bool new_payload_created = false; + CodecInst audio_codec = kTypicalAudioCodec; + audio_codec.pltype = payload_type; EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload( - kTypicalPayloadName, payload_type, kTypicalFrequency, - kTypicalChannels, kTypicalRate, &new_payload_created)); + audio_codec, &new_payload_created)); EXPECT_TRUE(new_payload_created) << "A new payload WAS created."; @@ -98,9 +140,14 @@ TEST_F(RtpPayloadRegistryTest, AudioRedWorkProperly) { new RTPPayloadRegistry(RTPPayloadStrategy::CreateStrategy(true))); bool new_payload_created = false; + CodecInst red_audio_codec; + strncpy(red_audio_codec.plname, "red", RTP_PAYLOAD_NAME_SIZE); + red_audio_codec.pltype = kRedPayloadType; + red_audio_codec.plfreq = kRedSampleRate; + red_audio_codec.channels = kRedChannels; + red_audio_codec.rate = kRedBitRate; EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload( - "red", kRedPayloadType, kRedSampleRate, kRedChannels, - kRedBitRate, &new_payload_created)); + red_audio_codec, &new_payload_created)); EXPECT_TRUE(new_payload_created); EXPECT_EQ(kRedPayloadType, rtp_payload_registry_->red_payload_type()); @@ -123,20 +170,21 @@ TEST_F(RtpPayloadRegistryTest, bool ignored = false; RtpUtility::Payload* first_payload_on_heap = ExpectReturnOfTypicalAudioPayload(payload_type, kTypicalRate); - EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload( - kTypicalPayloadName, payload_type, kTypicalFrequency, - kTypicalChannels, kTypicalRate, &ignored)); + CodecInst audio_codec = kTypicalAudioCodec; + audio_codec.pltype = payload_type; + EXPECT_EQ( + 0, rtp_payload_registry_->RegisterReceivePayload(audio_codec, &ignored)); - EXPECT_EQ(-1, rtp_payload_registry_->RegisterReceivePayload( - kTypicalPayloadName, payload_type, kTypicalFrequency, - kTypicalChannels, kTypicalRate, &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); - EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload( - kTypicalPayloadName, payload_type - 1, kTypicalFrequency, - kTypicalChannels, kTypicalRate, &ignored)) + CodecInst audio_codec_2 = kTypicalAudioCodec; + audio_codec_2.pltype = payload_type - 1; + EXPECT_EQ( + 0, rtp_payload_registry_->RegisterReceivePayload(audio_codec_2, &ignored)) << "With a different payload type is fine though."; // Ensure both payloads are preserved. @@ -156,9 +204,8 @@ TEST_F(RtpPayloadRegistryTest, .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)); + EXPECT_EQ( + 0, rtp_payload_registry_->RegisterReceivePayload(audio_codec, &ignored)); } TEST_F(RtpPayloadRegistryTest, @@ -172,13 +219,15 @@ TEST_F(RtpPayloadRegistryTest, bool ignored = false; ExpectReturnOfTypicalAudioPayload(payload_type, kTypicalRate); - EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload( - kTypicalPayloadName, payload_type, kTypicalFrequency, - kTypicalChannels, kTypicalRate, &ignored)); + 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( - kTypicalPayloadName, payload_type - 1, kTypicalFrequency, - kTypicalChannels, kTypicalRate, &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_FALSE(rtp_payload_registry_->PayloadTypeToPayload(payload_type)) << "The first payload should be " @@ -190,9 +239,10 @@ TEST_F(RtpPayloadRegistryTest, 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)); + CodecInst audio_codec_3 = kTypicalAudioCodec; + audio_codec_3.pltype = payload_type + 1; + EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload(audio_codec_3, + &ignored)); EXPECT_TRUE(rtp_payload_registry_->PayloadTypeToPayload(payload_type - 1)) << "Not compatible; both payloads should be kept."; @@ -212,9 +262,10 @@ TEST_F(RtpPayloadRegistryTest, bool ignored; ExpectReturnOfTypicalAudioPayload(34, kTypicalRate); - EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload( - kTypicalPayloadName, 34, kTypicalFrequency, kTypicalChannels, - kTypicalRate, &ignored)); + CodecInst audio_codec = kTypicalAudioCodec; + audio_codec.pltype = 34; + 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); @@ -230,9 +281,14 @@ TEST_P(ParameterizedRtpPayloadRegistryTest, int payload_type = GetParam(); bool ignored; - EXPECT_EQ(-1, rtp_payload_registry_->RegisterReceivePayload( - "whatever", static_cast(payload_type), 19, 1, 17, - &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.channels = 1; + audio_codec.rate = 17; + EXPECT_EQ( + -1, rtp_payload_registry_->RegisterReceivePayload(audio_codec, &ignored)); } INSTANTIATE_TEST_CASE_P(TestKnownBadPayloadTypes, @@ -244,13 +300,16 @@ class RtpPayloadRegistryGenericTest public ::testing::WithParamInterface {}; TEST_P(RtpPayloadRegistryGenericTest, RegisterGenericReceivePayloadType) { - int payload_type = GetParam(); - bool ignored; - - EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload( - "generic-codec", static_cast(payload_type), 19, 1, - 17, &ignored)); // dummy values, except for payload_type + 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.channels = 1; + audio_codec.rate = 17; + EXPECT_EQ( + 0, rtp_payload_registry_->RegisterReceivePayload(audio_codec, &ignored)); } // Generates an RTX packet for the given length and original sequence number. diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc b/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc index 082d2a8b18..acc5926e5b 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc @@ -14,6 +14,7 @@ #include // pow() #include // memcpy() +#include "webrtc/common_types.h" #include "webrtc/base/logging.h" #include "webrtc/base/trace_event.h" @@ -103,24 +104,22 @@ bool RTPReceiverAudio::ShouldReportCsrcChanges(uint8_t payload_type) const { // - // - G7221 frame N/A int32_t RTPReceiverAudio::OnNewPayloadTypeCreated( - const char payload_name[RTP_PAYLOAD_NAME_SIZE], - int8_t payload_type, - uint32_t frequency) { + const CodecInst& audio_codec) { rtc::CritScope lock(&crit_sect_); - if (RtpUtility::StringCompare(payload_name, "telephone-event", 15)) { - telephone_event_payload_type_ = payload_type; + if (RtpUtility::StringCompare(audio_codec.plname, "telephone-event", 15)) { + telephone_event_payload_type_ = audio_codec.pltype; } - if (RtpUtility::StringCompare(payload_name, "cn", 2)) { + if (RtpUtility::StringCompare(audio_codec.plname, "cn", 2)) { // We support comfort noise at four different frequencies. - if (frequency == 8000) { - cng_nb_payload_type_ = payload_type; - } else if (frequency == 16000) { - cng_wb_payload_type_ = payload_type; - } else if (frequency == 32000) { - cng_swb_payload_type_ = payload_type; - } else if (frequency == 48000) { - cng_fb_payload_type_ = payload_type; + if (audio_codec.plfreq == 8000) { + cng_nb_payload_type_ = audio_codec.pltype; + } else if (audio_codec.plfreq == 16000) { + cng_wb_payload_type_ = audio_codec.pltype; + } else if (audio_codec.plfreq == 32000) { + cng_swb_payload_type_ = audio_codec.pltype; + } else if (audio_codec.plfreq == 48000) { + cng_fb_payload_type_ = audio_codec.pltype; } else { assert(false); return -1; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h b/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h index 29a4c7c391..15d0bdeae6 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h @@ -56,10 +56,7 @@ class RTPReceiverAudio : public RTPReceiverStrategy, bool ShouldReportCsrcChanges(uint8_t payload_type) const override; - int32_t OnNewPayloadTypeCreated( - const char payload_name[RTP_PAYLOAD_NAME_SIZE], - int8_t payload_type, - uint32_t frequency) override; + int32_t OnNewPayloadTypeCreated(const CodecInst& audio_codec) override; int32_t InvokeOnInitializeDecoder( RtpFeedback* callback, diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc index 190449b3dd..1e0bf732ad 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc @@ -16,6 +16,7 @@ #include #include "webrtc/base/logging.h" +#include "webrtc/common_types.h" #include "webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "webrtc/modules/rtp_rtcp/source/rtp_receiver_strategy.h" @@ -80,12 +81,7 @@ RtpReceiverImpl::~RtpReceiverImpl() { } } -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) { +int32_t RtpReceiverImpl::RegisterReceivePayload(const CodecInst& audio_codec) { rtc::CritScope lock(&critical_section_rtp_receiver_); // TODO(phoglund): Try to streamline handling of the RED codec and some other @@ -93,19 +89,33 @@ int32_t RtpReceiverImpl::RegisterReceivePayload( // payload or not. bool created_new_payload = false; int32_t result = rtp_payload_registry_->RegisterReceivePayload( - payload_name, payload_type, frequency, channels, rate, - &created_new_payload); + audio_codec, &created_new_payload); if (created_new_payload) { - if (rtp_media_receiver_->OnNewPayloadTypeCreated(payload_name, payload_type, - frequency) != 0) { - LOG(LS_ERROR) << "Failed to register payload: " << payload_name << "/" - << static_cast(payload_type); + if (rtp_media_receiver_->OnNewPayloadTypeCreated(audio_codec) != 0) { + LOG(LS_ERROR) << "Failed to register payload: " << audio_codec.plname + << "/" << static_cast(audio_codec.pltype); return -1; } } return result; } +// 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 1ae1c9168a..95a3c0674d 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.h @@ -33,6 +33,8 @@ class RtpReceiverImpl : public RtpReceiver { virtual ~RtpReceiverImpl(); + int32_t RegisterReceivePayload(const CodecInst& audio_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, diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_strategy.h b/webrtc/modules/rtp_rtcp/source/rtp_receiver_strategy.h index f435231764..490b4c59d5 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_strategy.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_strategy.h @@ -19,6 +19,8 @@ namespace webrtc { +struct CodecInst; + class TelephoneEventHandler; // This strategy deals with media-specific RTP packet processing. @@ -54,12 +56,9 @@ class RTPReceiverStrategy { // TODO(phoglund): should move out of here along with other payload stuff. virtual bool ShouldReportCsrcChanges(uint8_t payload_type) const = 0; - // Notifies the strategy that we have created a new non-RED payload type in - // the payload registry. - virtual int32_t OnNewPayloadTypeCreated( - const char payloadName[RTP_PAYLOAD_NAME_SIZE], - int8_t payloadType, - uint32_t frequency) = 0; + // Notifies the strategy that we have created a new non-RED audio payload type + // in the payload registry. + virtual int32_t OnNewPayloadTypeCreated(const CodecInst& audio_codec) = 0; // Invokes the OnInitializeDecoder callback in a media-specific way. virtual int32_t InvokeOnInitializeDecoder( diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc b/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc index 90e19c0645..4f79ea9e56 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc @@ -44,9 +44,8 @@ bool RTPReceiverVideo::ShouldReportCsrcChanges(uint8_t payload_type) const { } int32_t RTPReceiverVideo::OnNewPayloadTypeCreated( - const char payload_name[RTP_PAYLOAD_NAME_SIZE], - int8_t payload_type, - uint32_t frequency) { + const CodecInst& audio_codec) { + RTC_NOTREACHED(); return 0; } diff --git a/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.h b/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.h index f1628b10ef..c27fe45c82 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.h @@ -39,10 +39,7 @@ class RTPReceiverVideo : public RTPReceiverStrategy { bool ShouldReportCsrcChanges(uint8_t payload_type) const override; - int32_t OnNewPayloadTypeCreated( - const char payload_name[RTP_PAYLOAD_NAME_SIZE], - int8_t payload_type, - uint32_t frequency) override; + int32_t OnNewPayloadTypeCreated(const CodecInst& audio_codec) override; int32_t InvokeOnInitializeDecoder( RtpFeedback* callback, 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 5d66fd4189..175771755e 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_audio.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_audio.cc @@ -134,19 +134,9 @@ class RtpRtcpAudioTest : public ::testing::Test { void RegisterPayload(const CodecInst& codec) { EXPECT_EQ(0, module1->RegisterSendPayload(codec)); - EXPECT_EQ(0, rtp_receiver1_->RegisterReceivePayload( - codec.plname, - codec.pltype, - codec.plfreq, - codec.channels, - codec.rate)); + EXPECT_EQ(0, rtp_receiver1_->RegisterReceivePayload(codec)); EXPECT_EQ(0, module2->RegisterSendPayload(codec)); - EXPECT_EQ(0, rtp_receiver2_->RegisterReceivePayload( - codec.plname, - codec.pltype, - codec.plfreq, - codec.channels, - codec.rate)); + EXPECT_EQ(0, rtp_receiver2_->RegisterReceivePayload(codec)); } VerifyingAudioReceiver data_receiver1; @@ -222,12 +212,7 @@ TEST_F(RtpRtcpAudioTest, DTMF) { memcpy(voice_codec.plname, "telephone-event", 16); EXPECT_EQ(0, module1->RegisterSendPayload(voice_codec)); - EXPECT_EQ(0, rtp_receiver2_->RegisterReceivePayload( - voice_codec.plname, - voice_codec.pltype, - voice_codec.plfreq, - voice_codec.channels, - voice_codec.rate)); + EXPECT_EQ(0, rtp_receiver2_->RegisterReceivePayload(voice_codec)); // Start DTMF test. int timeStamp = 160; 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 63fa830cc4..a4715d2a1f 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_rtcp.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_rtcp.cc @@ -146,19 +146,9 @@ class RtpRtcpRtcpTest : public ::testing::Test { memcpy(voice_codec.plname, "PCMU", 5); EXPECT_EQ(0, module1->RegisterSendPayload(voice_codec)); - EXPECT_EQ(0, rtp_receiver1_->RegisterReceivePayload( - voice_codec.plname, - voice_codec.pltype, - voice_codec.plfreq, - voice_codec.channels, - (voice_codec.rate < 0) ? 0 : voice_codec.rate)); + EXPECT_EQ(0, rtp_receiver1_->RegisterReceivePayload(voice_codec)); EXPECT_EQ(0, module2->RegisterSendPayload(voice_codec)); - EXPECT_EQ(0, rtp_receiver2_->RegisterReceivePayload( - voice_codec.plname, - voice_codec.pltype, - voice_codec.plfreq, - voice_codec.channels, - (voice_codec.rate < 0) ? 0 : voice_codec.rate)); + EXPECT_EQ(0, rtp_receiver2_->RegisterReceivePayload(voice_codec)); // We need to send one RTP packet to get the RTCP packet to be accepted by // the receiving module. 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 39d272684b..102753782d 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_video.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_video.cc @@ -71,11 +71,7 @@ class RtpRtcpVideoTest : public ::testing::Test { memcpy(video_codec.plName, "I420", 5); EXPECT_EQ(0, video_module_->RegisterSendPayload(video_codec)); - EXPECT_EQ(0, rtp_receiver_->RegisterReceivePayload(video_codec.plName, - video_codec.plType, - 90000, - 0, - video_codec.maxBitrate)); + EXPECT_EQ(0, rtp_payload_registry_.RegisterReceivePayload(video_codec)); payload_data_length_ = sizeof(video_frame_); @@ -161,11 +157,7 @@ TEST_F(RtpRtcpVideoTest, PaddingOnlyFrames) { codec.codecType = kVideoCodecVP8; codec.plType = kPayloadType; strncpy(codec.plName, "VP8", 4); - EXPECT_EQ(0, rtp_receiver_->RegisterReceivePayload(codec.plName, - codec.plType, - 90000, - 0, - codec.maxBitrate)); + EXPECT_EQ(0, rtp_payload_registry_.RegisterReceivePayload(codec)); for (int frame_idx = 0; frame_idx < 10; ++frame_idx) { for (int packet_idx = 0; packet_idx < 5; ++packet_idx) { size_t packet_size = PaddingPacket(padding_packet, timestamp, seq_num, diff --git a/webrtc/video/rtp_stream_receiver.cc b/webrtc/video/rtp_stream_receiver.cc index fc84007610..f02cf3fa30 100644 --- a/webrtc/video/rtp_stream_receiver.cc +++ b/webrtc/video/rtp_stream_receiver.cc @@ -226,15 +226,12 @@ RtpStreamReceiver::~RtpStreamReceiver() { bool RtpStreamReceiver::SetReceiveCodec(const VideoCodec& video_codec) { int8_t old_pltype = -1; - if (rtp_payload_registry_.ReceivePayloadType( - video_codec.plName, kVideoPayloadTypeFrequency, 0, - video_codec.maxBitrate, &old_pltype) != -1) { + if (rtp_payload_registry_.ReceivePayloadType(video_codec, &old_pltype) != + -1) { rtp_payload_registry_.DeRegisterReceivePayload(old_pltype); } - return rtp_receiver_->RegisterReceivePayload( - video_codec.plName, video_codec.plType, kVideoPayloadTypeFrequency, - 0, 0) == 0; + return rtp_payload_registry_.RegisterReceivePayload(video_codec) == 0; } uint32_t RtpStreamReceiver::GetRemoteSsrc() const { diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc index c1410fa8b8..bdf6fb5387 100644 --- a/webrtc/voice_engine/channel.cc +++ b/webrtc/voice_engine/channel.cc @@ -1035,9 +1035,7 @@ int32_t Channel::Init() { for (int idx = 0; idx < nSupportedCodecs; idx++) { // Open up the RTP/RTCP receiver for all supported codecs if ((audio_coding_->Codec(idx, &codec) == -1) || - (rtp_receiver_->RegisterReceivePayload( - codec.plname, codec.pltype, codec.plfreq, codec.channels, - (codec.rate < 0) ? 0 : codec.rate) == -1)) { + (rtp_receiver_->RegisterReceivePayload(codec) == -1)) { WEBRTC_TRACE(kTraceWarning, kTraceVoice, VoEId(_instanceId, _channelId), "Channel::Init() unable to register %s " "(%d/%d/%" PRIuS "/%d) to RTP/RTCP receiver", @@ -1362,9 +1360,7 @@ int32_t Channel::SetRecPayloadType(const CodecInst& codec) { CodecInst rxCodec = codec; // Get payload type for the given codec - rtp_payload_registry_->ReceivePayloadType( - rxCodec.plname, rxCodec.plfreq, rxCodec.channels, - (rxCodec.rate < 0) ? 0 : rxCodec.rate, &pltype); + rtp_payload_registry_->ReceivePayloadType(rxCodec, &pltype); rxCodec.pltype = pltype; if (rtp_receiver_->DeRegisterReceivePayload(pltype) != 0) { @@ -1383,16 +1379,12 @@ int32_t Channel::SetRecPayloadType(const CodecInst& codec) { return 0; } - if (rtp_receiver_->RegisterReceivePayload( - codec.plname, codec.pltype, codec.plfreq, codec.channels, - (codec.rate < 0) ? 0 : codec.rate) != 0) { + if (rtp_receiver_->RegisterReceivePayload(codec) != 0) { // First attempt to register failed => de-register and try again // TODO(kwiberg): Retrying is probably not necessary, since // AcmReceiver::AddCodec also retries. rtp_receiver_->DeRegisterReceivePayload(codec.pltype); - if (rtp_receiver_->RegisterReceivePayload( - codec.plname, codec.pltype, codec.plfreq, codec.channels, - (codec.rate < 0) ? 0 : codec.rate) != 0) { + if (rtp_receiver_->RegisterReceivePayload(codec) != 0) { _engineStatisticsPtr->SetLastError( VE_RTP_RTCP_MODULE_ERROR, kTraceError, "SetRecPayloadType() RTP/RTCP-module registration failed"); @@ -1415,9 +1407,7 @@ int32_t Channel::SetRecPayloadType(const CodecInst& codec) { int32_t Channel::GetRecPayloadType(CodecInst& codec) { int8_t payloadType(-1); - if (rtp_payload_registry_->ReceivePayloadType( - codec.plname, codec.plfreq, codec.channels, - (codec.rate < 0) ? 0 : codec.rate, &payloadType) != 0) { + if (rtp_payload_registry_->ReceivePayloadType(codec, &payloadType) != 0) { _engineStatisticsPtr->SetLastError( VE_RTP_RTCP_MODULE_ERROR, kTraceWarning, "GetRecPayloadType() failed to retrieve RX payload type"); @@ -3152,9 +3142,7 @@ void Channel::RegisterReceiveCodecsToRTPModule() { for (int idx = 0; idx < nSupportedCodecs; idx++) { // Open up the RTP/RTCP receiver for all supported codecs if ((audio_coding_->Codec(idx, &codec) == -1) || - (rtp_receiver_->RegisterReceivePayload( - codec.plname, codec.pltype, codec.plfreq, codec.channels, - (codec.rate < 0) ? 0 : codec.rate) == -1)) { + (rtp_receiver_->RegisterReceivePayload(codec) == -1)) { WEBRTC_TRACE(kTraceWarning, kTraceVoice, VoEId(_instanceId, _channelId), "Channel::RegisterReceiveCodecsToRTPModule() unable" " to register %s (%d/%d/%" PRIuS