From b521aa704f75888a68edb6b5b046a0dce7d56f51 Mon Sep 17 00:00:00 2001 From: stefan Date: Tue, 1 Nov 2016 03:17:16 -0700 Subject: [PATCH] Clean up abs-send-time for audio. BUG=None Review-Url: https://codereview.webrtc.org/2455013003 Cr-Commit-Position: refs/heads/master@{#14870} --- webrtc/audio/audio_receive_stream.cc | 5 -- webrtc/audio/audio_receive_stream_unittest.cc | 25 ++----- webrtc/audio/audio_send_stream.cc | 4 +- webrtc/audio/audio_send_stream_unittest.cc | 14 ++-- webrtc/media/engine/webrtcvoiceengine.cc | 3 - webrtc/test/mock_voe_channel_proxy.h | 2 - webrtc/voice_engine/channel.cc | 15 ----- webrtc/voice_engine/channel_proxy.cc | 12 ---- webrtc/voice_engine/channel_proxy.h | 2 - webrtc/voice_engine/include/voe_rtp_rtcp.h | 10 --- .../auto_test/standard/rtp_rtcp_extensions.cc | 37 ----------- webrtc/voice_engine/voe_rtp_rtcp_impl.cc | 65 +------------------ webrtc/voice_engine/voe_rtp_rtcp_impl.h | 8 --- 13 files changed, 13 insertions(+), 189 deletions(-) diff --git a/webrtc/audio/audio_receive_stream.cc b/webrtc/audio/audio_receive_stream.cc index 657a7608ec..c325b9c9bc 100644 --- a/webrtc/audio/audio_receive_stream.cc +++ b/webrtc/audio/audio_receive_stream.cc @@ -118,11 +118,6 @@ AudioReceiveStream::AudioReceiveStream( bool registered = rtp_header_parser_->RegisterRtpHeaderExtension( kRtpExtensionAudioLevel, extension.id); RTC_DCHECK(registered); - } else if (extension.uri == RtpExtension::kAbsSendTimeUri) { - channel_proxy_->SetReceiveAbsoluteSenderTimeStatus(true, extension.id); - bool registered = rtp_header_parser_->RegisterRtpHeaderExtension( - kRtpExtensionAbsoluteSendTime, extension.id); - RTC_DCHECK(registered); } else if (extension.uri == RtpExtension::kTransportSequenceNumberUri) { channel_proxy_->EnableReceiveTransportSequenceNumber(extension.id); bool registered = rtp_header_parser_->RegisterRtpHeaderExtension( diff --git a/webrtc/audio/audio_receive_stream_unittest.cc b/webrtc/audio/audio_receive_stream_unittest.cc index b11d04bb46..bde68ed3ae 100644 --- a/webrtc/audio/audio_receive_stream_unittest.cc +++ b/webrtc/audio/audio_receive_stream_unittest.cc @@ -51,7 +51,6 @@ const uint32_t kRemoteSsrc = 1234; const uint32_t kLocalSsrc = 5678; const size_t kOneByteExtensionHeaderLength = 4; const size_t kOneByteExtensionLength = 4; -const int kAbsSendTimeId = 2; const int kAudioLevelId = 3; const int kTransportSequenceNumberId = 4; const int kJitterBufferDelay = -7; @@ -89,9 +88,6 @@ struct ConfigHelper { channel_proxy_ = new testing::StrictMock(); EXPECT_CALL(*channel_proxy_, SetLocalSSRC(kLocalSsrc)).Times(1); EXPECT_CALL(*channel_proxy_, SetNACKStatus(true, 15)).Times(1); - EXPECT_CALL(*channel_proxy_, - SetReceiveAbsoluteSenderTimeStatus(true, kAbsSendTimeId)) - .Times(1); EXPECT_CALL(*channel_proxy_, SetReceiveAudioLevelIndicationStatus(true, kAudioLevelId)) .Times(1); @@ -124,8 +120,6 @@ struct ConfigHelper { stream_config_.rtp.local_ssrc = kLocalSsrc; stream_config_.rtp.remote_ssrc = kRemoteSsrc; stream_config_.rtp.nack.rtp_history_ms = 300; - stream_config_.rtp.extensions.push_back( - RtpExtension(RtpExtension::kAbsSendTimeUri, kAbsSendTimeId)); stream_config_.rtp.extensions.push_back( RtpExtension(RtpExtension::kAudioLevelUri, kAudioLevelId)); stream_config_.rtp.extensions.push_back(RtpExtension( @@ -244,15 +238,14 @@ TEST(AudioReceiveStreamTest, ConfigToString) { AudioReceiveStream::Config config; config.rtp.remote_ssrc = kRemoteSsrc; config.rtp.local_ssrc = kLocalSsrc; - config.rtp.extensions.push_back( - RtpExtension(RtpExtension::kAbsSendTimeUri, kAbsSendTimeId)); config.voe_channel_id = kChannelId; + config.rtp.extensions.push_back( + RtpExtension(RtpExtension::kAudioLevelUri, kAudioLevelId)); EXPECT_EQ( - "{rtp: {remote_ssrc: 1234, local_ssrc: 5678, transport_cc: off, " - "nack: {rtp_history_ms: 0}, extensions: [{uri: " - "http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time, id: 2}]}, " - "rtcp_send_transport: nullptr, " - "voe_channel_id: 2}", + "{rtp: {remote_ssrc: 1234, local_ssrc: 5678, transport_cc: off, nack: " + "{rtp_history_ms: 0}, extensions: [{uri: " + "urn:ietf:params:rtp-hdrext:ssrc-audio-level, id: 3}]}, " + "rtcp_send_transport: nullptr, voe_channel_id: 2}", config.ToString()); } @@ -264,11 +257,7 @@ TEST(AudioReceiveStreamTest, ConstructDestruct) { } MATCHER_P(VerifyHeaderExtension, expected_extension, "") { - return arg.extension.hasAbsoluteSendTime == - expected_extension.hasAbsoluteSendTime && - arg.extension.absoluteSendTime == - expected_extension.absoluteSendTime && - arg.extension.hasTransportSequenceNumber == + return arg.extension.hasTransportSequenceNumber == expected_extension.hasTransportSequenceNumber && arg.extension.transportSequenceNumber == expected_extension.transportSequenceNumber; diff --git a/webrtc/audio/audio_send_stream.cc b/webrtc/audio/audio_send_stream.cc index 51f9073343..39066720cb 100644 --- a/webrtc/audio/audio_send_stream.cc +++ b/webrtc/audio/audio_send_stream.cc @@ -75,9 +75,7 @@ AudioSendStream::AudioSendStream( channel_proxy_->RegisterExternalTransport(config.send_transport); for (const auto& extension : config.rtp.extensions) { - if (extension.uri == RtpExtension::kAbsSendTimeUri) { - channel_proxy_->SetSendAbsoluteSenderTimeStatus(true, extension.id); - } else if (extension.uri == RtpExtension::kAudioLevelUri) { + if (extension.uri == RtpExtension::kAudioLevelUri) { channel_proxy_->SetSendAudioLevelIndicationStatus(true, extension.id); } else if (extension.uri == RtpExtension::kTransportSequenceNumberUri) { channel_proxy_->EnableSendTransportSequenceNumber(extension.id); diff --git a/webrtc/audio/audio_send_stream_unittest.cc b/webrtc/audio/audio_send_stream_unittest.cc index 3cfe13f10b..f310d00ba9 100644 --- a/webrtc/audio/audio_send_stream_unittest.cc +++ b/webrtc/audio/audio_send_stream_unittest.cc @@ -35,7 +35,6 @@ const int kChannelId = 1; const uint32_t kSsrc = 1234; const char* kCName = "foo_name"; const int kAudioLevelId = 2; -const int kAbsSendTimeId = 3; const int kTransportSequenceNumberId = 4; const int kEchoDelayMedian = 254; const int kEchoDelayStdDev = -3; @@ -93,8 +92,6 @@ struct ConfigHelper { stream_config_.rtp.c_name = kCName; stream_config_.rtp.extensions.push_back( RtpExtension(RtpExtension::kAudioLevelUri, kAudioLevelId)); - stream_config_.rtp.extensions.push_back( - RtpExtension(RtpExtension::kAbsSendTimeUri, kAbsSendTimeId)); stream_config_.rtp.extensions.push_back(RtpExtension( RtpExtension::kTransportSequenceNumberUri, kTransportSequenceNumberId)); // Use ISAC as default codec so as to prevent unnecessary |voice_engine_| @@ -120,9 +117,6 @@ struct ConfigHelper { EXPECT_CALL(*channel_proxy_, SetLocalSSRC(kSsrc)).Times(1); EXPECT_CALL(*channel_proxy_, SetRTCP_CNAME(StrEq(kCName))).Times(1); EXPECT_CALL(*channel_proxy_, SetNACKStatus(true, 10)).Times(1); - EXPECT_CALL(*channel_proxy_, - SetSendAbsoluteSenderTimeStatus(true, kAbsSendTimeId)) - .Times(1); EXPECT_CALL(*channel_proxy_, SetSendAudioLevelIndicationStatus(true, kAudioLevelId)) .Times(1); @@ -219,8 +213,6 @@ struct ConfigHelper { TEST(AudioSendStreamTest, ConfigToString) { AudioSendStream::Config config(nullptr); config.rtp.ssrc = kSsrc; - config.rtp.extensions.push_back( - RtpExtension(RtpExtension::kAbsSendTimeUri, kAbsSendTimeId)); config.rtp.c_name = kCName; config.voe_channel_id = kChannelId; config.min_bitrate_kbps = 12; @@ -235,10 +227,12 @@ TEST(AudioSendStreamTest, ConfigToString) { config.send_codec_spec.min_ptime_ms = 20; config.send_codec_spec.max_ptime_ms = 60; config.send_codec_spec.codec_inst = kIsacCodec; + config.rtp.extensions.push_back( + RtpExtension(RtpExtension::kAudioLevelUri, kAudioLevelId)); EXPECT_EQ( "{rtp: {ssrc: 1234, extensions: [{uri: " - "http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time, id: 3}], " - "nack: {rtp_history_ms: 0}, c_name: foo_name}, send_transport: nullptr, " + "urn:ietf:params:rtp-hdrext:ssrc-audio-level, id: 2}], nack: " + "{rtp_history_ms: 0}, c_name: foo_name}, send_transport: nullptr, " "voe_channel_id: 1, min_bitrate_kbps: 12, max_bitrate_kbps: 34, " "send_codec_spec: {nack_enabled: true, transport_cc_enabled: false, " "enable_codec_fec: true, enable_opus_dtx: false, opus_max_playback_rate: " diff --git a/webrtc/media/engine/webrtcvoiceengine.cc b/webrtc/media/engine/webrtcvoiceengine.cc index 21e7c8120f..fc901ae1e6 100644 --- a/webrtc/media/engine/webrtcvoiceengine.cc +++ b/webrtc/media/engine/webrtcvoiceengine.cc @@ -978,9 +978,6 @@ RtpCapabilities WebRtcVoiceEngine::GetCapabilities() const { capabilities.header_extensions.push_back( webrtc::RtpExtension(webrtc::RtpExtension::kAudioLevelUri, webrtc::RtpExtension::kAudioLevelDefaultId)); - capabilities.header_extensions.push_back( - webrtc::RtpExtension(webrtc::RtpExtension::kAbsSendTimeUri, - webrtc::RtpExtension::kAbsSendTimeDefaultId)); if (webrtc::field_trial::FindFullName("WebRTC-Audio-SendSideBwe") == "Enabled") { capabilities.header_extensions.push_back(webrtc::RtpExtension( diff --git a/webrtc/test/mock_voe_channel_proxy.h b/webrtc/test/mock_voe_channel_proxy.h index 4cbe50933b..2383519a85 100644 --- a/webrtc/test/mock_voe_channel_proxy.h +++ b/webrtc/test/mock_voe_channel_proxy.h @@ -25,9 +25,7 @@ class MockVoEChannelProxy : public voe::ChannelProxy { MOCK_METHOD1(SetLocalSSRC, void(uint32_t ssrc)); MOCK_METHOD1(SetRTCP_CNAME, void(const std::string& c_name)); MOCK_METHOD2(SetNACKStatus, void(bool enable, int max_packets)); - MOCK_METHOD2(SetSendAbsoluteSenderTimeStatus, void(bool enable, int id)); MOCK_METHOD2(SetSendAudioLevelIndicationStatus, void(bool enable, int id)); - MOCK_METHOD2(SetReceiveAbsoluteSenderTimeStatus, void(bool enable, int id)); MOCK_METHOD2(SetReceiveAudioLevelIndicationStatus, void(bool enable, int id)); MOCK_METHOD1(EnableSendTransportSequenceNumber, void(int id)); MOCK_METHOD1(EnableReceiveTransportSequenceNumber, void(int id)); diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc index c434b1d61c..3d195eb141 100644 --- a/webrtc/voice_engine/channel.cc +++ b/webrtc/voice_engine/channel.cc @@ -2361,21 +2361,6 @@ int Channel::SetReceiveAudioLevelIndicationStatus(bool enable, return 0; } -int Channel::SetSendAbsoluteSenderTimeStatus(bool enable, unsigned char id) { - return SetSendRtpHeaderExtension(enable, kRtpExtensionAbsoluteSendTime, id); -} - -int Channel::SetReceiveAbsoluteSenderTimeStatus(bool enable, unsigned char id) { - rtp_header_parser_->DeregisterRtpHeaderExtension( - kRtpExtensionAbsoluteSendTime); - if (enable && - !rtp_header_parser_->RegisterRtpHeaderExtension( - kRtpExtensionAbsoluteSendTime, id)) { - return -1; - } - return 0; -} - void Channel::EnableSendTransportSequenceNumber(int id) { int ret = SetSendRtpHeaderExtension(true, kRtpExtensionTransportSequenceNumber, id); diff --git a/webrtc/voice_engine/channel_proxy.cc b/webrtc/voice_engine/channel_proxy.cc index 67c7fe0cb4..6aef29cf0c 100644 --- a/webrtc/voice_engine/channel_proxy.cc +++ b/webrtc/voice_engine/channel_proxy.cc @@ -50,24 +50,12 @@ void ChannelProxy::SetNACKStatus(bool enable, int max_packets) { channel()->SetNACKStatus(enable, max_packets); } -void ChannelProxy::SetSendAbsoluteSenderTimeStatus(bool enable, int id) { - RTC_DCHECK(thread_checker_.CalledOnValidThread()); - int error = channel()->SetSendAbsoluteSenderTimeStatus(enable, id); - RTC_DCHECK_EQ(0, error); -} - void ChannelProxy::SetSendAudioLevelIndicationStatus(bool enable, int id) { RTC_DCHECK(thread_checker_.CalledOnValidThread()); int error = channel()->SetSendAudioLevelIndicationStatus(enable, id); RTC_DCHECK_EQ(0, error); } -void ChannelProxy::SetReceiveAbsoluteSenderTimeStatus(bool enable, int id) { - RTC_DCHECK(thread_checker_.CalledOnValidThread()); - int error = channel()->SetReceiveAbsoluteSenderTimeStatus(enable, id); - RTC_DCHECK_EQ(0, error); -} - void ChannelProxy::SetReceiveAudioLevelIndicationStatus(bool enable, int id) { RTC_DCHECK(thread_checker_.CalledOnValidThread()); int error = channel()->SetReceiveAudioLevelIndicationStatus(enable, id); diff --git a/webrtc/voice_engine/channel_proxy.h b/webrtc/voice_engine/channel_proxy.h index 8bb452ba19..ea0446e5b5 100644 --- a/webrtc/voice_engine/channel_proxy.h +++ b/webrtc/voice_engine/channel_proxy.h @@ -52,9 +52,7 @@ class ChannelProxy { virtual void SetLocalSSRC(uint32_t ssrc); virtual void SetRTCP_CNAME(const std::string& c_name); virtual void SetNACKStatus(bool enable, int max_packets); - virtual void SetSendAbsoluteSenderTimeStatus(bool enable, int id); virtual void SetSendAudioLevelIndicationStatus(bool enable, int id); - virtual void SetReceiveAbsoluteSenderTimeStatus(bool enable, int id); virtual void SetReceiveAudioLevelIndicationStatus(bool enable, int id); virtual void EnableSendTransportSequenceNumber(int id); virtual void EnableReceiveTransportSequenceNumber(int id); diff --git a/webrtc/voice_engine/include/voe_rtp_rtcp.h b/webrtc/voice_engine/include/voe_rtp_rtcp.h index e344d0488c..716566de32 100644 --- a/webrtc/voice_engine/include/voe_rtp_rtcp.h +++ b/webrtc/voice_engine/include/voe_rtp_rtcp.h @@ -133,16 +133,6 @@ class WEBRTC_DLLEXPORT VoERTP_RTCP { return 0; } - // Sets the status of sending absolute sender time on a specific |channel|. - virtual int SetSendAbsoluteSenderTimeStatus(int channel, - bool enable, - unsigned char id) = 0; - - // Sets status of receiving absolute sender time on a specific |channel|. - virtual int SetReceiveAbsoluteSenderTimeStatus(int channel, - bool enable, - unsigned char id) = 0; - // Sets the RTCP status on a specific |channel|. virtual int SetRTCPStatus(int channel, bool enable) = 0; diff --git a/webrtc/voice_engine/test/auto_test/standard/rtp_rtcp_extensions.cc b/webrtc/voice_engine/test/auto_test/standard/rtp_rtcp_extensions.cc index 16f17b1340..7b343cb1d4 100644 --- a/webrtc/voice_engine/test/auto_test/standard/rtp_rtcp_extensions.cc +++ b/webrtc/voice_engine/test/auto_test/standard/rtp_rtcp_extensions.cc @@ -118,40 +118,3 @@ TEST_F(SendRtpRtcpHeaderExtensionsTest, SentPacketsIncludeAudioLevel) { EXPECT_TRUE(verifying_transport_.Wait()); } -TEST_F(SendRtpRtcpHeaderExtensionsTest, SentPacketsIncludeNoAbsoluteSenderTime) -{ - verifying_transport_.SetAbsoluteSenderTimeId(0); - ResumePlaying(); - EXPECT_FALSE(verifying_transport_.Wait()); -} - -TEST_F(SendRtpRtcpHeaderExtensionsTest, SentPacketsIncludeAbsoluteSenderTime) { - EXPECT_EQ(0, voe_rtp_rtcp_->SetSendAbsoluteSenderTimeStatus(channel_, true, - 11)); - verifying_transport_.SetAbsoluteSenderTimeId(11); - ResumePlaying(); - EXPECT_TRUE(verifying_transport_.Wait()); -} - -TEST_F(SendRtpRtcpHeaderExtensionsTest, SentPacketsIncludeAllExtensions1) { - EXPECT_EQ(0, voe_rtp_rtcp_->SetSendAudioLevelIndicationStatus(channel_, true, - 9)); - EXPECT_EQ(0, voe_rtp_rtcp_->SetSendAbsoluteSenderTimeStatus(channel_, true, - 11)); - verifying_transport_.SetAudioLevelId(9); - verifying_transport_.SetAbsoluteSenderTimeId(11); - ResumePlaying(); - EXPECT_TRUE(verifying_transport_.Wait()); -} - -TEST_F(SendRtpRtcpHeaderExtensionsTest, SentPacketsIncludeAllExtensions2) { - EXPECT_EQ(0, voe_rtp_rtcp_->SetSendAbsoluteSenderTimeStatus(channel_, true, - 3)); - EXPECT_EQ(0, voe_rtp_rtcp_->SetSendAudioLevelIndicationStatus(channel_, true, - 9)); - verifying_transport_.SetAbsoluteSenderTimeId(3); - // Don't register audio level with header parser - unknown extensions should - // be ignored when parsing. - ResumePlaying(); - EXPECT_TRUE(verifying_transport_.Wait()); -} diff --git a/webrtc/voice_engine/voe_rtp_rtcp_impl.cc b/webrtc/voice_engine/voe_rtp_rtcp_impl.cc index 2a7359737a..44e571b232 100644 --- a/webrtc/voice_engine/voe_rtp_rtcp_impl.cc +++ b/webrtc/voice_engine/voe_rtp_rtcp_impl.cc @@ -141,7 +141,7 @@ int VoERTP_RTCPImpl::SetReceiveAudioLevelIndicationStatus(int channel, // the range 1-14 inclusive. _shared->SetLastError( VE_INVALID_ARGUMENT, kTraceError, - "SetReceiveAbsoluteSenderTimeStatus() invalid id parameter"); + "SetReceiveAudioLevelIndicationStatus() invalid id parameter"); return -1; } // Set state and id for the specified channel. @@ -156,69 +156,6 @@ int VoERTP_RTCPImpl::SetReceiveAudioLevelIndicationStatus(int channel, return channel_ptr->SetReceiveAudioLevelIndicationStatus(enable, id); } -int VoERTP_RTCPImpl::SetSendAbsoluteSenderTimeStatus(int channel, - bool enable, - unsigned char id) { - WEBRTC_TRACE(kTraceApiCall, kTraceVoice, VoEId(_shared->instance_id(), -1), - "SetSendAbsoluteSenderTimeStatus(channel=%d, enable=%d, id=%u)", - channel, enable, id); - if (!_shared->statistics().Initialized()) { - _shared->SetLastError(VE_NOT_INITED, kTraceError); - return -1; - } - if (enable && (id < kVoiceEngineMinRtpExtensionId || - id > kVoiceEngineMaxRtpExtensionId)) { - // [RFC5285] The 4-bit id is the local identifier of this element in - // the range 1-14 inclusive. - _shared->SetLastError( - VE_INVALID_ARGUMENT, kTraceError, - "SetSendAbsoluteSenderTimeStatus() invalid id parameter"); - return -1; - } - // Set state and id for the specified channel. - voe::ChannelOwner ch = _shared->channel_manager().GetChannel(channel); - voe::Channel* channelPtr = ch.channel(); - if (channelPtr == NULL) { - _shared->SetLastError( - VE_CHANNEL_NOT_VALID, kTraceError, - "SetSendAbsoluteSenderTimeStatus() failed to locate channel"); - return -1; - } - return channelPtr->SetSendAbsoluteSenderTimeStatus(enable, id); -} - -int VoERTP_RTCPImpl::SetReceiveAbsoluteSenderTimeStatus(int channel, - bool enable, - unsigned char id) { - WEBRTC_TRACE( - kTraceApiCall, kTraceVoice, VoEId(_shared->instance_id(), -1), - "SetReceiveAbsoluteSenderTimeStatus(channel=%d, enable=%d, id=%u)", - channel, enable, id); - if (!_shared->statistics().Initialized()) { - _shared->SetLastError(VE_NOT_INITED, kTraceError); - return -1; - } - if (enable && (id < kVoiceEngineMinRtpExtensionId || - id > kVoiceEngineMaxRtpExtensionId)) { - // [RFC5285] The 4-bit id is the local identifier of this element in - // the range 1-14 inclusive. - _shared->SetLastError( - VE_INVALID_ARGUMENT, kTraceError, - "SetReceiveAbsoluteSenderTimeStatus() invalid id parameter"); - return -1; - } - // Set state and id for the specified channel. - voe::ChannelOwner ch = _shared->channel_manager().GetChannel(channel); - voe::Channel* channelPtr = ch.channel(); - if (channelPtr == NULL) { - _shared->SetLastError( - VE_CHANNEL_NOT_VALID, kTraceError, - "SetReceiveAbsoluteSenderTimeStatus() failed to locate channel"); - return -1; - } - return channelPtr->SetReceiveAbsoluteSenderTimeStatus(enable, id); -} - int VoERTP_RTCPImpl::SetRTCPStatus(int channel, bool enable) { WEBRTC_TRACE(kTraceApiCall, kTraceVoice, VoEId(_shared->instance_id(), -1), "SetRTCPStatus(channel=%d, enable=%d)", channel, enable); diff --git a/webrtc/voice_engine/voe_rtp_rtcp_impl.h b/webrtc/voice_engine/voe_rtp_rtcp_impl.h index c1e84849b9..dcf28410a1 100644 --- a/webrtc/voice_engine/voe_rtp_rtcp_impl.h +++ b/webrtc/voice_engine/voe_rtp_rtcp_impl.h @@ -51,14 +51,6 @@ class VoERTP_RTCPImpl : public VoERTP_RTCP { bool enable, unsigned char id) override; - // RTP Header Extension for Absolute Sender Time - int SetSendAbsoluteSenderTimeStatus(int channel, - bool enable, - unsigned char id) override; - int SetReceiveAbsoluteSenderTimeStatus(int channel, - bool enable, - unsigned char id) override; - // Statistics int GetRTPStatistics(int channel, unsigned int& averageJitterMs,