From 6a61762659e8578b3072d9189311a4d2cfe5fb93 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Wed, 26 Oct 2016 17:41:34 -0700 Subject: [PATCH 1/5] First try AudioMixer has same issue transitioning from silence to encoded audio and back, it seems. This makes that go away, but I don't like it. Better to support a flush function in the encoder but for now lets do this and be sure it is the entire issue. --- assignment-client/src/audio/AudioMixer.cpp | 6 ++++++ assignment-client/src/audio/AudioMixerClientData.cpp | 10 ++++++++++ assignment-client/src/audio/AudioMixerClientData.h | 4 ++++ 3 files changed, 20 insertions(+) diff --git a/assignment-client/src/audio/AudioMixer.cpp b/assignment-client/src/audio/AudioMixer.cpp index 0252c037bf..e580c62905 100644 --- a/assignment-client/src/audio/AudioMixer.cpp +++ b/assignment-client/src/audio/AudioMixer.cpp @@ -819,6 +819,9 @@ void AudioMixer::broadcastMixes() { std::unique_ptr mixPacket; if (mixHasAudio) { + // may need to flush, this checks + nodeData->flushEncoder(); + int mixPacketBytes = sizeof(quint16) + AudioConstants::MAX_CODEC_NAME_LENGTH_ON_WIRE + AudioConstants::NETWORK_FRAME_BYTES_STEREO; mixPacket = NLPacket::create(PacketType::MixedAudio, mixPacketBytes); @@ -852,6 +855,9 @@ void AudioMixer::broadcastMixes() { // pack number of silent audio samples quint16 numSilentSamples = AudioConstants::NETWORK_FRAME_SAMPLES_STEREO; mixPacket->writePrimitive(numSilentSamples); + + // we will need to flush encoder since we are sending silent packets outside it + nodeData->shouldFlushEncoder(); } // Send audio environment diff --git a/assignment-client/src/audio/AudioMixerClientData.cpp b/assignment-client/src/audio/AudioMixerClientData.cpp index 1eb36cd8a7..a0b96ff39b 100644 --- a/assignment-client/src/audio/AudioMixerClientData.cpp +++ b/assignment-client/src/audio/AudioMixerClientData.cpp @@ -366,6 +366,16 @@ void AudioMixerClientData::sendSelectAudioFormat(SharedNodePointer node, const Q nodeList->sendPacket(std::move(replyPacket), *node); } +void AudioMixerClientData::flushEncoder() { + static QByteArray zeros(AudioConstants::NETWORK_FRAME_BYTES_STEREO, 0); + static QByteArray encodedZeros; + if (_shouldFlushEncoder) { + _shouldFlushEncoder = false; + if (_encoder) { + _encoder->encode(zeros, encodedZeros); + } + } +} void AudioMixerClientData::setupCodec(CodecPluginPointer codec, const QString& codecName) { cleanupCodec(); // cleanup any previously allocated coders first diff --git a/assignment-client/src/audio/AudioMixerClientData.h b/assignment-client/src/audio/AudioMixerClientData.h index 2f8ff4d049..51f06b41c4 100644 --- a/assignment-client/src/audio/AudioMixerClientData.h +++ b/assignment-client/src/audio/AudioMixerClientData.h @@ -77,6 +77,8 @@ public: encodedBuffer = decodedBuffer; } } + void flushEncoder(); + void shouldFlushEncoder() { _shouldFlushEncoder = true; } QString getCodecName() { return _selectedCodecName; } @@ -105,6 +107,8 @@ private: QString _selectedCodecName; Encoder* _encoder{ nullptr }; // for outbound mixed stream Decoder* _decoder{ nullptr }; // for mic stream + + bool _shouldFlushEncoder { false }; }; #endif // hifi_AudioMixerClientData_h From d00a73dde1c937e6e43ea54a1607f131fd4ee9c7 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Thu, 27 Oct 2016 14:27:15 -0700 Subject: [PATCH 2/5] Play the flushed encoder sound at end Basically 0-padding the end of the sound buffer. Next do that in AudioMixer and we should be good. --- assignment-client/src/Agent.cpp | 31 +++++++++++++++---------------- assignment-client/src/Agent.h | 2 +- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/assignment-client/src/Agent.cpp b/assignment-client/src/Agent.cpp index 9e6ec1209c..5459a0a148 100644 --- a/assignment-client/src/Agent.cpp +++ b/assignment-client/src/Agent.cpp @@ -474,10 +474,9 @@ void Agent::processAgentAvatar() { nodeList->broadcastToNodes(std::move(avatarPacket), NodeSet() << NodeType::AvatarMixer); } } -void Agent::flushEncoder() { +void Agent::flushEncoder(QByteArray& encodedZeros) { _flushEncoder = false; - static QByteArray zeros(AudioConstants::NETWORK_FRAME_BYTES_PER_CHANNEL, 0); - static QByteArray encodedZeros; + static const QByteArray zeros(AudioConstants::NETWORK_FRAME_BYTES_PER_CHANNEL, 0); if (_encoder) { _encoder->encode(zeros, encodedZeros); } @@ -485,13 +484,6 @@ void Agent::flushEncoder() { void Agent::processAgentAvatarAudio() { if (_isAvatar && (_isListeningToAudioStream || _avatarSound)) { - // after sound is done playing, encoder has a bit of state in it, - // and needs some 0s to forget or you get a little click next time - // you play something - if (_flushEncoder) { - flushEncoder(); - } - // if we have an avatar audio stream then send it out to our audio-mixer auto scriptedAvatar = DependencyManager::get(); bool silentFrame = true; @@ -528,7 +520,7 @@ void Agent::processAgentAvatarAudio() { } } - auto audioPacket = NLPacket::create(silentFrame + auto audioPacket = NLPacket::create(silentFrame && !_flushEncoder ? PacketType::SilentAudioFrame : PacketType::MicrophoneAudioNoEcho); @@ -564,13 +556,20 @@ void Agent::processAgentAvatarAudio() { glm::quat headOrientation = scriptedAvatar->getHeadOrientation(); audioPacket->writePrimitive(headOrientation); - QByteArray decodedBuffer(reinterpret_cast(nextSoundOutput), numAvailableSamples*sizeof(int16_t)); QByteArray encodedBuffer; - // encode it - if(_encoder) { - _encoder->encode(decodedBuffer, encodedBuffer); + if (_flushEncoder) { + // after sound is done playing, encoder has a bit of state in it, + // and needs some 0s to forget or you get a little click next time + // you play something. So, basically 0-pad the end of the sound buffer + flushEncoder(encodedBuffer); } else { - encodedBuffer = decodedBuffer; + // encode it + QByteArray decodedBuffer(reinterpret_cast(nextSoundOutput), numAvailableSamples*sizeof(int16_t)); + if (_encoder) { + _encoder->encode(decodedBuffer, encodedBuffer); + } else { + encodedBuffer = decodedBuffer; + } } audioPacket->write(encodedBuffer.constData(), encodedBuffer.size()); } diff --git a/assignment-client/src/Agent.h b/assignment-client/src/Agent.h index b882ac3125..b11e492136 100644 --- a/assignment-client/src/Agent.h +++ b/assignment-client/src/Agent.h @@ -81,7 +81,7 @@ signals: private: void negotiateAudioFormat(); void selectAudioFormat(const QString& selectedCodecName); - void flushEncoder(); + void flushEncoder(QByteArray& encodedZeros); std::unique_ptr _scriptEngine; EntityEditPacketSender _entityEditSender; From 5e4f5391442ecf233934a744c213cc2e7fe22154 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Thu, 27 Oct 2016 14:46:04 -0700 Subject: [PATCH 3/5] AudioMixer flushes now And protected agent from case with no codec --- assignment-client/src/Agent.cpp | 2 ++ assignment-client/src/audio/AudioMixer.cpp | 14 ++++++++------ .../src/audio/AudioMixerClientData.cpp | 7 ++++--- assignment-client/src/audio/AudioMixerClientData.h | 6 ++++-- 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/assignment-client/src/Agent.cpp b/assignment-client/src/Agent.cpp index 5459a0a148..5b299ea6a9 100644 --- a/assignment-client/src/Agent.cpp +++ b/assignment-client/src/Agent.cpp @@ -479,6 +479,8 @@ void Agent::flushEncoder(QByteArray& encodedZeros) { static const QByteArray zeros(AudioConstants::NETWORK_FRAME_BYTES_PER_CHANNEL, 0); if (_encoder) { _encoder->encode(zeros, encodedZeros); + } else { + encodedZeros = zeros; } } diff --git a/assignment-client/src/audio/AudioMixer.cpp b/assignment-client/src/audio/AudioMixer.cpp index e580c62905..ad4e6c5138 100644 --- a/assignment-client/src/audio/AudioMixer.cpp +++ b/assignment-client/src/audio/AudioMixer.cpp @@ -818,9 +818,7 @@ void AudioMixer::broadcastMixes() { std::unique_ptr mixPacket; - if (mixHasAudio) { - // may need to flush, this checks - nodeData->flushEncoder(); + if (mixHasAudio || nodeData->shouldFlushEncoder()) { int mixPacketBytes = sizeof(quint16) + AudioConstants::MAX_CODEC_NAME_LENGTH_ON_WIRE + AudioConstants::NETWORK_FRAME_BYTES_STEREO; @@ -834,10 +832,14 @@ void AudioMixer::broadcastMixes() { QString codecInPacket = nodeData->getCodecName(); mixPacket->writeString(codecInPacket); - QByteArray decodedBuffer(reinterpret_cast(_clampedSamples), AudioConstants::NETWORK_FRAME_BYTES_STEREO); QByteArray encodedBuffer; - nodeData->encode(decodedBuffer, encodedBuffer); - + if ( mixHasAudio) { + QByteArray decodedBuffer(reinterpret_cast(_clampedSamples), AudioConstants::NETWORK_FRAME_BYTES_STEREO); + nodeData->encode(decodedBuffer, encodedBuffer); + } else { + // time to flush, which resets the shouldFlush until next time we encode something + nodeData->flushEncoder(encodedBuffer); + } // pack mixed audio samples mixPacket->write(encodedBuffer.constData(), encodedBuffer.size()); } else { diff --git a/assignment-client/src/audio/AudioMixerClientData.cpp b/assignment-client/src/audio/AudioMixerClientData.cpp index a0b96ff39b..4084df7090 100644 --- a/assignment-client/src/audio/AudioMixerClientData.cpp +++ b/assignment-client/src/audio/AudioMixerClientData.cpp @@ -366,15 +366,16 @@ void AudioMixerClientData::sendSelectAudioFormat(SharedNodePointer node, const Q nodeList->sendPacket(std::move(replyPacket), *node); } -void AudioMixerClientData::flushEncoder() { +void AudioMixerClientData::flushEncoder(QByteArray& encodedZeros) { static QByteArray zeros(AudioConstants::NETWORK_FRAME_BYTES_STEREO, 0); - static QByteArray encodedZeros; if (_shouldFlushEncoder) { - _shouldFlushEncoder = false; if (_encoder) { _encoder->encode(zeros, encodedZeros); + } else { + encodedZeros = zeros; } } + _shouldFlushEncoder = false; } void AudioMixerClientData::setupCodec(CodecPluginPointer codec, const QString& codecName) { diff --git a/assignment-client/src/audio/AudioMixerClientData.h b/assignment-client/src/audio/AudioMixerClientData.h index 51f06b41c4..ba8e5c021b 100644 --- a/assignment-client/src/audio/AudioMixerClientData.h +++ b/assignment-client/src/audio/AudioMixerClientData.h @@ -76,9 +76,11 @@ public: } else { encodedBuffer = decodedBuffer; } + // once you have encoded, you need to flush eventually. + _shouldFlushEncoder = true; } - void flushEncoder(); - void shouldFlushEncoder() { _shouldFlushEncoder = true; } + void flushEncoder(QByteArray& encodedZeros); + bool shouldFlushEncoder() { return _shouldFlushEncoder; } QString getCodecName() { return _selectedCodecName; } From 292dd674335867cd070e09332aeb3dcca793f98e Mon Sep 17 00:00:00 2001 From: David Kelly Date: Thu, 27 Oct 2016 16:04:30 -0700 Subject: [PATCH 4/5] cleanup --- assignment-client/src/audio/AudioMixer.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/assignment-client/src/audio/AudioMixer.cpp b/assignment-client/src/audio/AudioMixer.cpp index ad4e6c5138..7861f69b2e 100644 --- a/assignment-client/src/audio/AudioMixer.cpp +++ b/assignment-client/src/audio/AudioMixer.cpp @@ -857,9 +857,6 @@ void AudioMixer::broadcastMixes() { // pack number of silent audio samples quint16 numSilentSamples = AudioConstants::NETWORK_FRAME_SAMPLES_STEREO; mixPacket->writePrimitive(numSilentSamples); - - // we will need to flush encoder since we are sending silent packets outside it - nodeData->shouldFlushEncoder(); } // Send audio environment From e3a22618d1abaea263fc5f87cd0d0e6e1273e3be Mon Sep 17 00:00:00 2001 From: David Kelly Date: Thu, 27 Oct 2016 17:49:43 -0700 Subject: [PATCH 5/5] CR feedback --- assignment-client/src/Agent.cpp | 9 +++------ assignment-client/src/Agent.h | 2 +- assignment-client/src/audio/AudioMixer.cpp | 5 +++-- assignment-client/src/audio/AudioMixerClientData.cpp | 2 +- assignment-client/src/audio/AudioMixerClientData.h | 2 +- 5 files changed, 9 insertions(+), 11 deletions(-) diff --git a/assignment-client/src/Agent.cpp b/assignment-client/src/Agent.cpp index 5b299ea6a9..23bee2c91a 100644 --- a/assignment-client/src/Agent.cpp +++ b/assignment-client/src/Agent.cpp @@ -474,7 +474,7 @@ void Agent::processAgentAvatar() { nodeList->broadcastToNodes(std::move(avatarPacket), NodeSet() << NodeType::AvatarMixer); } } -void Agent::flushEncoder(QByteArray& encodedZeros) { +void Agent::encodeFrameOfZeros(QByteArray& encodedZeros) { _flushEncoder = false; static const QByteArray zeros(AudioConstants::NETWORK_FRAME_BYTES_PER_CHANNEL, 0); if (_encoder) { @@ -560,14 +560,11 @@ void Agent::processAgentAvatarAudio() { QByteArray encodedBuffer; if (_flushEncoder) { - // after sound is done playing, encoder has a bit of state in it, - // and needs some 0s to forget or you get a little click next time - // you play something. So, basically 0-pad the end of the sound buffer - flushEncoder(encodedBuffer); + encodeFrameOfZeros(encodedBuffer); } else { - // encode it QByteArray decodedBuffer(reinterpret_cast(nextSoundOutput), numAvailableSamples*sizeof(int16_t)); if (_encoder) { + // encode it _encoder->encode(decodedBuffer, encodedBuffer); } else { encodedBuffer = decodedBuffer; diff --git a/assignment-client/src/Agent.h b/assignment-client/src/Agent.h index b11e492136..939b51625a 100644 --- a/assignment-client/src/Agent.h +++ b/assignment-client/src/Agent.h @@ -81,7 +81,7 @@ signals: private: void negotiateAudioFormat(); void selectAudioFormat(const QString& selectedCodecName); - void flushEncoder(QByteArray& encodedZeros); + void encodeFrameOfZeros(QByteArray& encodedZeros); std::unique_ptr _scriptEngine; EntityEditPacketSender _entityEditSender; diff --git a/assignment-client/src/audio/AudioMixer.cpp b/assignment-client/src/audio/AudioMixer.cpp index 2f26042266..bc356b8ce1 100644 --- a/assignment-client/src/audio/AudioMixer.cpp +++ b/assignment-client/src/audio/AudioMixer.cpp @@ -825,15 +825,16 @@ void AudioMixer::broadcastMixes() { mixPacket->writeString(codecInPacket); QByteArray encodedBuffer; - if ( mixHasAudio) { + if (mixHasAudio) { QByteArray decodedBuffer(reinterpret_cast(_clampedSamples), AudioConstants::NETWORK_FRAME_BYTES_STEREO); nodeData->encode(decodedBuffer, encodedBuffer); } else { // time to flush, which resets the shouldFlush until next time we encode something - nodeData->flushEncoder(encodedBuffer); + nodeData->encodeFrameOfZeros(encodedBuffer); } // pack mixed audio samples mixPacket->write(encodedBuffer.constData(), encodedBuffer.size()); + } else { int silentPacketBytes = sizeof(quint16) + sizeof(quint16) + AudioConstants::MAX_CODEC_NAME_LENGTH_ON_WIRE; mixPacket = NLPacket::create(PacketType::SilentAudioFrame, silentPacketBytes); diff --git a/assignment-client/src/audio/AudioMixerClientData.cpp b/assignment-client/src/audio/AudioMixerClientData.cpp index 836cf9321b..58d89697af 100644 --- a/assignment-client/src/audio/AudioMixerClientData.cpp +++ b/assignment-client/src/audio/AudioMixerClientData.cpp @@ -371,7 +371,7 @@ void AudioMixerClientData::sendSelectAudioFormat(SharedNodePointer node, const Q nodeList->sendPacket(std::move(replyPacket), *node); } -void AudioMixerClientData::flushEncoder(QByteArray& encodedZeros) { +void AudioMixerClientData::encodeFrameOfZeros(QByteArray& encodedZeros) { static QByteArray zeros(AudioConstants::NETWORK_FRAME_BYTES_STEREO, 0); if (_shouldFlushEncoder) { if (_encoder) { diff --git a/assignment-client/src/audio/AudioMixerClientData.h b/assignment-client/src/audio/AudioMixerClientData.h index 682c99584e..34263f9cbe 100644 --- a/assignment-client/src/audio/AudioMixerClientData.h +++ b/assignment-client/src/audio/AudioMixerClientData.h @@ -80,7 +80,7 @@ public: // once you have encoded, you need to flush eventually. _shouldFlushEncoder = true; } - void flushEncoder(QByteArray& encodedZeros); + void encodeFrameOfZeros(QByteArray& encodedZeros); bool shouldFlushEncoder() { return _shouldFlushEncoder; } QString getCodecName() { return _selectedCodecName; }