From 6021fe2b1e4c7d55cca92ef01cc2750516aa3e9f Mon Sep 17 00:00:00 2001 From: solenberg Date: Tue, 15 Mar 2016 11:41:53 -0700 Subject: [PATCH] Clean away use of RtpAudioFeedback interface from RTP/RTCP sender code. BUG=webrtc:4690 Review URL: https://codereview.webrtc.org/1803923003 Cr-Commit-Position: refs/heads/master@{#12003} --- webrtc/call/rtc_event_log_unittest.cc | 1 - webrtc/modules/rtp_rtcp/include/rtp_rtcp.h | 3 --- webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h | 2 -- webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 2 -- webrtc/modules/rtp_rtcp/source/rtp_sender.cc | 3 +-- webrtc/modules/rtp_rtcp/source/rtp_sender.h | 1 - webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc | 11 +---------- webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h | 5 +---- .../modules/rtp_rtcp/source/rtp_sender_unittest.cc | 10 +++++----- .../modules/rtp_rtcp/test/testAPI/test_api_audio.cc | 2 -- webrtc/voice_engine/channel.cc | 13 +------------ webrtc/voice_engine/channel.h | 6 ------ webrtc/voice_engine/include/voe_errors.h | 2 +- 13 files changed, 10 insertions(+), 51 deletions(-) diff --git a/webrtc/call/rtc_event_log_unittest.cc b/webrtc/call/rtc_event_log_unittest.cc index 3039c8babb..e3104591d9 100644 --- a/webrtc/call/rtc_event_log_unittest.cc +++ b/webrtc/call/rtc_event_log_unittest.cc @@ -308,7 +308,6 @@ size_t GenerateRtpPacket(uint32_t extensions_bitvector, RTPSender rtp_sender(false, // bool audio clock, // Clock* clock nullptr, // Transport* - nullptr, // RtpAudioFeedback* nullptr, // PacedSender* nullptr, // PacketRouter* nullptr, // SendTimeObserver* diff --git a/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h b/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h index 1d01009f35..d01465b9f8 100644 --- a/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h @@ -55,8 +55,6 @@ class RtpRtcp : public Module { * intra_frame_callback - Called when the receiver request a intra frame. * bandwidth_callback - Called when we receive a changed estimate from * the receiver of out stream. - * audio_messages - Telephone events. May not be NULL; default - * callback will do nothing. * remote_bitrate_estimator - Estimates the bandwidth available for a set of * streams from the same client. * paced_sender - Spread any bursts of packets into smaller @@ -72,7 +70,6 @@ class RtpRtcp : public Module { TransportFeedbackObserver* transport_feedback_callback; RtcpRttStats* rtt_stats; RtcpPacketTypeCounterObserver* rtcp_packet_type_counter_observer; - RtpAudioFeedback* audio_messages; RemoteBitrateEstimator* remote_bitrate_estimator; RtpPacketSender* paced_sender; TransportSequenceNumberAllocator* transport_sequence_number_allocator; diff --git a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index 4a4bb22e00..9991aa2110 100644 --- a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -219,8 +219,6 @@ class MockRtpRtcp : public RtpRtcp { MOCK_METHOD1(RegisterRtcpStatisticsCallback, void(RtcpStatisticsCallback*)); MOCK_METHOD0(GetRtcpStatisticsCallback, RtcpStatisticsCallback*()); MOCK_METHOD1(SendFeedbackPacket, bool(const rtcp::TransportFeedback& packet)); - MOCK_METHOD1(RegisterAudioCallback, - int32_t(RtpAudioFeedback* messagesCallback)); MOCK_METHOD1(SetAudioPacketSize, int32_t(const uint16_t packetSizeSamples)); MOCK_METHOD3(SendTelephoneEventOutband, diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index e67b635214..ef720bef52 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -54,7 +54,6 @@ RtpRtcp::Configuration::Configuration() transport_feedback_callback(nullptr), rtt_stats(nullptr), rtcp_packet_type_counter_observer(nullptr), - audio_messages(NullObjectRtpAudioFeedback()), remote_bitrate_estimator(nullptr), paced_sender(nullptr), transport_sequence_number_allocator(nullptr), @@ -80,7 +79,6 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) : rtp_sender_(configuration.audio, configuration.clock, configuration.outgoing_transport, - configuration.audio_messages, configuration.paced_sender, configuration.transport_sequence_number_allocator, configuration.transport_feedback_callback, diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc index 3197d60f92..36d7eb573c 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc @@ -108,7 +108,6 @@ RTPSender::RTPSender( bool audio, Clock* clock, Transport* transport, - RtpAudioFeedback* audio_feedback, RtpPacketSender* paced_sender, TransportSequenceNumberAllocator* sequence_number_allocator, TransportFeedbackObserver* transport_feedback_observer, @@ -125,7 +124,7 @@ RTPSender::RTPSender( bitrates_(bitrate_callback), total_bitrate_sent_(clock, bitrates_.total_bitrate_observer()), audio_configured_(audio), - audio_(audio ? new RTPSenderAudio(clock, this, audio_feedback) : nullptr), + audio_(audio ? new RTPSenderAudio(clock, this) : nullptr), video_(audio ? nullptr : new RTPSenderVideo(clock, this)), paced_sender_(paced_sender), transport_sequence_number_allocator_(sequence_number_allocator), diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.h b/webrtc/modules/rtp_rtcp/source/rtp_sender.h index 5b6d4f7af7..b376c380d0 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.h @@ -90,7 +90,6 @@ class RTPSender : public RTPSenderInterface { RTPSender(bool audio, Clock* clock, Transport* transport, - RtpAudioFeedback* audio_feedback, RtpPacketSender* paced_sender, TransportSequenceNumberAllocator* sequence_number_allocator, TransportFeedbackObserver* transport_feedback_callback, diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc index 2aa4961cdc..804294ac54 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc @@ -21,12 +21,9 @@ namespace webrtc { static const int kDtmfFrequencyHz = 8000; -RTPSenderAudio::RTPSenderAudio(Clock* clock, - RTPSender* rtpSender, - RtpAudioFeedback* audio_feedback) +RTPSenderAudio::RTPSenderAudio(Clock* clock, RTPSender* rtpSender) : _clock(clock), _rtpSender(rtpSender), - _audioFeedback(audio_feedback), _sendAudioCritsect(CriticalSectionWrapper::CreateCriticalSection()), _packetSizeSamples(160), _dtmfEventIsOn(false), @@ -158,7 +155,6 @@ int32_t RTPSenderAudio::SendAudio(FrameType frameType, // TODO(pwestin) Breakup function in smaller functions. size_t payloadSize = dataSize; size_t maxPayloadLength = _rtpSender->MaxPayloadLength(); - bool dtmfToneStarted = false; uint16_t dtmfLengthMS = 0; uint8_t key = 0; int red_payload_type; @@ -185,15 +181,10 @@ int32_t RTPSenderAudio::SendAudio(FrameType frameType, _dtmfEventFirstPacketSent = false; _dtmfKey = key; _dtmfLengthSamples = (kDtmfFrequencyHz / 1000) * dtmfLengthMS; - dtmfToneStarted = true; _dtmfEventIsOn = true; } } } - if (dtmfToneStarted) { - if (_audioFeedback) - _audioFeedback->OnPlayTelephoneEvent(key, dtmfLengthMS, _dtmfLevel); - } // A source MAY send events and coded audio packets for the same time // but we don't support it diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h b/webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h index 1e96d17a67..25c5e4dd88 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h @@ -21,9 +21,7 @@ namespace webrtc { class RTPSenderAudio : public DTMFqueue { public: - RTPSenderAudio(Clock* clock, - RTPSender* rtpSender, - RtpAudioFeedback* audio_feedback); + RTPSenderAudio(Clock* clock, RTPSender* rtpSender); virtual ~RTPSenderAudio(); int32_t RegisterAudioPayload(const char payloadName[RTP_PAYLOAD_NAME_SIZE], @@ -73,7 +71,6 @@ class RTPSenderAudio : public DTMFqueue { private: Clock* const _clock; RTPSender* const _rtpSender; - RtpAudioFeedback* const _audioFeedback; rtc::scoped_ptr _sendAudioCritsect; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc index a0d6145909..b7238d26a2 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -141,7 +141,7 @@ class RtpSenderTest : public ::testing::Test { void SetUp() override { SetUpRtpSender(true); } void SetUpRtpSender(bool pacer) { - rtp_sender_.reset(new RTPSender(false, &fake_clock_, &transport_, nullptr, + rtp_sender_.reset(new RTPSender(false, &fake_clock_, &transport_, pacer ? &mock_paced_sender_ : nullptr, &seq_num_allocator_, nullptr, nullptr, nullptr, nullptr, &mock_rtc_event_log_)); @@ -954,7 +954,7 @@ TEST_F(RtpSenderTest, SendPadding) { TEST_F(RtpSenderTest, SendRedundantPayloads) { MockTransport transport; rtp_sender_.reset(new RTPSender( - false, &fake_clock_, &transport, nullptr, &mock_paced_sender_, nullptr, + false, &fake_clock_, &transport, &mock_paced_sender_, nullptr, nullptr, nullptr, nullptr, nullptr, &mock_rtc_event_log_)); rtp_sender_->SetSequenceNumber(kSeqNum); rtp_sender_->SetRtxPayloadType(kRtxPayload, kPayload); @@ -1096,7 +1096,7 @@ TEST_F(RtpSenderTest, FrameCountCallbacks) { FrameCounts frame_counts_; } callback; - rtp_sender_.reset(new RTPSender(false, &fake_clock_, &transport_, nullptr, + rtp_sender_.reset(new RTPSender(false, &fake_clock_, &transport_, &mock_paced_sender_, nullptr, nullptr, nullptr, &callback, nullptr, nullptr)); @@ -1152,7 +1152,7 @@ TEST_F(RtpSenderTest, BitrateCallbacks) { BitrateStatistics total_stats_; BitrateStatistics retransmit_stats_; } callback; - rtp_sender_.reset(new RTPSender(false, &fake_clock_, &transport_, nullptr, + rtp_sender_.reset(new RTPSender(false, &fake_clock_, &transport_, nullptr, nullptr, nullptr, &callback, nullptr, nullptr, nullptr)); @@ -1205,7 +1205,7 @@ class RtpSenderAudioTest : public RtpSenderTest { void SetUp() override { payload_ = kAudioPayload; - rtp_sender_.reset(new RTPSender(true, &fake_clock_, &transport_, nullptr, + rtp_sender_.reset(new RTPSender(true, &fake_clock_, &transport_, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr)); rtp_sender_->SetSequenceNumber(kSeqNum); 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 32ab9379ae..11b592afcc 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_audio.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_audio.cc @@ -106,7 +106,6 @@ class RtpRtcpAudioTest : public ::testing::Test { configuration.clock = &fake_clock; configuration.receive_statistics = receive_statistics1_.get(); configuration.outgoing_transport = transport1; - configuration.audio_messages = audioFeedback; module1 = RtpRtcp::CreateRtpRtcp(configuration); rtp_receiver1_.reset(RtpReceiver::CreateAudioReceiver( @@ -115,7 +114,6 @@ class RtpRtcpAudioTest : public ::testing::Test { configuration.receive_statistics = receive_statistics2_.get(); configuration.outgoing_transport = transport2; - configuration.audio_messages = audioFeedback; module2 = RtpRtcp::CreateRtpRtcp(configuration); rtp_receiver2_.reset(RtpReceiver::CreateAudioReceiver( diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc index 1c0cb340fd..5bbbce3c40 100644 --- a/webrtc/voice_engine/channel.cc +++ b/webrtc/voice_engine/channel.cc @@ -353,15 +353,6 @@ bool Channel::SendRtcp(const uint8_t* data, size_t len) { return true; } -void Channel::OnPlayTelephoneEvent(uint8_t event, - uint16_t lengthMs, - uint8_t volume) { - WEBRTC_TRACE(kTraceStream, kTraceVoice, VoEId(_instanceId, _channelId), - "Channel::OnPlayTelephoneEvent(event=%u, lengthMs=%u," - " volume=%u)", - event, lengthMs, volume); -} - void Channel::OnIncomingSSRCChanged(uint32_t ssrc) { WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId), "Channel::OnIncomingSSRCChanged(SSRC=%d)", ssrc); @@ -731,7 +722,7 @@ Channel::Channel(int32_t channelId, ReceiveStatistics::Create(Clock::GetRealTimeClock())), rtp_receiver_( RtpReceiver::CreateAudioReceiver(Clock::GetRealTimeClock(), - this, + nullptr, this, this, rtp_payload_registry_.get())), @@ -816,7 +807,6 @@ Channel::Channel(int32_t channelId, RtpRtcp::Configuration configuration; configuration.audio = true; configuration.outgoing_transport = this; - configuration.audio_messages = this; configuration.receive_statistics = rtp_receive_statistics_.get(); configuration.bandwidth_callback = rtcp_observer_.get(); if (pacing_enabled_) { @@ -2201,7 +2191,6 @@ int Channel::SendTelephoneEventOutband(int event, int duration_ms) { if (!Sending()) { return -1; } - if (_rtpRtcpModule->SendTelephoneEventOutband( event, duration_ms, kTelephoneEventAttenuationdB) != 0) { _engineStatisticsPtr->SetLastError( diff --git a/webrtc/voice_engine/channel.h b/webrtc/voice_engine/channel.h index c89b0e0c8b..7d9a17fcf3 100644 --- a/webrtc/voice_engine/channel.h +++ b/webrtc/voice_engine/channel.h @@ -156,7 +156,6 @@ class Channel public FileCallback, // receiving notification from file player & // recorder public Transport, - public RtpAudioFeedback, public AudioPacketizationCallback, // receive encoded packets from the // ACM public ACMVADCallback, // receive voice activity from the ACM @@ -385,11 +384,6 @@ class Channel void OnIncomingSSRCChanged(uint32_t ssrc) override; void OnIncomingCSRCChanged(uint32_t CSRC, bool added) override; - // From RtpAudioFeedback in the RTP/RTCP module - void OnPlayTelephoneEvent(uint8_t event, - uint16_t lengthMs, - uint8_t volume) override; - // From Transport (called by the RTP/RTCP module) bool SendRtp(const uint8_t* data, size_t len, diff --git a/webrtc/voice_engine/include/voe_errors.h b/webrtc/voice_engine/include/voe_errors.h index 04f02be934..645774de25 100644 --- a/webrtc/voice_engine/include/voe_errors.h +++ b/webrtc/voice_engine/include/voe_errors.h @@ -33,7 +33,7 @@ #define VE_INVALID_IP_ADDRESS 8019 #define VE_ALREADY_PLAYING 8020 #define VE_NOT_ALL_VERSION_INFO 8021 -#define VE_DTMF_OUTOF_RANGE 8022 +// 8022 is not used #define VE_INVALID_CHANNELS 8023 #define VE_SET_PLTYPE_FAILED 8024 // 8025 is not used