From 24bd58e689a4eb3a0b45b965968e08cdcd0491e9 Mon Sep 17 00:00:00 2001 From: "andrew@webrtc.org" Date: Wed, 25 Jan 2012 18:57:44 +0000 Subject: [PATCH] Properly count anonymous mixing participants. When _amountOfMixableParticipants == 1, we skip mixing and saturation protection. Without this fix, an anonymous participant would only be properly counted if it was the last added. For example, if an anonymous participant was added first, followed by a regular participant, _amoutOfMixableParticipants would == 1 and the regular participant would not be mixed. BUG=issue209 TEST=New test added to voe_auto_test to verify, and used voe_cmd_test. Review URL: https://webrtc-codereview.appspot.com/367006 git-svn-id: http://webrtc.googlecode.com/svn/trunk@1551 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../interface/audio_conference_mixer.h | 4 - .../source/audio_conference_mixer_impl.cc | 30 +++--- .../source/audio_conference_mixer_impl.h | 4 +- .../main/test/auto_test/voe_extended_test.cc | 98 ++++++++++++------- .../main/test/auto_test/voe_extended_test.h | 3 +- 5 files changed, 81 insertions(+), 58 deletions(-) diff --git a/src/modules/audio_conference_mixer/interface/audio_conference_mixer.h b/src/modules/audio_conference_mixer/interface/audio_conference_mixer.h index 4914042df..4ece1bf39 100644 --- a/src/modules/audio_conference_mixer/interface/audio_conference_mixer.h +++ b/src/modules/audio_conference_mixer/interface/audio_conference_mixer.h @@ -62,10 +62,6 @@ public: virtual WebRtc_Word32 MixabilityStatus( MixerParticipant& participant, bool& mixable) = 0; - // amountOfMixableParticipants is set to the number of participants that are - // eligible for mixing. - virtual WebRtc_Word32 AmountOfMixables( - WebRtc_UWord32& amountOfMixableParticipants) = 0; // Inform the mixer that the participant should always be mixed and not // count toward the number of mixed participants. Note that a participant diff --git a/src/modules/audio_conference_mixer/source/audio_conference_mixer_impl.cc b/src/modules/audio_conference_mixer/source/audio_conference_mixer_impl.cc index e9fa7a1bd..6934c522a 100644 --- a/src/modules/audio_conference_mixer/source/audio_conference_mixer_impl.cc +++ b/src/modules/audio_conference_mixer/source/audio_conference_mixer_impl.cc @@ -106,7 +106,7 @@ AudioConferenceMixerImpl::AudioConferenceMixerImpl(int id) _audioFramePool(NULL), _participantList(), _additionalParticipantList(), - _amountOfMixableParticipants(0), + _numMixedParticipants(0), _timeStamp(0), _timeScheduler(kProcessPeriodicityInMs), _mixedAudioLevel(), @@ -525,7 +525,7 @@ WebRtc_Word32 AudioConferenceMixerImpl::SetMixabilityStatus( // participant is in the _participantList if it is being mixed. SetAnonymousMixabilityStatus(participant, false); } - WebRtc_UWord32 amountOfMixableParticipants; + WebRtc_UWord32 numMixedParticipants; { CriticalSectionScoped cs(_cbCrit.get()); const bool isMixed = @@ -555,13 +555,19 @@ WebRtc_Word32 AudioConferenceMixerImpl::SetMixabilityStatus( assert(false); return -1; } - amountOfMixableParticipants = _participantList.GetSize(); + + const int numMixedNonAnonymous = (_participantList.GetSize() < + kMaximumAmountOfMixedParticipants) ? _participantList.GetSize() : + kMaximumAmountOfMixedParticipants; + + numMixedParticipants = numMixedNonAnonymous + + _additionalParticipantList.GetSize(); } // A MixerParticipant was added or removed. Make sure the scratch // buffer is updated if necessary. // Note: The scratch buffer may only be updated in Process(). CriticalSectionScoped cs(_crit.get()); - _amountOfMixableParticipants = amountOfMixableParticipants; + _numMixedParticipants = numMixedParticipants; return 0; } @@ -576,16 +582,6 @@ WebRtc_Word32 AudioConferenceMixerImpl::MixabilityStatus( return 0; } -WebRtc_Word32 AudioConferenceMixerImpl::AmountOfMixables( - WebRtc_UWord32& amountOfMixableParticipants) -{ - WEBRTC_TRACE(kTraceModuleCall, kTraceAudioMixerServer, _id, - "AmountOfMixables(amountOfMixableParticipants)"); - CriticalSectionScoped cs(_crit.get()); - amountOfMixableParticipants = _amountOfMixableParticipants; - return 0; -} - WebRtc_Word32 AudioConferenceMixerImpl::SetAnonymousMixabilityStatus( MixerParticipant& participant, const bool anonymous) { @@ -1104,7 +1100,7 @@ WebRtc_Word32 AudioConferenceMixerImpl::MixFromList( return 0; } - if(_amountOfMixableParticipants == 1) + if(_numMixedParticipants == 1) { // No mixing required here; skip the saturation protection. AudioFrame* audioFrame = static_cast(item->GetItem()); @@ -1155,7 +1151,7 @@ WebRtc_Word32 AudioConferenceMixerImpl::MixAnonomouslyFromList( if(item == NULL) return 0; - if(_amountOfMixableParticipants == 1) + if(_numMixedParticipants == 1) { // No mixing required here; skip the saturation protection. AudioFrame* audioFrame = static_cast(item->GetItem()); @@ -1176,7 +1172,7 @@ WebRtc_Word32 AudioConferenceMixerImpl::MixAnonomouslyFromList( bool AudioConferenceMixerImpl::LimitMixedAudio(AudioFrame& mixedAudio) { - if(_amountOfMixableParticipants == 1) + if(_numMixedParticipants == 1) { return true; } diff --git a/src/modules/audio_conference_mixer/source/audio_conference_mixer_impl.h b/src/modules/audio_conference_mixer/source/audio_conference_mixer_impl.h index 04a433c13..efa5d68ae 100644 --- a/src/modules/audio_conference_mixer/source/audio_conference_mixer_impl.h +++ b/src/modules/audio_conference_mixer/source/audio_conference_mixer_impl.h @@ -77,8 +77,6 @@ public: virtual WebRtc_Word32 MixabilityStatus(MixerParticipant& participant, bool& mixable); virtual WebRtc_Word32 SetMinimumMixingFrequency(Frequency freq); - virtual WebRtc_Word32 AmountOfMixables( - WebRtc_UWord32& amountOfMixableParticipants); virtual WebRtc_Word32 SetAnonymousMixabilityStatus( MixerParticipant& participant, const bool mixable); virtual WebRtc_Word32 AnonymousMixabilityStatus( @@ -189,7 +187,7 @@ private: ListWrapper _participantList; // May be mixed. ListWrapper _additionalParticipantList; // Always mixed, anonomously. - WebRtc_UWord32 _amountOfMixableParticipants; + WebRtc_UWord32 _numMixedParticipants; WebRtc_UWord32 _timeStamp; diff --git a/src/voice_engine/main/test/auto_test/voe_extended_test.cc b/src/voice_engine/main/test/auto_test/voe_extended_test.cc index df9578cc4..8b4ad455b 100644 --- a/src/voice_engine/main/test/auto_test/voe_extended_test.cc +++ b/src/voice_engine/main/test/auto_test/voe_extended_test.cc @@ -4762,10 +4762,14 @@ int VoEExtendedTest::TestFile() { // VoEExtendedTest::TestMixing // ---------------------------------------------------------------------------- -// Creates and mixes |num_channels| with a constant amplitude of |input_value|. +// Creates and mixes |num_remote_channels| which play a file "as microphone" +// with |num_local_channels| which play a file "locally", using a constant +// amplitude of |input_value|. +// // The mixed output is verified to always fall between |max_output_value| and // |min_output_value|, after a startup phase. -int VoEExtendedTest::RunMixingTest(int num_channels, +int VoEExtendedTest::RunMixingTest(int num_remote_channels, + int num_local_channels, int16_t input_value, int16_t max_output_value, int16_t min_output_value) { @@ -4799,43 +4803,54 @@ int VoEExtendedTest::RunMixingTest(int num_channels, TEST_MUSTPASS(voe_base_->Init()); - std::vector channels(num_channels); - for (int channel_index = 0; channel_index < num_channels; ++channel_index) { - const int channel = voe_base_->CreateChannel(); - channels[channel_index] = channel; - ASSERT_TRUE(channel != -1); - TEST_MUSTPASS(codec->SetRecPayloadType(channel, codec_inst)); - TEST_MUSTPASS(voe_base_->SetLocalReceiver(channel, - 1234 + 2 * channel_index)); - TEST_MUSTPASS(voe_base_->SetSendDestination(channel, - 1234 + 2 * channel_index, - "127.0.0.1")); - TEST_MUSTPASS(voe_base_->StartReceive(channel)); - TEST_MUSTPASS(voe_base_->StartPlayout(channel)); - TEST_MUSTPASS(codec->SetSendCodec(channel, codec_inst)); - TEST_MUSTPASS(voe_base_->StartSend(channel)); + std::vector local_channels(num_local_channels); + for (int i = 0; i < num_local_channels; ++i) { + local_channels[i] = voe_base_->CreateChannel(); + ASSERT_TRUE(local_channels[i] != -1); + TEST_MUSTPASS(voe_base_->StartPlayout(local_channels[i])); + TEST_MUSTPASS(file->StartPlayingFileLocally(local_channels[i], + input_filename, + true)); } - for (int channel_index = 0; channel_index < num_channels; ++channel_index) { - const int channel = channels[channel_index]; - TEST_MUSTPASS(file->StartPlayingFileAsMicrophone(channel, - input_filename, - true)); + + std::vector remote_channels(num_remote_channels); + for (int i = 0; i < num_remote_channels; ++i) { + remote_channels[i] = voe_base_->CreateChannel(); + ASSERT_TRUE(remote_channels[i] != -1); + TEST_MUSTPASS(codec->SetRecPayloadType(remote_channels[i], codec_inst)); + TEST_MUSTPASS(voe_base_->SetLocalReceiver(remote_channels[i], + 1234 + 2 * i)); + TEST_MUSTPASS(voe_base_->SetSendDestination(remote_channels[i], + 1234 + 2 * i, + "127.0.0.1")); + TEST_MUSTPASS(voe_base_->StartReceive(remote_channels[i])); + TEST_MUSTPASS(voe_base_->StartPlayout(remote_channels[i])); + TEST_MUSTPASS(codec->SetSendCodec(remote_channels[i], codec_inst)); + TEST_MUSTPASS(voe_base_->StartSend(remote_channels[i])); + TEST_MUSTPASS(file->StartPlayingFileAsMicrophone(remote_channels[i], + input_filename, + true)); } + const char mix_result[] = "mix_result.pcm"; TEST_MUSTPASS(file->StartRecordingPlayout(-1/*record meeting*/, mix_result)); - TEST_LOG("Playing %d channels\n", num_channels); + TEST_LOG("Playing %d remote channels.\n", num_remote_channels); + TEST_LOG("Playing %d local channels.\n", num_local_channels); SLEEP(5000); TEST_MUSTPASS(file->StopRecordingPlayout(-1)); TEST_LOG("Stopping\n"); - for (int channel_index = 0; channel_index < num_channels; ++channel_index) { - const int channel = channels[channel_index]; - channels[channel_index] = channel; - TEST_MUSTPASS(voe_base_->StopSend(channel)); - TEST_MUSTPASS(voe_base_->StopPlayout(channel)); - TEST_MUSTPASS(voe_base_->StopReceive(channel)); - TEST_MUSTPASS(voe_base_->DeleteChannel(channel)); + for (int i = 0; i < num_local_channels; ++i) { + TEST_MUSTPASS(voe_base_->StopPlayout(local_channels[i])); + TEST_MUSTPASS(voe_base_->DeleteChannel(local_channels[i])); + } + + for (int i = 0; i < num_remote_channels; ++i) { + TEST_MUSTPASS(voe_base_->StopSend(remote_channels[i])); + TEST_MUSTPASS(voe_base_->StopPlayout(remote_channels[i])); + TEST_MUSTPASS(voe_base_->StopReceive(remote_channels[i])); + TEST_MUSTPASS(voe_base_->DeleteChannel(remote_channels[i])); } FILE* verification_file = fopen(mix_result, "rb"); @@ -4862,7 +4877,7 @@ int VoEExtendedTest::TestMixing() { TEST_LOG("Test max-three-participant mixing.\n"); int16_t input_value = 1000; int16_t expected_output = input_value * 3; - if (RunMixingTest(4, input_value, 1.1 * expected_output, + if (RunMixingTest(4, 0, input_value, 1.1 * expected_output, 0.9 * expected_output) != 0) { return -1; } @@ -4876,7 +4891,7 @@ int VoEExtendedTest::TestMixing() { // If this isn't satisfied, we're not testing anything. assert(input_value * 3 > 32767); assert(1.1 * expected_output < 32767); - if (RunMixingTest(3, input_value, 1.1 * expected_output, + if (RunMixingTest(3, 0, input_value, 1.1 * expected_output, 0.9 * expected_output) != 0) { return -1; } @@ -4888,11 +4903,28 @@ int VoEExtendedTest::TestMixing() { expected_output = 32767; // If this isn't satisfied, we're not testing anything. assert(0.95 * expected_output > 29204); // = -1 dBFS, the limiter headroom. - if (RunMixingTest(1, input_value, expected_output, + if (RunMixingTest(1, 0, input_value, expected_output, 0.95 * expected_output) != 0) { return -1; } + TEST_LOG("Test combinations of 'anonymous' participants and regular " + "participants.\n"); + input_value = 1000; + expected_output = input_value * 2; + if (RunMixingTest(1, 1, input_value, 1.1 * expected_output, + 0.9 * expected_output) != 0) { + + return -1; + } + + expected_output = input_value * 4; + if (RunMixingTest(3, 1, input_value, 1.1 * expected_output, + 0.9 * expected_output) != 0) { + + return -1; + } + return 0; } diff --git a/src/voice_engine/main/test/auto_test/voe_extended_test.h b/src/voice_engine/main/test/auto_test/voe_extended_test.h index 0b53d09bd..9a1003730 100644 --- a/src/voice_engine/main/test/auto_test/voe_extended_test.h +++ b/src/voice_engine/main/test/auto_test/voe_extended_test.h @@ -446,7 +446,8 @@ class VoEExtendedTest : public VoiceEngineObserver, void Sleep(unsigned int timeMillisec, bool addMarker = false); void StartMedia(int channel, int rtpPort, bool listen, bool playout, bool send); void StopMedia(int channel); - int RunMixingTest(int num_channels, int16_t input_value, int16_t max_output_value, + int RunMixingTest(int num_remote_channels, int num_local_channels, + int16_t input_value, int16_t max_output_value, int16_t min_output_value); private: VoETestManager& _mgr;