From 8b8a98bfccbcd55885c474d12268a706d51c3fea Mon Sep 17 00:00:00 2001 From: David Kelly Date: Fri, 22 Jul 2016 16:40:23 -0700 Subject: [PATCH] Acutally audio was really broken Seems there were 2 issues: * If you couldn't inject locally (say, no audio interface because you are an assignment client), we would not do the network injection at all. That's bad. * When we don't have a codec, like when using an injector, we were serializing the empty string in some problematic way. I just use the built-in serialization, and it seems fine. I tested this by first playing a sound in the console (or maybe you want to tip some cows, that is ok too). Then in _another_ interface client on another machine in the same domain, I better hear the sound. Then, I added a script to play audio as a persistent script, which just loops forever and ever. You should hear that on both interface clients also. A detailed test plan to follow. Also: using @zappoman serialization code, which is safer than using the Qt code which does magical things for nulls. Good to do this cuz you know, things happen... --- libraries/audio/src/AudioInjector.cpp | 53 +++++++++++++++++---------- 1 file changed, 33 insertions(+), 20 deletions(-) diff --git a/libraries/audio/src/AudioInjector.cpp b/libraries/audio/src/AudioInjector.cpp index 878a4c627c..244740320b 100644 --- a/libraries/audio/src/AudioInjector.cpp +++ b/libraries/audio/src/AudioInjector.cpp @@ -159,8 +159,8 @@ bool AudioInjector::injectLocally() { } if (!success) { - // we never started so we are finished, call our stop method - stop(); + // we never started so we are finished with local injection + finishLocalInjection(); } return success; @@ -170,6 +170,21 @@ const uchar MAX_INJECTOR_VOLUME = 0xFF; static const int64_t NEXT_FRAME_DELTA_ERROR_OR_FINISHED = -1; static const int64_t NEXT_FRAME_DELTA_IMMEDIATELY = 0; +qint64 writeStringToStream(const QString& string, QDataStream& stream) { + QByteArray data = string.toUtf8(); + uint32_t length = data.length(); + if (length == 0) { + stream << static_cast(length); + } else { + // http://doc.qt.io/qt-5/datastreamformat.html + // QDataStream << QByteArray - + // If the byte array is null : 0xFFFFFFFF (quint32) + // Otherwise : the array size(quint32) followed by the array bytes, i.e.size bytes + stream << data; + } + return length + sizeof(uint32_t); +} + int64_t AudioInjector::injectNextFrame() { if (_state == AudioInjector::State::Finished) { qDebug() << "AudioInjector::injectNextFrame called but AudioInjector has finished and was not restarted. Returning."; @@ -180,7 +195,7 @@ int64_t AudioInjector::injectNextFrame() { static int positionOptionOffset = -1; static int volumeOptionOffset = -1; static int audioDataOffset = -1; - + if (!_currentPacket) { if (_currentSendOffset < 0 || _currentSendOffset >= _audioData.size()) { @@ -216,6 +231,10 @@ int64_t AudioInjector::injectNextFrame() { // pack some placeholder sequence number for now audioPacketStream << (quint16) 0; + // current injectors don't use codecs, so pack in the unknown codec name + QString noCodecForInjectors(""); + writeStringToStream(noCodecForInjectors, audioPacketStream); + // pack stream identifier (a generated UUID) audioPacketStream << QUuid::createUuid(); @@ -243,7 +262,6 @@ int64_t AudioInjector::injectNextFrame() { volumeOptionOffset = _currentPacket->pos(); quint8 volume = MAX_INJECTOR_VOLUME; audioPacketStream << volume; - audioPacketStream << _options.ignorePenumbra; audioDataOffset = _currentPacket->pos(); @@ -254,7 +272,6 @@ int64_t AudioInjector::injectNextFrame() { return NEXT_FRAME_DELTA_ERROR_OR_FINISHED; } } - if (!_frameTimer->isValid()) { // in the case where we have been restarted, the frame timer will be invalid and we need to start it back over here _frameTimer->restart(); @@ -353,7 +370,7 @@ void AudioInjector::triggerDeleteAfterFinish() { return; } - if (_state == State::Finished) { + if (stateHas(AudioInjectorState::Finished)) { stopAndDeleteLater(); } else { _state = State::NotFinishedWithPendingDelete; @@ -419,21 +436,17 @@ AudioInjector* AudioInjector::playSound(const QByteArray& buffer, const AudioInj // setup parameters required for injection injector->setupInjection(); - if (options.localOnly) { - if (injector->injectLocally()) { - // local injection succeeded, return the pointer to injector - return injector; - } else { - // unable to inject locally, return a nullptr - return nullptr; - } - } else { - // attempt to thread the new injector - if (injectorManager->threadInjector(injector)) { - return injector; - } else { + // we always inject locally, except when there is no localInterface + injector->injectLocally(); + + // if localOnly, we are done, just return injector. + if (!options.localOnly) { + + // send off to server for everyone else + if (!injectorManager->threadInjector(injector)) { // we failed to thread the new injector (we are at the max number of injector threads) - return nullptr; + qDebug() << "AudioInjector::playSound failed to thread injector"; } } + return injector; }