From 4bac9c53da9988741d59753c2d789adb94de5e68 Mon Sep 17 00:00:00 2001 From: solenberg Date: Fri, 9 Oct 2015 02:32:53 -0700 Subject: [PATCH] Change SetOutputScaling to set a single level, not left/right levels. BUG=webrtc:4690 Review URL: https://codereview.webrtc.org/1397773002 Cr-Commit-Position: refs/heads/master@{#10234} --- talk/app/webrtc/webrtcsession.cc | 6 +- talk/app/webrtc/webrtcsession_unittest.cc | 17 ++-- talk/media/base/fakemediaengine.h | 26 ++---- talk/media/base/mediachannel.h | 4 +- talk/media/webrtc/fakewebrtcvoiceengine.h | 18 +--- talk/media/webrtc/webrtcvoiceengine.cc | 24 +---- talk/media/webrtc/webrtcvoiceengine.h | 2 +- .../webrtc/webrtcvoiceengine_unittest.cc | 18 ++-- talk/session/media/channel.cc | 6 +- talk/session/media/channel.h | 4 +- talk/session/media/channel_unittest.cc | 88 ++++++++----------- 11 files changed, 78 insertions(+), 135 deletions(-) diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc index 2ab9a1e696..10b607caa3 100644 --- a/talk/app/webrtc/webrtcsession.cc +++ b/talk/app/webrtc/webrtcsession.cc @@ -1246,8 +1246,8 @@ void WebRtcSession::SetAudioPlayout(uint32_t ssrc, LOG(LS_ERROR) << "SetAudioPlayout: ssrc is incorrect: " << ssrc; return; } - if (!voice_channel_->SetOutputScaling(ssrc, enable ? 1 : 0, enable ? 1 : 0)) { - // Allow that SetOutputScaling fail if |enable| is false but assert + if (!voice_channel_->SetOutputVolume(ssrc, enable ? 1 : 0)) { + // Allow that SetOutputVolume fail if |enable| is false but assert // otherwise. This in the normal case when the underlying media channel has // already been deleted. ASSERT(enable == false); @@ -1276,7 +1276,7 @@ void WebRtcSession::SetAudioPlayoutVolume(uint32_t ssrc, double volume) { return; } - if (!voice_channel_->SetOutputScaling(ssrc, volume, volume)) { + if (!voice_channel_->SetOutputVolume(ssrc, volume)) { ASSERT(false); } } diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc index 2853ca43a7..ab8c14cb26 100644 --- a/talk/app/webrtc/webrtcsession_unittest.cc +++ b/talk/app/webrtc/webrtcsession_unittest.cc @@ -3108,20 +3108,17 @@ TEST_F(WebRtcSessionTest, SetAudioPlayout) { ASSERT_TRUE(channel != NULL); ASSERT_EQ(1u, channel->recv_streams().size()); uint32_t receive_ssrc = channel->recv_streams()[0].first_ssrc(); - double left_vol, right_vol; - EXPECT_TRUE(channel->GetOutputScaling(receive_ssrc, &left_vol, &right_vol)); - EXPECT_EQ(1, left_vol); - EXPECT_EQ(1, right_vol); + double volume; + EXPECT_TRUE(channel->GetOutputVolume(receive_ssrc, &volume)); + EXPECT_EQ(1, volume); rtc::scoped_ptr renderer(new FakeAudioRenderer()); session_->SetAudioPlayout(receive_ssrc, false, renderer.get()); - EXPECT_TRUE(channel->GetOutputScaling(receive_ssrc, &left_vol, &right_vol)); - EXPECT_EQ(0, left_vol); - EXPECT_EQ(0, right_vol); + EXPECT_TRUE(channel->GetOutputVolume(receive_ssrc, &volume)); + EXPECT_EQ(0, volume); EXPECT_EQ(0, renderer->channel_id()); session_->SetAudioPlayout(receive_ssrc, true, NULL); - EXPECT_TRUE(channel->GetOutputScaling(receive_ssrc, &left_vol, &right_vol)); - EXPECT_EQ(1, left_vol); - EXPECT_EQ(1, right_vol); + EXPECT_TRUE(channel->GetOutputVolume(receive_ssrc, &volume)); + EXPECT_EQ(1, volume); EXPECT_EQ(-1, renderer->channel_id()); } diff --git a/talk/media/base/fakemediaengine.h b/talk/media/base/fakemediaengine.h index 7325667aa5..4296ec1e85 100644 --- a/talk/media/base/fakemediaengine.h +++ b/talk/media/base/fakemediaengine.h @@ -243,7 +243,7 @@ class FakeVoiceMediaChannel : public RtpHelper { const AudioOptions& options) : engine_(engine), time_since_last_typing_(-1) { - output_scalings_[0] = OutputScaling(); // For default channel. + output_scalings_[0] = 1.0; // For default channel. SetOptions(options); } ~FakeVoiceMediaChannel(); @@ -291,7 +291,7 @@ class FakeVoiceMediaChannel : public RtpHelper { virtual bool AddRecvStream(const StreamParams& sp) { if (!RtpHelper::AddRecvStream(sp)) return false; - output_scalings_[sp.first_ssrc()] = OutputScaling(); + output_scalings_[sp.first_ssrc()] = 1.0; return true; } virtual bool RemoveRecvStream(uint32_t ssrc) { @@ -347,37 +347,29 @@ class FakeVoiceMediaChannel : public RtpHelper { return true; } - virtual bool SetOutputScaling(uint32_t ssrc, double left, double right) { + virtual bool SetOutputVolume(uint32_t ssrc, double volume) { if (0 == ssrc) { - std::map::iterator it; + std::map::iterator it; for (it = output_scalings_.begin(); it != output_scalings_.end(); ++it) { - it->second.left = left; - it->second.right = right; + it->second = volume; } return true; } else if (output_scalings_.find(ssrc) != output_scalings_.end()) { - output_scalings_[ssrc].left = left; - output_scalings_[ssrc].right = right; + output_scalings_[ssrc] = volume; return true; } return false; } - bool GetOutputScaling(uint32_t ssrc, double* left, double* right) { + bool GetOutputVolume(uint32_t ssrc, double* volume) { if (output_scalings_.find(ssrc) == output_scalings_.end()) return false; - *left = output_scalings_[ssrc].left; - *right = output_scalings_[ssrc].right; + *volume = output_scalings_[ssrc]; return true; } virtual bool GetStats(VoiceMediaInfo* info) { return false; } private: - struct OutputScaling { - OutputScaling() : left(1.0), right(1.0) {} - double left, right; - }; - class VoiceChannelAudioSink : public AudioRenderer::Sink { public: explicit VoiceChannelAudioSink(AudioRenderer* renderer) @@ -446,7 +438,7 @@ class FakeVoiceMediaChannel : public RtpHelper { FakeVoiceEngine* engine_; std::vector recv_codecs_; std::vector send_codecs_; - std::map output_scalings_; + std::map output_scalings_; std::vector dtmf_info_queue_; int time_since_last_typing_; AudioOptions options_; diff --git a/talk/media/base/mediachannel.h b/talk/media/base/mediachannel.h index 693fef0822..ed49961bd7 100644 --- a/talk/media/base/mediachannel.h +++ b/talk/media/base/mediachannel.h @@ -1067,8 +1067,8 @@ class VoiceMediaChannel : public MediaChannel { virtual void SetTypingDetectionParameters(int time_window, int cost_per_typing, int reporting_threshold, int penalty_decay, int type_event_delay) = 0; - // Set left and right scale for speaker output volume of the specified ssrc. - virtual bool SetOutputScaling(uint32_t ssrc, double left, double right) = 0; + // Set speaker output volume of the specified ssrc. + virtual bool SetOutputVolume(uint32_t ssrc, double volume) = 0; // Returns if the telephone-event has been negotiated. virtual bool CanInsertDtmf() { return false; } // Send and/or play a DTMF |event| according to the |flags|. diff --git a/talk/media/webrtc/fakewebrtcvoiceengine.h b/talk/media/webrtc/fakewebrtcvoiceengine.h index 10b5357eae..1167b6b983 100644 --- a/talk/media/webrtc/fakewebrtcvoiceengine.h +++ b/talk/media/webrtc/fakewebrtcvoiceengine.h @@ -200,8 +200,6 @@ class FakeWebRtcVoiceEngine send(false), playout(false), volume_scale(1.0), - volume_pan_left(1.0), - volume_pan_right(1.0), vad(false), codec_fec(false), max_encoding_bandwidth(0), @@ -227,8 +225,6 @@ class FakeWebRtcVoiceEngine bool send; bool playout; float volume_scale; - float volume_pan_left; - float volume_pan_right; bool vad; bool codec_fec; int max_encoding_bandwidth; @@ -924,18 +920,8 @@ class FakeWebRtcVoiceEngine scale = channels_[channel]->volume_scale; return 0; } - WEBRTC_FUNC(SetOutputVolumePan, (int channel, float left, float right)) { - WEBRTC_CHECK_CHANNEL(channel); - channels_[channel]->volume_pan_left = left; - channels_[channel]->volume_pan_right = right; - return 0; - } - WEBRTC_FUNC(GetOutputVolumePan, (int channel, float& left, float& right)) { - WEBRTC_CHECK_CHANNEL(channel); - left = channels_[channel]->volume_pan_left; - right = channels_[channel]->volume_pan_right; - return 0; - } + WEBRTC_STUB(SetOutputVolumePan, (int channel, float left, float right)); + WEBRTC_STUB(GetOutputVolumePan, (int channel, float& left, float& right)); // webrtc::VoEAudioProcessing WEBRTC_FUNC(SetNsStatus, (bool enable, webrtc::NsModes mode)) { diff --git a/talk/media/webrtc/webrtcvoiceengine.cc b/talk/media/webrtc/webrtcvoiceengine.cc index 89397b4822..a10c6f8aec 100644 --- a/talk/media/webrtc/webrtcvoiceengine.cc +++ b/talk/media/webrtc/webrtcvoiceengine.cc @@ -2491,9 +2491,7 @@ void WebRtcVoiceMediaChannel::SetTypingDetectionParameters(int time_window, } } -bool WebRtcVoiceMediaChannel::SetOutputScaling(uint32_t ssrc, - double left, - double right) { +bool WebRtcVoiceMediaChannel::SetOutputVolume(uint32_t ssrc, double volume) { RTC_DCHECK(thread_checker_.CalledOnValidThread()); rtc::CritScope lock(&receive_channels_cs_); // Collect the channels to scale the output volume. @@ -2515,27 +2513,13 @@ bool WebRtcVoiceMediaChannel::SetOutputScaling(uint32_t ssrc, channels.push_back(channel); } - // Scale the output volume for the collected channels. We first normalize to - // scale the volume and then set the left and right pan. - float scale = static_cast(std::max(left, right)); - if (scale > 0.0001f) { - left /= scale; - right /= scale; - } for (int ch_id : channels) { if (-1 == engine()->voe()->volume()->SetChannelOutputVolumeScaling( - ch_id, scale)) { - LOG_RTCERR2(SetChannelOutputVolumeScaling, ch_id, scale); + ch_id, volume)) { + LOG_RTCERR2(SetChannelOutputVolumeScaling, ch_id, volume); return false; } - if (-1 == engine()->voe()->volume()->SetOutputVolumePan( - ch_id, static_cast(left), static_cast(right))) { - LOG_RTCERR3(SetOutputVolumePan, ch_id, left, right); - // Do not return if fails. SetOutputVolumePan is not available for all - // pltforms. - } - LOG(LS_INFO) << "SetOutputScaling to left=" << left * scale - << " right=" << right * scale + LOG(LS_INFO) << "SetOutputVolume to " << volume << " for channel " << ch_id << " and ssrc " << ssrc; } return true; diff --git a/talk/media/webrtc/webrtcvoiceengine.h b/talk/media/webrtc/webrtcvoiceengine.h index 269920523f..5af8088f14 100644 --- a/talk/media/webrtc/webrtcvoiceengine.h +++ b/talk/media/webrtc/webrtcvoiceengine.h @@ -209,7 +209,7 @@ class WebRtcVoiceMediaChannel : public VoiceMediaChannel, int reporting_threshold, int penalty_decay, int type_event_delay) override; - bool SetOutputScaling(uint32_t ssrc, double left, double right) override; + bool SetOutputVolume(uint32_t ssrc, double volume) override; bool CanInsertDtmf() override; bool InsertDtmf(uint32_t ssrc, int event, int duration, int flags) override; diff --git a/talk/media/webrtc/webrtcvoiceengine_unittest.cc b/talk/media/webrtc/webrtcvoiceengine_unittest.cc index 413a969438..3801a208e1 100644 --- a/talk/media/webrtc/webrtcvoiceengine_unittest.cc +++ b/talk/media/webrtc/webrtcvoiceengine_unittest.cc @@ -2880,27 +2880,23 @@ TEST_F(WebRtcVoiceEngineTestFake, TestGetChannelNumInConferenceCalls) { media_channel->GetReceiveChannelId(kSsrc2)); } -TEST_F(WebRtcVoiceEngineTestFake, SetOutputScaling) { +TEST_F(WebRtcVoiceEngineTestFake, SetOutputVolume) { EXPECT_TRUE(SetupEngine()); - float scale, left, right; - EXPECT_TRUE(channel_->SetOutputScaling(0, 1, 2)); + float scale; + EXPECT_TRUE(channel_->SetOutputVolume(0, 2)); int channel_id = voe_.GetLastChannel(); EXPECT_EQ(0, voe_.GetChannelOutputVolumeScaling(channel_id, scale)); - EXPECT_EQ(0, voe_.GetOutputVolumePan(channel_id, left, right)); - EXPECT_DOUBLE_EQ(1, left * scale); - EXPECT_DOUBLE_EQ(2, right * scale); + EXPECT_DOUBLE_EQ(2, scale); - EXPECT_FALSE(channel_->SetOutputScaling(kSsrc2, 1, 2)); + EXPECT_FALSE(channel_->SetOutputVolume(kSsrc2, 0.5)); cricket::StreamParams stream; stream.ssrcs.push_back(kSsrc2); EXPECT_TRUE(channel_->AddRecvStream(stream)); - EXPECT_TRUE(channel_->SetOutputScaling(kSsrc2, 2, 1)); + EXPECT_TRUE(channel_->SetOutputVolume(kSsrc2, 3)); channel_id = voe_.GetLastChannel(); EXPECT_EQ(0, voe_.GetChannelOutputVolumeScaling(channel_id, scale)); - EXPECT_EQ(0, voe_.GetOutputVolumePan(channel_id, left, right)); - EXPECT_DOUBLE_EQ(2, left * scale); - EXPECT_DOUBLE_EQ(1, right * scale); + EXPECT_DOUBLE_EQ(3, scale); } TEST_F(WebRtcVoiceEngineTestFake, SetsSyncGroupFromSyncLabel) { diff --git a/talk/session/media/channel.cc b/talk/session/media/channel.cc index 57d09a2d6f..5a7ba607cd 100644 --- a/talk/session/media/channel.cc +++ b/talk/session/media/channel.cc @@ -1347,9 +1347,9 @@ bool VoiceChannel::InsertDtmf(uint32_t ssrc, ssrc, event_code, duration, flags)); } -bool VoiceChannel::SetOutputScaling(uint32_t ssrc, double left, double right) { - return InvokeOnWorker(Bind(&VoiceMediaChannel::SetOutputScaling, - media_channel(), ssrc, left, right)); +bool VoiceChannel::SetOutputVolume(uint32_t ssrc, double volume) { + return InvokeOnWorker(Bind(&VoiceMediaChannel::SetOutputVolume, + media_channel(), ssrc, volume)); } bool VoiceChannel::GetStats(VoiceMediaInfo* stats) { diff --git a/talk/session/media/channel.h b/talk/session/media/channel.h index c557ef35b5..36cc6b0464 100644 --- a/talk/session/media/channel.h +++ b/talk/session/media/channel.h @@ -365,7 +365,7 @@ class VoiceChannel : public BaseChannel { // The valid value for the |event| are 0 which corresponding to DTMF // event 0-9, *, #, A-D. bool InsertDtmf(uint32_t ssrc, int event_code, int duration, int flags); - bool SetOutputScaling(uint32_t ssrc, double left, double right); + bool SetOutputVolume(uint32_t ssrc, double volume); // Get statistics about the current media session. bool GetStats(VoiceMediaInfo* stats); @@ -402,7 +402,7 @@ class VoiceChannel : public BaseChannel { std::string* error_desc); void HandleEarlyMediaTimeout(); bool InsertDtmf_w(uint32_t ssrc, int event, int duration, int flags); - bool SetOutputScaling_w(uint32_t ssrc, double left, double right); + bool SetOutputVolume_w(uint32_t ssrc, double volume); bool GetStats_w(VoiceMediaInfo* stats); virtual void OnMessage(rtc::Message* pmsg); diff --git a/talk/session/media/channel_unittest.cc b/talk/session/media/channel_unittest.cc index b861d0a4ef..1b14cdac9e 100644 --- a/talk/session/media/channel_unittest.cc +++ b/talk/session/media/channel_unittest.cc @@ -2189,26 +2189,23 @@ TEST_F(VoiceChannelTest, TestScaleVolume1to1Call) { CreateChannels(RTCP, RTCP); EXPECT_TRUE(SendInitiate()); EXPECT_TRUE(SendAccept()); - double left, right; + double volume; - // Default is (1.0, 1.0). - EXPECT_TRUE(media_channel1_->GetOutputScaling(0, &left, &right)); - EXPECT_DOUBLE_EQ(1.0, left); - EXPECT_DOUBLE_EQ(1.0, right); + // Default is (1.0). + EXPECT_TRUE(media_channel1_->GetOutputVolume(0, &volume)); + EXPECT_DOUBLE_EQ(1.0, volume); // invalid ssrc. - EXPECT_FALSE(media_channel1_->GetOutputScaling(3, &left, &right)); + EXPECT_FALSE(media_channel1_->GetOutputVolume(3, &volume)); - // Set scale to (1.5, 0.5). - EXPECT_TRUE(channel1_->SetOutputScaling(0, 1.5, 0.5)); - EXPECT_TRUE(media_channel1_->GetOutputScaling(0, &left, &right)); - EXPECT_DOUBLE_EQ(1.5, left); - EXPECT_DOUBLE_EQ(0.5, right); + // Set scale to (1.5). + EXPECT_TRUE(channel1_->SetOutputVolume(0, 1.5)); + EXPECT_TRUE(media_channel1_->GetOutputVolume(0, &volume)); + EXPECT_DOUBLE_EQ(1.5, volume); - // Set scale to (0, 0). - EXPECT_TRUE(channel1_->SetOutputScaling(0, 0.0, 0.0)); - EXPECT_TRUE(media_channel1_->GetOutputScaling(0, &left, &right)); - EXPECT_DOUBLE_EQ(0.0, left); - EXPECT_DOUBLE_EQ(0.0, right); + // Set scale to (0). + EXPECT_TRUE(channel1_->SetOutputVolume(0, 0.0)); + EXPECT_TRUE(media_channel1_->GetOutputVolume(0, &volume)); + EXPECT_DOUBLE_EQ(0.0, volume); } // Test that we can scale the output volume properly for multiway calls. @@ -2219,43 +2216,34 @@ TEST_F(VoiceChannelTest, TestScaleVolumeMultiwayCall) { EXPECT_TRUE(AddStream1(1)); EXPECT_TRUE(AddStream1(2)); - double left, right; - // Default is (1.0, 1.0). - EXPECT_TRUE(media_channel1_->GetOutputScaling(0, &left, &right)); - EXPECT_DOUBLE_EQ(1.0, left); - EXPECT_DOUBLE_EQ(1.0, right); - EXPECT_TRUE(media_channel1_->GetOutputScaling(1, &left, &right)); - EXPECT_DOUBLE_EQ(1.0, left); - EXPECT_DOUBLE_EQ(1.0, right); - EXPECT_TRUE(media_channel1_->GetOutputScaling(2, &left, &right)); - EXPECT_DOUBLE_EQ(1.0, left); - EXPECT_DOUBLE_EQ(1.0, right); + double volume; + // Default is (1.0). + EXPECT_TRUE(media_channel1_->GetOutputVolume(0, &volume)); + EXPECT_DOUBLE_EQ(1.0, volume); + EXPECT_TRUE(media_channel1_->GetOutputVolume(1, &volume)); + EXPECT_DOUBLE_EQ(1.0, volume); + EXPECT_TRUE(media_channel1_->GetOutputVolume(2, &volume)); + EXPECT_DOUBLE_EQ(1.0, volume); // invalid ssrc. - EXPECT_FALSE(media_channel1_->GetOutputScaling(3, &left, &right)); + EXPECT_FALSE(media_channel1_->GetOutputVolume(3, &volume)); - // Set scale to (1.5, 0.5) for ssrc = 1. - EXPECT_TRUE(channel1_->SetOutputScaling(1, 1.5, 0.5)); - EXPECT_TRUE(media_channel1_->GetOutputScaling(1, &left, &right)); - EXPECT_DOUBLE_EQ(1.5, left); - EXPECT_DOUBLE_EQ(0.5, right); - EXPECT_TRUE(media_channel1_->GetOutputScaling(2, &left, &right)); - EXPECT_DOUBLE_EQ(1.0, left); - EXPECT_DOUBLE_EQ(1.0, right); - EXPECT_TRUE(media_channel1_->GetOutputScaling(0, &left, &right)); - EXPECT_DOUBLE_EQ(1.0, left); - EXPECT_DOUBLE_EQ(1.0, right); + // Set scale to (1.5) for ssrc = 1. + EXPECT_TRUE(channel1_->SetOutputVolume(1, 1.5)); + EXPECT_TRUE(media_channel1_->GetOutputVolume(1, &volume)); + EXPECT_DOUBLE_EQ(1.5, volume); + EXPECT_TRUE(media_channel1_->GetOutputVolume(2, &volume)); + EXPECT_DOUBLE_EQ(1.0, volume); + EXPECT_TRUE(media_channel1_->GetOutputVolume(0, &volume)); + EXPECT_DOUBLE_EQ(1.0, volume); - // Set scale to (0, 0) for all ssrcs. - EXPECT_TRUE(channel1_->SetOutputScaling(0, 0.0, 0.0)); - EXPECT_TRUE(media_channel1_->GetOutputScaling(0, &left, &right)); - EXPECT_DOUBLE_EQ(0.0, left); - EXPECT_DOUBLE_EQ(0.0, right); - EXPECT_TRUE(media_channel1_->GetOutputScaling(1, &left, &right)); - EXPECT_DOUBLE_EQ(0.0, left); - EXPECT_DOUBLE_EQ(0.0, right); - EXPECT_TRUE(media_channel1_->GetOutputScaling(2, &left, &right)); - EXPECT_DOUBLE_EQ(0.0, left); - EXPECT_DOUBLE_EQ(0.0, right); + // Set scale to (0) for all ssrcs. + EXPECT_TRUE(channel1_->SetOutputVolume(0, 0.0)); + EXPECT_TRUE(media_channel1_->GetOutputVolume(0, &volume)); + EXPECT_DOUBLE_EQ(0.0, volume); + EXPECT_TRUE(media_channel1_->GetOutputVolume(1, &volume)); + EXPECT_DOUBLE_EQ(0.0, volume); + EXPECT_TRUE(media_channel1_->GetOutputVolume(2, &volume)); + EXPECT_DOUBLE_EQ(0.0, volume); } TEST_F(VoiceChannelTest, SendBundleToBundle) {