From 8842c3e41bcc4a2968d7c299f84f87099485a8e8 Mon Sep 17 00:00:00 2001 From: solenberg Date: Fri, 11 Mar 2016 03:06:41 -0800 Subject: [PATCH] Relanding https://codereview.webrtc.org/1715883002/ in pieces. - Use better types in AudioSendStream::SendTelephoneEvent() and related methods. BUG=webrtc:4690 Review URL: https://codereview.webrtc.org/1782053002 Cr-Commit-Position: refs/heads/master@{#11953} --- webrtc/audio/audio_send_stream.cc | 4 ++-- webrtc/audio/audio_send_stream.h | 4 ++-- webrtc/audio/audio_send_stream_unittest.cc | 4 ++-- webrtc/audio_send_stream.h | 4 ++-- webrtc/media/engine/fakewebrtccall.cc | 4 ++-- webrtc/media/engine/fakewebrtccall.h | 8 ++++---- webrtc/media/engine/webrtcvoiceengine.cc | 3 +-- .../rtp_rtcp/test/testAPI/test_api_audio.cc | 2 +- webrtc/test/mock_voe_channel_proxy.h | 3 +-- webrtc/voice_engine/channel.cc | 20 ++++++++++--------- webrtc/voice_engine/channel.h | 5 +---- webrtc/voice_engine/channel_proxy.cc | 6 ++---- webrtc/voice_engine/channel_proxy.h | 2 +- webrtc/voice_engine/dtmf_inband.h | 3 +++ 14 files changed, 35 insertions(+), 37 deletions(-) diff --git a/webrtc/audio/audio_send_stream.cc b/webrtc/audio/audio_send_stream.cc index 160a818323..24afcbcf58 100644 --- a/webrtc/audio/audio_send_stream.cc +++ b/webrtc/audio/audio_send_stream.cc @@ -125,8 +125,8 @@ bool AudioSendStream::DeliverRtcp(const uint8_t* packet, size_t length) { return false; } -bool AudioSendStream::SendTelephoneEvent(int payload_type, uint8_t event, - uint32_t duration_ms) { +bool AudioSendStream::SendTelephoneEvent(int payload_type, int event, + int duration_ms) { RTC_DCHECK(thread_checker_.CalledOnValidThread()); return channel_proxy_->SetSendTelephoneEventPayloadType(payload_type) && channel_proxy_->SendTelephoneEventOutband(event, duration_ms); diff --git a/webrtc/audio/audio_send_stream.h b/webrtc/audio/audio_send_stream.h index cf0a19ca4b..d463b3da30 100644 --- a/webrtc/audio/audio_send_stream.h +++ b/webrtc/audio/audio_send_stream.h @@ -40,8 +40,8 @@ class AudioSendStream final : public webrtc::AudioSendStream { bool DeliverRtcp(const uint8_t* packet, size_t length) override; // webrtc::AudioSendStream implementation. - bool SendTelephoneEvent(int payload_type, uint8_t event, - uint32_t duration_ms) override; + bool SendTelephoneEvent(int payload_type, int event, + int duration_ms) override; webrtc::AudioSendStream::Stats GetStats() const override; const webrtc::AudioSendStream::Config& config() const; diff --git a/webrtc/audio/audio_send_stream_unittest.cc b/webrtc/audio/audio_send_stream_unittest.cc index 6788699cd5..c04a3de77c 100644 --- a/webrtc/audio/audio_send_stream_unittest.cc +++ b/webrtc/audio/audio_send_stream_unittest.cc @@ -46,8 +46,8 @@ const CallStatistics kCallStats = { const CodecInst kCodecInst = {-121, "codec_name_send", 48000, -231, 0, -671}; const ReportBlock kReportBlock = {456, 780, 123, 567, 890, 132, 143, 13354}; const int kTelephoneEventPayloadType = 123; -const uint8_t kTelephoneEventCode = 45; -const uint32_t kTelephoneEventDuration = 6789; +const int kTelephoneEventCode = 45; +const int kTelephoneEventDuration = 6789; struct ConfigHelper { ConfigHelper() diff --git a/webrtc/audio_send_stream.h b/webrtc/audio_send_stream.h index d1af9e0103..24c3d77ab2 100644 --- a/webrtc/audio_send_stream.h +++ b/webrtc/audio_send_stream.h @@ -90,8 +90,8 @@ class AudioSendStream : public SendStream { }; // TODO(solenberg): Make payload_type a config property instead. - virtual bool SendTelephoneEvent(int payload_type, uint8_t event, - uint32_t duration_ms) = 0; + virtual bool SendTelephoneEvent(int payload_type, int event, + int duration_ms) = 0; virtual Stats GetStats() const = 0; }; } // namespace webrtc diff --git a/webrtc/media/engine/fakewebrtccall.cc b/webrtc/media/engine/fakewebrtccall.cc index aa94d48dfb..3277e75a64 100644 --- a/webrtc/media/engine/fakewebrtccall.cc +++ b/webrtc/media/engine/fakewebrtccall.cc @@ -39,8 +39,8 @@ FakeAudioSendStream::TelephoneEvent return latest_telephone_event_; } -bool FakeAudioSendStream::SendTelephoneEvent(int payload_type, uint8_t event, - uint32_t duration_ms) { +bool FakeAudioSendStream::SendTelephoneEvent(int payload_type, int event, + int duration_ms) { latest_telephone_event_.payload_type = payload_type; latest_telephone_event_.event_code = event; latest_telephone_event_.duration_ms = duration_ms; diff --git a/webrtc/media/engine/fakewebrtccall.h b/webrtc/media/engine/fakewebrtccall.h index 89a644a296..41a92dfac0 100644 --- a/webrtc/media/engine/fakewebrtccall.h +++ b/webrtc/media/engine/fakewebrtccall.h @@ -35,8 +35,8 @@ class FakeAudioSendStream final : public webrtc::AudioSendStream { public: struct TelephoneEvent { int payload_type = -1; - uint8_t event_code = 0; - uint32_t duration_ms = 0; + int event_code = 0; + int duration_ms = 0; }; explicit FakeAudioSendStream(const webrtc::AudioSendStream::Config& config); @@ -56,8 +56,8 @@ class FakeAudioSendStream final : public webrtc::AudioSendStream { } // webrtc::AudioSendStream implementation. - bool SendTelephoneEvent(int payload_type, uint8_t event, - uint32_t duration_ms) override; + bool SendTelephoneEvent(int payload_type, int event, + int duration_ms) override; webrtc::AudioSendStream::Stats GetStats() const override; TelephoneEvent latest_telephone_event_; diff --git a/webrtc/media/engine/webrtcvoiceengine.cc b/webrtc/media/engine/webrtcvoiceengine.cc index f57db27a31..a496316b42 100644 --- a/webrtc/media/engine/webrtcvoiceengine.cc +++ b/webrtc/media/engine/webrtcvoiceengine.cc @@ -1178,8 +1178,7 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream RTC_CHECK(stream_); } - bool SendTelephoneEvent(int payload_type, uint8_t event, - uint32_t duration_ms) { + bool SendTelephoneEvent(int payload_type, int event, int duration_ms) { RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); RTC_DCHECK(stream_); return stream_->SendTelephoneEvent(payload_type, event, duration_ms); 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 634969b311..32ab9379ae 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_audio.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_audio.cc @@ -322,7 +322,7 @@ TEST_F(RtpRtcpAudioTest, DTMF) { (voice_codec.rate < 0) ? 0 : voice_codec.rate)); // Start DTMF test. - uint32_t timeStamp = 160; + int timeStamp = 160; // Send a DTMF tone using RFC 2833 (4733). for (int i = 0; i < 16; i++) { diff --git a/webrtc/test/mock_voe_channel_proxy.h b/webrtc/test/mock_voe_channel_proxy.h index f5c87334cc..c2211f8554 100644 --- a/webrtc/test/mock_voe_channel_proxy.h +++ b/webrtc/test/mock_voe_channel_proxy.h @@ -43,8 +43,7 @@ class MockVoEChannelProxy : public voe::ChannelProxy { MOCK_CONST_METHOD0(GetSpeechOutputLevelFullRange, int32_t()); MOCK_CONST_METHOD0(GetDelayEstimate, uint32_t()); MOCK_METHOD1(SetSendTelephoneEventPayloadType, bool(int payload_type)); - MOCK_METHOD2(SendTelephoneEventOutband, bool(uint8_t event, - uint32_t duration_ms)); + MOCK_METHOD2(SendTelephoneEventOutband, bool(int event, int duration_ms)); }; } // namespace test } // namespace webrtc diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc index 11af45edc8..25ecca1d74 100644 --- a/webrtc/voice_engine/channel.cc +++ b/webrtc/voice_engine/channel.cc @@ -47,6 +47,8 @@ namespace webrtc { namespace voe { +const int kTelephoneEventAttenuationdB = 10; + class TransportFeedbackProxy : public TransportFeedbackObserver { public: TransportFeedbackProxy() : feedback_observer_(nullptr) { @@ -2212,21 +2214,21 @@ int Channel::GetChannelOutputVolumeScaling(float& scaling) const { return 0; } -int Channel::SendTelephoneEventOutband(unsigned char eventCode, - int lengthMs, - int attenuationDb, - bool playDtmfEvent) { +int Channel::SendTelephoneEventOutband(int event, int duration_ms) { WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId), - "Channel::SendTelephoneEventOutband(..., playDtmfEvent=%d)", - playDtmfEvent); + "Channel::SendTelephoneEventOutband(...)"); + RTC_DCHECK_LE(0, event); + RTC_DCHECK_GE(255, event); + RTC_DCHECK_LE(0, duration_ms); + RTC_DCHECK_GE(65535, duration_ms); if (!Sending()) { return -1; } - _playOutbandDtmfEvent = playDtmfEvent; + _playOutbandDtmfEvent = false; - if (_rtpRtcpModule->SendTelephoneEventOutband(eventCode, lengthMs, - attenuationDb) != 0) { + if (_rtpRtcpModule->SendTelephoneEventOutband( + event, duration_ms, kTelephoneEventAttenuationdB) != 0) { _engineStatisticsPtr->SetLastError( VE_SEND_DTMF_FAILED, kTraceWarning, "SendTelephoneEventOutband() failed to send event"); diff --git a/webrtc/voice_engine/channel.h b/webrtc/voice_engine/channel.h index 75c4fd87cb..65c34f66c8 100644 --- a/webrtc/voice_engine/channel.h +++ b/webrtc/voice_engine/channel.h @@ -297,10 +297,7 @@ class Channel int GetRtpRtcp(RtpRtcp** rtpRtcpModule, RtpReceiver** rtp_receiver) const; // VoEDtmf - int SendTelephoneEventOutband(unsigned char eventCode, - int lengthMs, - int attenuationDb, - bool playDtmfEvent); + int SendTelephoneEventOutband(int event, int duration_ms); int SendTelephoneEventInband(unsigned char eventCode, int lengthMs, int attenuationDb, diff --git a/webrtc/voice_engine/channel_proxy.cc b/webrtc/voice_engine/channel_proxy.cc index da7864f15f..10c8821202 100644 --- a/webrtc/voice_engine/channel_proxy.cc +++ b/webrtc/voice_engine/channel_proxy.cc @@ -148,11 +148,9 @@ bool ChannelProxy::SetSendTelephoneEventPayloadType(int payload_type) { return channel()->SetSendTelephoneEventPayloadType(payload_type) == 0; } -bool ChannelProxy::SendTelephoneEventOutband(uint8_t event, - uint32_t duration_ms) { +bool ChannelProxy::SendTelephoneEventOutband(int event, int duration_ms) { RTC_DCHECK(thread_checker_.CalledOnValidThread()); - return - channel()->SendTelephoneEventOutband(event, duration_ms, 10, false) == 0; + return channel()->SendTelephoneEventOutband(event, duration_ms) == 0; } void ChannelProxy::SetSink(std::unique_ptr sink) { diff --git a/webrtc/voice_engine/channel_proxy.h b/webrtc/voice_engine/channel_proxy.h index 3461cf3e78..dec726cb23 100644 --- a/webrtc/voice_engine/channel_proxy.h +++ b/webrtc/voice_engine/channel_proxy.h @@ -68,7 +68,7 @@ class ChannelProxy { virtual uint32_t GetDelayEstimate() const; virtual bool SetSendTelephoneEventPayloadType(int payload_type); - virtual bool SendTelephoneEventOutband(uint8_t event, uint32_t duration_ms); + virtual bool SendTelephoneEventOutband(int event, int duration_ms); virtual void SetSink(std::unique_ptr sink); diff --git a/webrtc/voice_engine/dtmf_inband.h b/webrtc/voice_engine/dtmf_inband.h index 6000d99214..795c5ce8fc 100644 --- a/webrtc/voice_engine/dtmf_inband.h +++ b/webrtc/voice_engine/dtmf_inband.h @@ -17,6 +17,9 @@ namespace webrtc { +// TODO(solenberg): Used as a DTMF tone generator in voe::OutputMixer. Pull out +// the one in NetEq and use that instead? We don't need several +// implemenations of this. class DtmfInband { public: