From e1212c54cbc5931030aefba4132d67e07db49f0f Mon Sep 17 00:00:00 2001 From: David Kelly Date: Fri, 15 Jul 2016 09:06:13 -0700 Subject: [PATCH 01/18] Updated to deal with streaming out of both buffers So gotta keep track of when finished streaming to network and locally separately. That means the State needed to be more of a bitflag and less of an enum. --- libraries/audio-client/src/AudioClient.cpp | 1 - libraries/audio/src/AudioInjector.cpp | 105 ++++++++++++++------- libraries/audio/src/AudioInjector.h | 34 +++++-- 3 files changed, 95 insertions(+), 45 deletions(-) diff --git a/libraries/audio-client/src/AudioClient.cpp b/libraries/audio-client/src/AudioClient.cpp index dd72125d93..bec30edb4e 100644 --- a/libraries/audio-client/src/AudioClient.cpp +++ b/libraries/audio-client/src/AudioClient.cpp @@ -954,7 +954,6 @@ void AudioClient::processReceivedSamples(const QByteArray& decodedBuffer, QByteA if (hasReverb) { assert(_outputFormat.channelCount() == 2); updateReverbOptions(); - qDebug() << "handling reverb"; _listenerReverb.render(outputSamples, outputSamples, numDeviceOutputSamples/2); } } diff --git a/libraries/audio/src/AudioInjector.cpp b/libraries/audio/src/AudioInjector.cpp index 08143b8491..f94890ea7d 100644 --- a/libraries/audio/src/AudioInjector.cpp +++ b/libraries/audio/src/AudioInjector.cpp @@ -28,6 +28,15 @@ int audioInjectorPtrMetaTypeId = qRegisterMetaType(); +AudioInjectorState operator& (AudioInjectorState lhs, AudioInjectorState rhs) { + return static_cast(static_cast(lhs) | static_cast(rhs)); +}; + +AudioInjectorState& operator&= (AudioInjectorState& lhs, AudioInjectorState rhs) { + lhs = static_cast(static_cast(lhs) & static_cast(rhs)); + return lhs; +}; + AudioInjector::AudioInjector(QObject* parent) : QObject(parent) { @@ -56,10 +65,26 @@ void AudioInjector::setOptions(const AudioInjectorOptions& options) { _options.stereo = currentlyStereo; } +void AudioInjector::finishNetworkInjection() { + _state &= AudioInjectorState::NetworkInjectionFinished; + + // if we are already finished with local + // injection, then we are finished + if((_state & AudioInjectorState::LocalInjectionFinished) == AudioInjectorState::LocalInjectionFinished) { + finish(); + } +} + +void AudioInjector::finishLocalInjection() { + _state &= AudioInjectorState::LocalInjectionFinished; + if(_options.localOnly || ((_state & AudioInjectorState::NetworkInjectionFinished) == AudioInjectorState::NetworkInjectionFinished)) { + finish(); + } +} void AudioInjector::finish() { - bool shouldDelete = (_state == State::NotFinishedWithPendingDelete); - _state = State::Finished; + bool shouldDelete = ((_state & AudioInjectorState::PendingDelete) == AudioInjectorState::PendingDelete); + _state &= AudioInjectorState::Finished; emit finished(); @@ -121,23 +146,31 @@ void AudioInjector::restart() { _hasSentFirstFrame = false; // check our state to decide if we need extra handling for the restart request - if (_state == State::Finished) { + if ((_state & AudioInjectorState::Finished) == AudioInjectorState::Finished) { // we finished playing, need to reset state so we can get going again _hasSetup = false; _shouldStop = false; - _state = State::NotFinished; + _state = AudioInjectorState::NotFinished; // call inject audio to start injection over again setupInjection(); - // if we're a local injector, just inject again - if (_options.localOnly) { - injectLocally(); - } else { - // wake the AudioInjectorManager back up if it's stuck waiting - if (!injectorManager->restartFinishedInjector(this)) { - _state = State::Finished; // we're not playing, so reset the state used by isPlaying. + // inject locally + if(injectLocally()) { + + // if not localOnly, wake the AudioInjectorManager back up if it is stuck waiting + if (!_options.localOnly) { + + if (!injectorManager->restartFinishedInjector(this)) { + // TODO: this logic seems to remove the pending delete, + // which makes me wonder about the deleteLater calls + _state = AudioInjectorState::Finished; // we're not playing, so reset the state used by isPlaying. + } } + } else { + // TODO: this logic seems to remove the pending delete, + // which makes me wonder about the deleteLater calls + _state = AudioInjectorState::Finished; // we failed to play, so we are finished again } } } @@ -183,7 +216,7 @@ static const int64_t NEXT_FRAME_DELTA_ERROR_OR_FINISHED = -1; static const int64_t NEXT_FRAME_DELTA_IMMEDIATELY = 0; int64_t AudioInjector::injectNextFrame() { - if (_state == AudioInjector::State::Finished) { + if ((_state & AudioInjectorState::NetworkInjectionFinished) == AudioInjectorState::NetworkInjectionFinished) { qDebug() << "AudioInjector::injectNextFrame called but AudioInjector has finished and was not restarted. Returning."; return NEXT_FRAME_DELTA_ERROR_OR_FINISHED; } @@ -234,8 +267,10 @@ int64_t AudioInjector::injectNextFrame() { // pack the stereo/mono type of the stream audioPacketStream << _options.stereo; - // pack the flag for loopback - uchar loopbackFlag = (uchar)true; + // pack the flag for loopback. Now, we don't loopback + // and _always_ play locally, so loopbackFlag should be + // false always. + uchar loopbackFlag = (uchar)false; audioPacketStream << loopbackFlag; // pack the position for injected audio @@ -333,7 +368,7 @@ int64_t AudioInjector::injectNextFrame() { } if (_currentSendOffset >= _audioData.size() && !_options.loop) { - finish(); + finishNetworkInjection(); return NEXT_FRAME_DELTA_ERROR_OR_FINISHED; } @@ -372,10 +407,10 @@ void AudioInjector::triggerDeleteAfterFinish() { return; } - if (_state == State::Finished) { + if (_state == AudioInjectorState::Finished) { stopAndDeleteLater(); } else { - _state = State::NotFinishedWithPendingDelete; + _state &= AudioInjectorState::PendingDelete; } } @@ -421,7 +456,7 @@ AudioInjector* AudioInjector::playSoundAndDelete(const QByteArray& buffer, const AudioInjector* sound = playSound(buffer, options, localInterface); if (sound) { - sound->_state = AudioInjector::State::NotFinishedWithPendingDelete; + sound->_state &= AudioInjectorState::PendingDelete; } return sound; @@ -438,21 +473,23 @@ 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 failed to thread the new injector (we are at the max number of injector threads) - return nullptr; - } + // we always inject locally + // + if (!injector->injectLocally()) { + // failed, so don't bother sending to server + qDebug() << "AudioInjector::playSound failed to inject locally"; + return nullptr; } + // if localOnly, we are done, just return injector. + if (options.localOnly) { + return injector; + } + + // 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) + qDebug() << "AudioInjector::playSound failed to thread injector"; + } + return injector; + } diff --git a/libraries/audio/src/AudioInjector.h b/libraries/audio/src/AudioInjector.h index 1af733ace6..e13fc15c26 100644 --- a/libraries/audio/src/AudioInjector.h +++ b/libraries/audio/src/AudioInjector.h @@ -32,24 +32,35 @@ class AbstractAudioInterface; class AudioInjectorManager; + +enum class AudioInjectorState : uint8_t { + NotFinished = 1, + Finished = 2, + PendingDelete = 4, + LocalInjectionFinished = 8, + NetworkInjectionFinished = 16 +}; + +AudioInjectorState operator& (AudioInjectorState lhs, AudioInjectorState rhs); +AudioInjectorState& operator&= (AudioInjectorState& lhs, AudioInjectorState rhs); + // In order to make scripting cleaner for the AudioInjector, the script now holds on to the AudioInjector object // until it dies. - class AudioInjector : public QObject { Q_OBJECT public: - enum class State : uint8_t { - NotFinished, - NotFinishedWithPendingDelete, - Finished - }; - + static const uint8_t NotFinished = 1; + static const uint8_t Finished = 2; + static const uint8_t PendingDelete = 4; + static const uint8_t LocalInjectionFinished = 8; + static const uint8_t NetworkInjectionFinished = 16; + AudioInjector(QObject* parent); AudioInjector(const Sound& sound, const AudioInjectorOptions& injectorOptions); AudioInjector(const QByteArray& audioData, const AudioInjectorOptions& injectorOptions); - bool isFinished() const { return _state == State::Finished; } + bool isFinished() const { return (_state & AudioInjectorState::Finished) == AudioInjectorState::Finished; } int getCurrentSendOffset() const { return _currentSendOffset; } void setCurrentSendOffset(int currentSendOffset) { _currentSendOffset = currentSendOffset; } @@ -78,8 +89,10 @@ public slots: void setOptions(const AudioInjectorOptions& options); float getLoudness() const { return _loudness; } - bool isPlaying() const { return _state == State::NotFinished || _state == State::NotFinishedWithPendingDelete; } + bool isPlaying() const { return (_state & AudioInjectorState::NotFinished) == AudioInjectorState::NotFinished; } void finish(); + void finishLocalInjection(); + void finishNetworkInjection(); signals: void finished(); @@ -92,7 +105,7 @@ private: QByteArray _audioData; AudioInjectorOptions _options; - State _state { State::NotFinished }; + AudioInjectorState _state { AudioInjectorState::NotFinished }; bool _hasSentFirstFrame { false }; bool _hasSetup { false }; bool _shouldStop { false }; @@ -111,4 +124,5 @@ private: friend class AudioInjectorManager; }; + #endif // hifi_AudioInjector_h From f0db7b9d39e5b2da679515aa3fe0a3c471d0055a Mon Sep 17 00:00:00 2001 From: Leonardo Murillo Date: Fri, 15 Jul 2016 11:42:30 -0600 Subject: [PATCH 02/18] Checkpoint --- cmake/externals/hifiAudioCodec/CMakeLists.txt | 8 +++----- cmake/macros/SetupHifiClientServerPlugin.cmake | 4 ++-- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/cmake/externals/hifiAudioCodec/CMakeLists.txt b/cmake/externals/hifiAudioCodec/CMakeLists.txt index 3a8c714d79..642a58b685 100644 --- a/cmake/externals/hifiAudioCodec/CMakeLists.txt +++ b/cmake/externals/hifiAudioCodec/CMakeLists.txt @@ -1,13 +1,13 @@ include(ExternalProject) include(SelectLibraryConfigurations) -set(EXTERNAL_NAME HiFiAudioCodec) +set(EXTERNAL_NAME hifiAudioCodec) string(TOUPPER ${EXTERNAL_NAME} EXTERNAL_NAME_UPPER) ExternalProject_Add( ${EXTERNAL_NAME} - URL https://s3.amazonaws.com/hifi-public/dependencies/codecSDK-1.zip + URL http://s3.amazonaws.com/hifi-public/dependencies/codecSDK-1.zip URL_MD5 23ec3fe51eaa155ea159a4971856fc13 CONFIGURE_COMMAND "" BUILD_COMMAND "" @@ -27,7 +27,5 @@ if (WIN32) elseif(APPLE) set(${EXTERNAL_NAME_UPPER}_LIBRARIES ${SOURCE_DIR}/Release/libaudio.a CACHE TYPE INTERNAL) elseif(NOT ANDROID) - # FIXME need to account for different architectures - #set(${EXTERNAL_NAME_UPPER}_LIBRARIES ${SOURCE_DIR}/lib/linux64/audio.so CACHE TYPE INTERNAL) + set(${EXTERNAL_NAME_UPPER}_LIBRARIES ${SOURCE_DIR}/lib/linux64/audio.so CACHE TYPE INTERNAL) endif() - diff --git a/cmake/macros/SetupHifiClientServerPlugin.cmake b/cmake/macros/SetupHifiClientServerPlugin.cmake index 8ba38a09c3..74ceaeb0ab 100644 --- a/cmake/macros/SetupHifiClientServerPlugin.cmake +++ b/cmake/macros/SetupHifiClientServerPlugin.cmake @@ -37,7 +37,7 @@ macro(SETUP_HIFI_CLIENT_SERVER_PLUGIN) ${CLIENT_PLUGIN_FULL_PATH} ) # copy the client plugin binaries - add_custom_command(TARGET ${DIR} POST_BUILD + add_custom_command(TARGET ${TARGET_NAME} POST_BUILD COMMAND "${CMAKE_COMMAND}" -E copy "$" ${CLIENT_PLUGIN_FULL_PATH} @@ -50,7 +50,7 @@ macro(SETUP_HIFI_CLIENT_SERVER_PLUGIN) ${SERVER_PLUGIN_FULL_PATH} ) # copy the server plugin binaries - add_custom_command(TARGET ${DIR} POST_BUILD + add_custom_command(TARGET ${TARGET_NAME} POST_BUILD COMMAND "${CMAKE_COMMAND}" -E copy "$" ${SERVER_PLUGIN_FULL_PATH} From 53d524eb2fe40668424aac9feb1a0d0c1cb01e15 Mon Sep 17 00:00:00 2001 From: Leonardo Murillo Date: Fri, 15 Jul 2016 12:04:45 -0600 Subject: [PATCH 03/18] Checkpoint --- cmake/externals/hifiAudioCodec/CMakeLists.txt | 2 +- .../macros/AddDependencyExternalProjects.cmake | 5 ++++- plugins/hifiCodec/CMakeLists.txt | 18 +++++++----------- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/cmake/externals/hifiAudioCodec/CMakeLists.txt b/cmake/externals/hifiAudioCodec/CMakeLists.txt index 642a58b685..8a1b07253a 100644 --- a/cmake/externals/hifiAudioCodec/CMakeLists.txt +++ b/cmake/externals/hifiAudioCodec/CMakeLists.txt @@ -23,7 +23,7 @@ ExternalProject_Get_Property(${EXTERNAL_NAME} SOURCE_DIR) set(${EXTERNAL_NAME_UPPER}_INCLUDE_DIRS ${SOURCE_DIR}/include CACHE TYPE INTERNAL) if (WIN32) - set(${EXTERNAL_NAME_UPPER}_LIBRARIES ${SOURCE_DIR}/Release/audio.lib CACHE TYPE INTERNAL) + set(${EXTERNAL_NAME_UPPER}_LIBRARIES ${SOURCE_DIR}/Release/audio.lib CACHE TYPE INTERNAL) elseif(APPLE) set(${EXTERNAL_NAME_UPPER}_LIBRARIES ${SOURCE_DIR}/Release/libaudio.a CACHE TYPE INTERNAL) elseif(NOT ANDROID) diff --git a/cmake/macros/AddDependencyExternalProjects.cmake b/cmake/macros/AddDependencyExternalProjects.cmake index e35ca98959..3cbb961308 100644 --- a/cmake/macros/AddDependencyExternalProjects.cmake +++ b/cmake/macros/AddDependencyExternalProjects.cmake @@ -17,10 +17,12 @@ macro(ADD_DEPENDENCY_EXTERNAL_PROJECTS) # has the user told us they specific don't want this as an external project? if (NOT USE_LOCAL_${_PROJ_NAME_UPPER}) + message(STATUS "LEODEBUG: Looking for dependency ${_PROJ_NAME}") # have we already detected we can't have this as external project on this OS? if (NOT DEFINED ${_PROJ_NAME_UPPER}_EXTERNAL_PROJECT OR ${_PROJ_NAME_UPPER}_EXTERNAL_PROJECT) # have we already setup the target? if (NOT TARGET ${_PROJ_NAME}) + message(STATUS "LEODEBUG: We dont have a target with that name ${_PROJ_NAME} adding directory ${EXTERNAL_PROJECT_DIR}/${_PROJ_NAME}") add_subdirectory(${EXTERNAL_PROJECT_DIR}/${_PROJ_NAME} ${EXTERNALS_BINARY_DIR}/${_PROJ_NAME}) # did we end up adding an external project target? @@ -35,6 +37,7 @@ macro(ADD_DEPENDENCY_EXTERNAL_PROJECTS) endif () if (TARGET ${_PROJ_NAME}) + message(STATUS "LEODEBUG: We no have the target ${_PROJ_NAME}") add_dependencies(${TARGET_NAME} ${_PROJ_NAME}) endif () @@ -43,4 +46,4 @@ macro(ADD_DEPENDENCY_EXTERNAL_PROJECTS) endforeach() -endmacro() \ No newline at end of file +endmacro() diff --git a/plugins/hifiCodec/CMakeLists.txt b/plugins/hifiCodec/CMakeLists.txt index 0af7e42ea1..b278e839e4 100644 --- a/plugins/hifiCodec/CMakeLists.txt +++ b/plugins/hifiCodec/CMakeLists.txt @@ -6,15 +6,11 @@ # See the accompanying file LICENSE or http:#www.apache.org/licenses/LICENSE-2.0.html # -if (WIN32 OR APPLE) - set(TARGET_NAME hifiCodec) - setup_hifi_client_server_plugin() - - link_hifi_libraries(audio shared plugins) - - add_dependency_external_projects(HiFiAudioCodec) - target_include_directories(${TARGET_NAME} PRIVATE ${HIFIAUDIOCODEC_INCLUDE_DIRS}) - target_link_libraries(${TARGET_NAME} ${HIFIAUDIOCODEC_LIBRARIES}) - install_beside_console() -endif() +set(TARGET_NAME hifiCodec) +setup_hifi_client_server_plugin() +link_hifi_libraries(audio shared plugins) +add_dependency_external_projects(hifiAudioCodec) +target_include_directories(${TARGET_NAME} PRIVATE ${HIFIAUDIOCODEC_INCLUDE_DIRS}) +target_link_libraries(${TARGET_NAME} ${HIFIAUDIOCODEC_LIBRARIES}) +install_beside_console() From b0c27a53e2b2d7c2dc0434484bb217638d1e4a50 Mon Sep 17 00:00:00 2001 From: Leonardo Murillo Date: Fri, 15 Jul 2016 13:01:21 -0600 Subject: [PATCH 04/18] Linking to Linux only SDK --- cmake/externals/hifiAudioCodec/CMakeLists.txt | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/cmake/externals/hifiAudioCodec/CMakeLists.txt b/cmake/externals/hifiAudioCodec/CMakeLists.txt index 8a1b07253a..87b5044b42 100644 --- a/cmake/externals/hifiAudioCodec/CMakeLists.txt +++ b/cmake/externals/hifiAudioCodec/CMakeLists.txt @@ -5,15 +5,27 @@ set(EXTERNAL_NAME hifiAudioCodec) string(TOUPPER ${EXTERNAL_NAME} EXTERNAL_NAME_UPPER) -ExternalProject_Add( - ${EXTERNAL_NAME} - URL http://s3.amazonaws.com/hifi-public/dependencies/codecSDK-1.zip - URL_MD5 23ec3fe51eaa155ea159a4971856fc13 - CONFIGURE_COMMAND "" - BUILD_COMMAND "" - INSTALL_COMMAND "" - LOG_DOWNLOAD 1 -) +if (WIN32 OR APPLE) + ExternalProject_Add( + ${EXTERNAL_NAME} + URL http://s3.amazonaws.com/hifi-public/dependencies/codecSDK-1.zip + URL_MD5 23ec3fe51eaa155ea159a4971856fc13 + CONFIGURE_COMMAND "" + BUILD_COMMAND "" + INSTALL_COMMAND "" + LOG_DOWNLOAD 1 + ) +elseif(NOT ANDROID) + ExternalProject_Add( + ${EXTERNAL_NAME} + URL http://s3.amazonaws.com/hifi-public/dependencies/codecSDK-linux.zip + URL_MD5 7d37914a18aa4de971d2f45dd3043bde + CONFIGURE_COMMAND "" + BUILD_COMMAND "" + INSTALL_COMMAND "" + LOG_DOWNLOAD 1 + ) +endif() # Hide this external target (for ide users) set_target_properties(${EXTERNAL_NAME} PROPERTIES FOLDER "hidden/externals") @@ -27,5 +39,5 @@ if (WIN32) elseif(APPLE) set(${EXTERNAL_NAME_UPPER}_LIBRARIES ${SOURCE_DIR}/Release/libaudio.a CACHE TYPE INTERNAL) elseif(NOT ANDROID) - set(${EXTERNAL_NAME_UPPER}_LIBRARIES ${SOURCE_DIR}/lib/linux64/audio.so CACHE TYPE INTERNAL) + set(${EXTERNAL_NAME_UPPER}_LIBRARIES ${SOURCE_DIR}/Release/libaudio.a CACHE TYPE INTERNAL) endif() From ce67d57279209593daa18c033c1e601450273f0f Mon Sep 17 00:00:00 2001 From: Leonardo Murillo Date: Fri, 15 Jul 2016 13:03:40 -0600 Subject: [PATCH 05/18] Removing Debugs --- cmake/macros/AddDependencyExternalProjects.cmake | 3 --- 1 file changed, 3 deletions(-) diff --git a/cmake/macros/AddDependencyExternalProjects.cmake b/cmake/macros/AddDependencyExternalProjects.cmake index 3cbb961308..99b8317fd7 100644 --- a/cmake/macros/AddDependencyExternalProjects.cmake +++ b/cmake/macros/AddDependencyExternalProjects.cmake @@ -17,12 +17,10 @@ macro(ADD_DEPENDENCY_EXTERNAL_PROJECTS) # has the user told us they specific don't want this as an external project? if (NOT USE_LOCAL_${_PROJ_NAME_UPPER}) - message(STATUS "LEODEBUG: Looking for dependency ${_PROJ_NAME}") # have we already detected we can't have this as external project on this OS? if (NOT DEFINED ${_PROJ_NAME_UPPER}_EXTERNAL_PROJECT OR ${_PROJ_NAME_UPPER}_EXTERNAL_PROJECT) # have we already setup the target? if (NOT TARGET ${_PROJ_NAME}) - message(STATUS "LEODEBUG: We dont have a target with that name ${_PROJ_NAME} adding directory ${EXTERNAL_PROJECT_DIR}/${_PROJ_NAME}") add_subdirectory(${EXTERNAL_PROJECT_DIR}/${_PROJ_NAME} ${EXTERNALS_BINARY_DIR}/${_PROJ_NAME}) # did we end up adding an external project target? @@ -37,7 +35,6 @@ macro(ADD_DEPENDENCY_EXTERNAL_PROJECTS) endif () if (TARGET ${_PROJ_NAME}) - message(STATUS "LEODEBUG: We no have the target ${_PROJ_NAME}") add_dependencies(${TARGET_NAME} ${_PROJ_NAME}) endif () From 8df4ed01d92b26411f1d7c2e7e565792aa411dc3 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Fri, 15 Jul 2016 16:22:41 -0700 Subject: [PATCH 06/18] fixed typo which oddly I was sure I already did. Seems I didn't push it? OK now it is pushed. --- libraries/audio/src/AudioInjector.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/audio/src/AudioInjector.cpp b/libraries/audio/src/AudioInjector.cpp index f94890ea7d..0b0be9353b 100644 --- a/libraries/audio/src/AudioInjector.cpp +++ b/libraries/audio/src/AudioInjector.cpp @@ -29,7 +29,7 @@ int audioInjectorPtrMetaTypeId = qRegisterMetaType(); AudioInjectorState operator& (AudioInjectorState lhs, AudioInjectorState rhs) { - return static_cast(static_cast(lhs) | static_cast(rhs)); + return static_cast(static_cast(lhs) & static_cast(rhs)); }; AudioInjectorState& operator&= (AudioInjectorState& lhs, AudioInjectorState rhs) { From fb99828e30302f2c615e74926dbe62a647def972 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Fri, 15 Jul 2016 17:09:27 -0700 Subject: [PATCH 07/18] PR feedback Code a bit more readable. Sadly (and I guess it makes sense), a enum class XXX is not a class, so you cannot have member functions for it. I can imagine no way to have a vtable if you are really representing it as a uint8_t or whatever. So, I put a stateHas function in the AudioInjector instead. Definite improvement. --- libraries/audio/src/AudioInjector.cpp | 15 +++++++++------ libraries/audio/src/AudioInjector.h | 5 +++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/libraries/audio/src/AudioInjector.cpp b/libraries/audio/src/AudioInjector.cpp index 0b0be9353b..e992e3c541 100644 --- a/libraries/audio/src/AudioInjector.cpp +++ b/libraries/audio/src/AudioInjector.cpp @@ -57,6 +57,10 @@ AudioInjector::AudioInjector(const QByteArray& audioData, const AudioInjectorOpt } +bool AudioInjector::stateHas(AudioInjectorState state) const { + return (_state & state) == state; +} + void AudioInjector::setOptions(const AudioInjectorOptions& options) { // since options.stereo is computed from the audio stream, // we need to copy it from existing options just in case. @@ -70,20 +74,19 @@ void AudioInjector::finishNetworkInjection() { // if we are already finished with local // injection, then we are finished - if((_state & AudioInjectorState::LocalInjectionFinished) == AudioInjectorState::LocalInjectionFinished) { + if(stateHas(AudioInjectorState::LocalInjectionFinished)) { finish(); } } void AudioInjector::finishLocalInjection() { _state &= AudioInjectorState::LocalInjectionFinished; - if(_options.localOnly || ((_state & AudioInjectorState::NetworkInjectionFinished) == AudioInjectorState::NetworkInjectionFinished)) { + if(_options.localOnly || stateHas(AudioInjectorState::NetworkInjectionFinished)) { finish(); } } void AudioInjector::finish() { - bool shouldDelete = ((_state & AudioInjectorState::PendingDelete) == AudioInjectorState::PendingDelete); _state &= AudioInjectorState::Finished; emit finished(); @@ -94,7 +97,7 @@ void AudioInjector::finish() { _localBuffer = NULL; } - if (shouldDelete) { + if (stateHas(AudioInjectorState::PendingDelete)) { // we've been asked to delete after finishing, trigger a deleteLater here deleteLater(); } @@ -146,7 +149,7 @@ void AudioInjector::restart() { _hasSentFirstFrame = false; // check our state to decide if we need extra handling for the restart request - if ((_state & AudioInjectorState::Finished) == AudioInjectorState::Finished) { + if (stateHas(AudioInjectorState::Finished)) { // we finished playing, need to reset state so we can get going again _hasSetup = false; _shouldStop = false; @@ -216,7 +219,7 @@ static const int64_t NEXT_FRAME_DELTA_ERROR_OR_FINISHED = -1; static const int64_t NEXT_FRAME_DELTA_IMMEDIATELY = 0; int64_t AudioInjector::injectNextFrame() { - if ((_state & AudioInjectorState::NetworkInjectionFinished) == AudioInjectorState::NetworkInjectionFinished) { + if (stateHas(AudioInjectorState::NetworkInjectionFinished)) { qDebug() << "AudioInjector::injectNextFrame called but AudioInjector has finished and was not restarted. Returning."; return NEXT_FRAME_DELTA_ERROR_OR_FINISHED; } diff --git a/libraries/audio/src/AudioInjector.h b/libraries/audio/src/AudioInjector.h index e13fc15c26..ce0c88247e 100644 --- a/libraries/audio/src/AudioInjector.h +++ b/libraries/audio/src/AudioInjector.h @@ -60,7 +60,7 @@ public: AudioInjector(const Sound& sound, const AudioInjectorOptions& injectorOptions); AudioInjector(const QByteArray& audioData, const AudioInjectorOptions& injectorOptions); - bool isFinished() const { return (_state & AudioInjectorState::Finished) == AudioInjectorState::Finished; } + bool isFinished() const { return (stateHas(AudioInjectorState::Finished)); } int getCurrentSendOffset() const { return _currentSendOffset; } void setCurrentSendOffset(int currentSendOffset) { _currentSendOffset = currentSendOffset; } @@ -74,6 +74,7 @@ public: bool isStereo() const { return _options.stereo; } void setLocalAudioInterface(AbstractAudioInterface* localAudioInterface) { _localAudioInterface = localAudioInterface; } + bool stateHas(AudioInjectorState state) const ; static AudioInjector* playSoundAndDelete(const QByteArray& buffer, const AudioInjectorOptions options, AbstractAudioInterface* localInterface); static AudioInjector* playSound(const QByteArray& buffer, const AudioInjectorOptions options, AbstractAudioInterface* localInterface); static AudioInjector* playSound(SharedSoundPointer sound, const float volume, const float stretchFactor, const glm::vec3 position); @@ -89,7 +90,7 @@ public slots: void setOptions(const AudioInjectorOptions& options); float getLoudness() const { return _loudness; } - bool isPlaying() const { return (_state & AudioInjectorState::NotFinished) == AudioInjectorState::NotFinished; } + bool isPlaying() const { return stateHas(AudioInjectorState::NotFinished); } void finish(); void finishLocalInjection(); void finishNetworkInjection(); From 8c0eb1e4d27f4d925317a09fedf7632138f9172e Mon Sep 17 00:00:00 2001 From: David Kelly Date: Fri, 15 Jul 2016 19:48:15 -0700 Subject: [PATCH 08/18] Fixed some bad logic When I "fixed" my or instead of and issue, I did it in the wrong direction. But it looked right :) Now it is. Sigh. Long story how it got there, but it seems good now. --- libraries/audio/src/AudioInjector.cpp | 14 +++++++------- libraries/audio/src/AudioInjector.h | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/libraries/audio/src/AudioInjector.cpp b/libraries/audio/src/AudioInjector.cpp index e992e3c541..9c49ce66d8 100644 --- a/libraries/audio/src/AudioInjector.cpp +++ b/libraries/audio/src/AudioInjector.cpp @@ -32,8 +32,8 @@ AudioInjectorState operator& (AudioInjectorState lhs, AudioInjectorState rhs) { return static_cast(static_cast(lhs) & static_cast(rhs)); }; -AudioInjectorState& operator&= (AudioInjectorState& lhs, AudioInjectorState rhs) { - lhs = static_cast(static_cast(lhs) & static_cast(rhs)); +AudioInjectorState& operator|= (AudioInjectorState& lhs, AudioInjectorState rhs) { + lhs = static_cast(static_cast(lhs) | static_cast(rhs)); return lhs; }; @@ -70,7 +70,7 @@ void AudioInjector::setOptions(const AudioInjectorOptions& options) { } void AudioInjector::finishNetworkInjection() { - _state &= AudioInjectorState::NetworkInjectionFinished; + _state |= AudioInjectorState::NetworkInjectionFinished; // if we are already finished with local // injection, then we are finished @@ -80,14 +80,14 @@ void AudioInjector::finishNetworkInjection() { } void AudioInjector::finishLocalInjection() { - _state &= AudioInjectorState::LocalInjectionFinished; + _state |= AudioInjectorState::LocalInjectionFinished; if(_options.localOnly || stateHas(AudioInjectorState::NetworkInjectionFinished)) { finish(); } } void AudioInjector::finish() { - _state &= AudioInjectorState::Finished; + _state |= AudioInjectorState::Finished; emit finished(); @@ -413,7 +413,7 @@ void AudioInjector::triggerDeleteAfterFinish() { if (_state == AudioInjectorState::Finished) { stopAndDeleteLater(); } else { - _state &= AudioInjectorState::PendingDelete; + _state |= AudioInjectorState::PendingDelete; } } @@ -459,7 +459,7 @@ AudioInjector* AudioInjector::playSoundAndDelete(const QByteArray& buffer, const AudioInjector* sound = playSound(buffer, options, localInterface); if (sound) { - sound->_state &= AudioInjectorState::PendingDelete; + sound->_state |= AudioInjectorState::PendingDelete; } return sound; diff --git a/libraries/audio/src/AudioInjector.h b/libraries/audio/src/AudioInjector.h index ce0c88247e..9bdfcacb5c 100644 --- a/libraries/audio/src/AudioInjector.h +++ b/libraries/audio/src/AudioInjector.h @@ -42,7 +42,7 @@ enum class AudioInjectorState : uint8_t { }; AudioInjectorState operator& (AudioInjectorState lhs, AudioInjectorState rhs); -AudioInjectorState& operator&= (AudioInjectorState& lhs, AudioInjectorState rhs); +AudioInjectorState& operator|= (AudioInjectorState& lhs, AudioInjectorState rhs); // In order to make scripting cleaner for the AudioInjector, the script now holds on to the AudioInjector object // until it dies. From 4b2fb546eb1615ad68200d441c556594e56f9644 Mon Sep 17 00:00:00 2001 From: Anthony Thibault Date: Sat, 16 Jul 2016 12:21:08 -0700 Subject: [PATCH 09/18] Re-enable storing of attach points in user settings * bug fix, use localOffset property of grabbed entity, which is dynamically updated as the near grab action moves the entity. --- .../system/controllers/handControllerGrab.js | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/scripts/system/controllers/handControllerGrab.js b/scripts/system/controllers/handControllerGrab.js index 66c9e10795..36b0725f45 100644 --- a/scripts/system/controllers/handControllerGrab.js +++ b/scripts/system/controllers/handControllerGrab.js @@ -271,8 +271,7 @@ function propsArePhysical(props) { return isPhysical; } -// currently disabled. -var USE_ATTACH_POINT_SETTINGS = false; +var USE_ATTACH_POINT_SETTINGS = true; var ATTACH_POINT_SETTINGS = "io.highfidelity.attachPoints"; function getAttachPointSettings() { @@ -2091,16 +2090,10 @@ function MyController(hand) { this.holdExit = function () { // store the offset attach points into preferences. if (USE_ATTACH_POINT_SETTINGS && this.grabbedHotspot && this.grabbedEntity) { - entityPropertiesCache.addEntity(this.grabbedEntity); - var props = entityPropertiesCache.getProps(this.grabbedEntity); - var entityXform = new Xform(props.rotation, props.position); - var avatarXform = new Xform(MyAvatar.orientation, MyAvatar.position); - var handRot = (this.hand === RIGHT_HAND) ? MyAvatar.getRightPalmRotation() : MyAvatar.getLeftPalmRotation(); - var avatarHandPos = (this.hand === RIGHT_HAND) ? MyAvatar.rightHandPosition : MyAvatar.leftHandPosition; - var palmXform = new Xform(handRot, avatarXform.xformPoint(avatarHandPos)); - var offsetXform = Xform.mul(palmXform.inv(), entityXform); - - storeAttachPointForHotspotInSettings(this.grabbedHotspot, this.hand, offsetXform.pos, offsetXform.rot); + var props = Entities.getEntityProperties(this.grabbedEntity, ["localPosition", "localRotation"]); + if (props && props.localPosition && props.localRotation) { + storeAttachPointForHotspotInSettings(this.grabbedHotspot, this.hand, props.localPosition, props.localRotation); + } } }; From c7c804623166b5ed64a96103053bf8d5271c4e83 Mon Sep 17 00:00:00 2001 From: Anthony Thibault Date: Sat, 16 Jul 2016 14:58:21 -0700 Subject: [PATCH 10/18] De-equip goes into a near grab instead of dropping the object. --- .../system/controllers/handControllerGrab.js | 35 +++++++++++-------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/scripts/system/controllers/handControllerGrab.js b/scripts/system/controllers/handControllerGrab.js index 36b0725f45..f622bf9217 100644 --- a/scripts/system/controllers/handControllerGrab.js +++ b/scripts/system/controllers/handControllerGrab.js @@ -228,7 +228,7 @@ function colorPow(color, power) { return { red: Math.pow(color.red / 255.0, power) * 255, green: Math.pow(color.green / 255.0, power) * 255, - blue: Math.pow(color.blue / 255.0, power) * 255, + blue: Math.pow(color.blue / 255.0, power) * 255 }; } @@ -764,9 +764,8 @@ function MyController(hand) { } }; - var SEARCH_SPHERE_ALPHA = 0.5; this.searchSphereOn = function (location, size, color) { - + var rotation = Quat.lookAt(location, Camera.getPosition(), Vec3.UP); var brightColor = colorPow(color, 0.06); if (this.searchSphere === null) { @@ -789,7 +788,7 @@ function MyController(hand) { position: location, rotation: rotation, innerColor: brightColor, - outerColor: color, + outerColor: color, innerAlpha: 1.0, outerAlpha: 0.0, outerRadius: size * 1.2, @@ -1960,12 +1959,12 @@ function MyController(hand) { this.currentObjectRotation = grabbedProperties.rotation; this.currentVelocity = ZERO_VEC; this.currentAngularVelocity = ZERO_VEC; + + this.prevDropDetected = false; }; this.nearGrabbing = function (deltaTime, timestamp) { - var dropDetected = this.dropGestureProcess(deltaTime); - if (this.state == STATE_NEAR_GRABBING && this.triggerSmoothedReleased()) { this.callEntityMethodOnGrabbed("releaseGrab"); this.setState(STATE_OFF, "trigger released"); @@ -1974,6 +1973,16 @@ function MyController(hand) { if (this.state == STATE_HOLD) { + var dropDetected = this.dropGestureProcess(deltaTime); + + if (this.triggerSmoothedReleased()) { + this.waitForTriggerRelease = false; + } + + if (dropDetected && this.prevDropDetected != dropDetected) { + this.waitForTriggerRelease = true; + } + // highlight the grabbed hotspot when the dropGesture is detected. if (dropDetected) { entityPropertiesCache.addEntity(this.grabbedHotspot.entityID); @@ -1981,17 +1990,15 @@ function MyController(hand) { equipHotspotBuddy.highlightHotspot(this.grabbedHotspot); } - if (dropDetected && this.triggerSmoothedGrab()) { + if (dropDetected && !this.waitForTriggerRelease && this.triggerSmoothedGrab()) { this.callEntityMethodOnGrabbed("releaseEquip"); - this.setState(STATE_OFF, "drop gesture detected"); - return; - } - - if (this.thumbPressed()) { - this.callEntityMethodOnGrabbed("releaseEquip"); - this.setState(STATE_OFF, "drop via thumb press"); + var grabbedEntity = this.grabbedEntity; + this.release(); + this.grabbedEntity = grabbedEntity; + this.setState(STATE_NEAR_GRABBING, "drop gesture detected"); return; } + this.prevDropDetected = dropDetected; } this.heartBeat(this.grabbedEntity); From 0f240d39b61905313bd76127752d393cf65b6ff4 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Mon, 18 Jul 2016 11:21:09 -0700 Subject: [PATCH 11/18] Actually delete textures we're not using --- libraries/gpu-gl/src/gpu/gl/GLTexture.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libraries/gpu-gl/src/gpu/gl/GLTexture.cpp b/libraries/gpu-gl/src/gpu/gl/GLTexture.cpp index ed931437b7..74428b53f5 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLTexture.cpp +++ b/libraries/gpu-gl/src/gpu/gl/GLTexture.cpp @@ -187,7 +187,11 @@ GLTexture::~GLTexture() { } } - Backend::decrementTextureGPUCount(); + if (_id) { + glDeleteTextures(1, &_id); + const_cast(_id) = 0; + Backend::decrementTextureGPUCount(); + } Backend::updateTextureGPUMemoryUsage(_size, 0); Backend::updateTextureGPUVirtualMemoryUsage(_virtualSize, 0); } From 3df373252f8cbe7ae7e227e5bd545a7472085886 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Mon, 18 Jul 2016 14:00:41 -0700 Subject: [PATCH 12/18] Several minor things We could only partially fill the _scratchBuffer - .wav files may not be exactly N frames long. Doh. While at it, I needed to call finishLocalInjection() after local injectors are done, and the access to the injector vector needs to be locked, given that we do a QtDirectConnection with the networking and thus the outputLocalInjectors is on a different thread. The clicking was just 0-ing out the _scratchBuffer. --- libraries/audio-client/src/AudioClient.cpp | 9 +++++++-- libraries/audio-client/src/AudioClient.h | 5 +++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/libraries/audio-client/src/AudioClient.cpp b/libraries/audio-client/src/AudioClient.cpp index b5a7c8c0cf..7cf8574529 100644 --- a/libraries/audio-client/src/AudioClient.cpp +++ b/libraries/audio-client/src/AudioClient.cpp @@ -862,6 +862,9 @@ void AudioClient::mixLocalAudioInjectors(int16_t* inputBuffer) { static const float INT16_TO_FLOAT_SCALE_FACTOR = 1/32768.0f; bool injectorsHaveData = false; + + // lock the injector vector + Lock lock(_injectorsMutex); for (AudioInjector* injector : getActiveLocalAudioInjectors()) { if (injector->getLocalBuffer()) { @@ -871,6 +874,7 @@ void AudioClient::mixLocalAudioInjectors(int16_t* inputBuffer) { AudioConstants::NETWORK_FRAME_BYTES_PER_CHANNEL; // get one frame from the injector (mono or stereo) + memset(_scratchBuffer, 0, sizeof(_scratchBuffer)); if (0 < injector->getLocalBuffer()->readData((char*)_scratchBuffer, samplesToRead)) { injectorsHaveData = true; @@ -894,14 +898,14 @@ void AudioClient::mixLocalAudioInjectors(int16_t* inputBuffer) { } else { qDebug() << "injector has no more data, marking finished for removal"; - injector->finish(); + injector->finishLocalInjection(); injectorsToRemove.append(injector); } } else { qDebug() << "injector has no local buffer, marking as finished for removal"; - injector->finish(); + injector->finishLocalInjection(); injectorsToRemove.append(injector); } } @@ -1003,6 +1007,7 @@ void AudioClient::setIsStereoInput(bool isStereoInput) { bool AudioClient::outputLocalInjector(bool isStereo, AudioInjector* injector) { + Lock lock(_injectorsMutex); if (injector->getLocalBuffer() && _audioInput ) { // just add it to the vector of active local injectors, if // not already there. diff --git a/libraries/audio-client/src/AudioClient.h b/libraries/audio-client/src/AudioClient.h index 3e4aa931a6..472092163b 100644 --- a/libraries/audio-client/src/AudioClient.h +++ b/libraries/audio-client/src/AudioClient.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -83,6 +84,9 @@ public: using AudioPositionGetter = std::function; using AudioOrientationGetter = std::function; + using Mutex = std::mutex; + using Lock = std::unique_lock; + class AudioOutputIODevice : public QIODevice { public: AudioOutputIODevice(MixedProcessedAudioStream& receivedAudioStream, AudioClient* audio) : @@ -219,6 +223,7 @@ private: float azimuthForSource(const glm::vec3& relativePosition); float gainForSource(float distance, float volume); + Mutex _injectorsMutex; QByteArray firstInputFrame; QAudioInput* _audioInput; QAudioFormat _desiredInputFormat; From 659e1ff9845b80a9b0da64019adc77ff8015dc95 Mon Sep 17 00:00:00 2001 From: "Anthony J. Thibault" Date: Mon, 18 Jul 2016 14:17:06 -0700 Subject: [PATCH 13/18] remove debug prints --- scripts/system/controllers/handControllerGrab.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/scripts/system/controllers/handControllerGrab.js b/scripts/system/controllers/handControllerGrab.js index f622bf9217..43152f763f 100644 --- a/scripts/system/controllers/handControllerGrab.js +++ b/scripts/system/controllers/handControllerGrab.js @@ -277,7 +277,6 @@ var ATTACH_POINT_SETTINGS = "io.highfidelity.attachPoints"; function getAttachPointSettings() { try { var str = Settings.getValue(ATTACH_POINT_SETTINGS); - print("getAttachPointSettings = " + str); if (str === "false") { return {}; } else { @@ -290,7 +289,6 @@ function getAttachPointSettings() { } function setAttachPointSettings(attachPointSettings) { var str = JSON.stringify(attachPointSettings); - print("setAttachPointSettings = " + str); Settings.setValue(ATTACH_POINT_SETTINGS, str); } function getAttachPointForHotspotFromSettings(hotspot, hand) { From 69500643f3592ab3a669f12ed39879941b7ee87d Mon Sep 17 00:00:00 2001 From: "Anthony J. Thibault" Date: Mon, 18 Jul 2016 14:17:32 -0700 Subject: [PATCH 14/18] bugfix for saving attach points The feature to add transition from equip -> near-grab, in inadvertently broke attach point saving. --- .../system/controllers/handControllerGrab.js | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/scripts/system/controllers/handControllerGrab.js b/scripts/system/controllers/handControllerGrab.js index 43152f763f..cf8146fba9 100644 --- a/scripts/system/controllers/handControllerGrab.js +++ b/scripts/system/controllers/handControllerGrab.js @@ -202,8 +202,7 @@ CONTROLLER_STATE_MACHINE[STATE_NEAR_GRABBING] = { CONTROLLER_STATE_MACHINE[STATE_HOLD] = { name: "hold", enterMethod: "nearGrabbingEnter", - updateMethod: "nearGrabbing", - exitMethod: "holdExit" + updateMethod: "nearGrabbing" }; CONTROLLER_STATE_MACHINE[STATE_NEAR_TRIGGER] = { name: "trigger", @@ -1990,6 +1989,15 @@ function MyController(hand) { if (dropDetected && !this.waitForTriggerRelease && this.triggerSmoothedGrab()) { this.callEntityMethodOnGrabbed("releaseEquip"); + + // store the offset attach points into preferences. + if (USE_ATTACH_POINT_SETTINGS && this.grabbedHotspot && this.grabbedEntity) { + var props = Entities.getEntityProperties(this.grabbedEntity, ["localPosition", "localRotation"]); + if (props && props.localPosition && props.localRotation) { + storeAttachPointForHotspotInSettings(this.grabbedHotspot, this.hand, props.localPosition, props.localRotation); + } + } + var grabbedEntity = this.grabbedEntity; this.release(); this.grabbedEntity = grabbedEntity; @@ -2092,16 +2100,6 @@ function MyController(hand) { } }; - this.holdExit = function () { - // store the offset attach points into preferences. - if (USE_ATTACH_POINT_SETTINGS && this.grabbedHotspot && this.grabbedEntity) { - var props = Entities.getEntityProperties(this.grabbedEntity, ["localPosition", "localRotation"]); - if (props && props.localPosition && props.localRotation) { - storeAttachPointForHotspotInSettings(this.grabbedHotspot, this.hand, props.localPosition, props.localRotation); - } - } - }; - this.nearTriggerEnter = function () { this.clearEquipHaptics(); From 2d73b23c5693d48cee876752b61dd63e4095b59f Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Mon, 18 Jul 2016 14:44:41 -0700 Subject: [PATCH 15/18] when deciding on the release velocity of something thrown, don't include a zero velocity caused by seeing the same controller data as last frame --- interface/src/avatar/AvatarActionHold.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/interface/src/avatar/AvatarActionHold.cpp b/interface/src/avatar/AvatarActionHold.cpp index bac3b1e02f..5acee052f2 100644 --- a/interface/src/avatar/AvatarActionHold.cpp +++ b/interface/src/avatar/AvatarActionHold.cpp @@ -207,8 +207,10 @@ void AvatarActionHold::doKinematicUpdate(float deltaTimeStep) { } withWriteLock([&]{ - if (_previousSet) { + if (_previousSet && + _positionalTarget != _previousPositionalTarget) { // don't average in a zero velocity if we get the same data glm::vec3 oneFrameVelocity = (_positionalTarget - _previousPositionalTarget) / deltaTimeStep; + _measuredLinearVelocities[_measuredLinearVelocitiesIndex++] = oneFrameVelocity; if (_measuredLinearVelocitiesIndex >= AvatarActionHold::velocitySmoothFrames) { _measuredLinearVelocitiesIndex = 0; @@ -228,9 +230,9 @@ void AvatarActionHold::doKinematicUpdate(float deltaTimeStep) { // 3 -- ignore i of 0 1 2 // 4 -- ignore i of 1 2 3 // 5 -- ignore i of 2 3 4 - if ((i + 1) % 6 == _measuredLinearVelocitiesIndex || - (i + 2) % 6 == _measuredLinearVelocitiesIndex || - (i + 3) % 6 == _measuredLinearVelocitiesIndex) { + if ((i + 1) % AvatarActionHold::velocitySmoothFrames == _measuredLinearVelocitiesIndex || + (i + 2) % AvatarActionHold::velocitySmoothFrames == _measuredLinearVelocitiesIndex || + (i + 3) % AvatarActionHold::velocitySmoothFrames == _measuredLinearVelocitiesIndex) { continue; } measuredLinearVelocity += _measuredLinearVelocities[i]; From df615b1503ab554682cf6f383461b0008745d90b Mon Sep 17 00:00:00 2001 From: David Kelly Date: Mon, 18 Jul 2016 16:02:02 -0700 Subject: [PATCH 16/18] NotFinished never should have had its own bit Since there is a Finished flag too. So now, it is just 0, used as a starting point, and we check for !hasState(Finished). --- libraries/audio/src/AudioInjector.h | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/libraries/audio/src/AudioInjector.h b/libraries/audio/src/AudioInjector.h index 9bdfcacb5c..b872800e15 100644 --- a/libraries/audio/src/AudioInjector.h +++ b/libraries/audio/src/AudioInjector.h @@ -34,11 +34,11 @@ class AudioInjectorManager; enum class AudioInjectorState : uint8_t { - NotFinished = 1, - Finished = 2, - PendingDelete = 4, - LocalInjectionFinished = 8, - NetworkInjectionFinished = 16 + NotFinished = 0, + Finished = 1, + PendingDelete = 2, + LocalInjectionFinished = 4, + NetworkInjectionFinished = 8 }; AudioInjectorState operator& (AudioInjectorState lhs, AudioInjectorState rhs); @@ -50,12 +50,6 @@ class AudioInjector : public QObject { Q_OBJECT public: - static const uint8_t NotFinished = 1; - static const uint8_t Finished = 2; - static const uint8_t PendingDelete = 4; - static const uint8_t LocalInjectionFinished = 8; - static const uint8_t NetworkInjectionFinished = 16; - AudioInjector(QObject* parent); AudioInjector(const Sound& sound, const AudioInjectorOptions& injectorOptions); AudioInjector(const QByteArray& audioData, const AudioInjectorOptions& injectorOptions); @@ -90,7 +84,7 @@ public slots: void setOptions(const AudioInjectorOptions& options); float getLoudness() const { return _loudness; } - bool isPlaying() const { return stateHas(AudioInjectorState::NotFinished); } + bool isPlaying() const { return !stateHas(AudioInjectorState::Finished); } void finish(); void finishLocalInjection(); void finishNetworkInjection(); From 2a89fa25bbc64f69bd5717e84af9bee3e5120962 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Mon, 18 Jul 2016 16:22:12 -0700 Subject: [PATCH 17/18] Removing comments Since I looked into it, seems ok --- libraries/audio/src/AudioInjector.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/libraries/audio/src/AudioInjector.cpp b/libraries/audio/src/AudioInjector.cpp index 9c49ce66d8..58122fee3c 100644 --- a/libraries/audio/src/AudioInjector.cpp +++ b/libraries/audio/src/AudioInjector.cpp @@ -165,14 +165,10 @@ void AudioInjector::restart() { if (!_options.localOnly) { if (!injectorManager->restartFinishedInjector(this)) { - // TODO: this logic seems to remove the pending delete, - // which makes me wonder about the deleteLater calls _state = AudioInjectorState::Finished; // we're not playing, so reset the state used by isPlaying. } } } else { - // TODO: this logic seems to remove the pending delete, - // which makes me wonder about the deleteLater calls _state = AudioInjectorState::Finished; // we failed to play, so we are finished again } } From cbbb2a7975b7276170f5fd85c7473fdc9d8eedf5 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Mon, 18 Jul 2016 19:04:17 -0700 Subject: [PATCH 18/18] Properly delete GL context and offscreen surfaces --- libraries/gl/src/gl/OffscreenGLCanvas.cpp | 8 ++++++++ libraries/gl/src/gl/OffscreenGLCanvas.h | 4 ++-- libraries/gl/src/gl/QOpenGLContextWrapper.cpp | 15 ++++++++------- libraries/gl/src/gl/QOpenGLContextWrapper.h | 2 ++ 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/libraries/gl/src/gl/OffscreenGLCanvas.cpp b/libraries/gl/src/gl/OffscreenGLCanvas.cpp index eec3d2bf6b..a6b5a03ff6 100644 --- a/libraries/gl/src/gl/OffscreenGLCanvas.cpp +++ b/libraries/gl/src/gl/OffscreenGLCanvas.cpp @@ -36,7 +36,15 @@ OffscreenGLCanvas::~OffscreenGLCanvas() { delete _logger; _logger = nullptr; } + _context->doneCurrent(); + delete _context; + _context = nullptr; + + _offscreenSurface->destroy(); + delete _offscreenSurface; + _offscreenSurface = nullptr; + } bool OffscreenGLCanvas::create(QOpenGLContext* sharedContext) { diff --git a/libraries/gl/src/gl/OffscreenGLCanvas.h b/libraries/gl/src/gl/OffscreenGLCanvas.h index 69210f638d..8def09796d 100644 --- a/libraries/gl/src/gl/OffscreenGLCanvas.h +++ b/libraries/gl/src/gl/OffscreenGLCanvas.h @@ -34,8 +34,8 @@ public: protected: std::once_flag _reportOnce; - QOpenGLContext* _context; - QOffscreenSurface* _offscreenSurface; + QOpenGLContext* _context{ nullptr }; + QOffscreenSurface* _offscreenSurface{ nullptr }; QOpenGLDebugLogger* _logger{ nullptr }; }; diff --git a/libraries/gl/src/gl/QOpenGLContextWrapper.cpp b/libraries/gl/src/gl/QOpenGLContextWrapper.cpp index bc8afc3927..0b153a5ae8 100644 --- a/libraries/gl/src/gl/QOpenGLContextWrapper.cpp +++ b/libraries/gl/src/gl/QOpenGLContextWrapper.cpp @@ -28,16 +28,17 @@ QOpenGLContext* QOpenGLContextWrapper::currentContext() { return QOpenGLContext::currentContext(); } - QOpenGLContextWrapper::QOpenGLContextWrapper() : -_context(new QOpenGLContext) -{ -} - + _ownContext(true), _context(new QOpenGLContext) { } QOpenGLContextWrapper::QOpenGLContextWrapper(QOpenGLContext* context) : - _context(context) -{ + _context(context) { } + +QOpenGLContextWrapper::~QOpenGLContextWrapper() { + if (_ownContext) { + delete _context; + _context = nullptr; + } } void QOpenGLContextWrapper::setFormat(const QSurfaceFormat& format) { diff --git a/libraries/gl/src/gl/QOpenGLContextWrapper.h b/libraries/gl/src/gl/QOpenGLContextWrapper.h index d097284e68..b09ad1a4c3 100644 --- a/libraries/gl/src/gl/QOpenGLContextWrapper.h +++ b/libraries/gl/src/gl/QOpenGLContextWrapper.h @@ -23,6 +23,7 @@ class QOpenGLContextWrapper { public: QOpenGLContextWrapper(); QOpenGLContextWrapper(QOpenGLContext* context); + virtual ~QOpenGLContextWrapper(); void setFormat(const QSurfaceFormat& format); bool create(); void swapBuffers(QSurface* surface); @@ -40,6 +41,7 @@ public: private: + bool _ownContext { false }; QOpenGLContext* _context { nullptr }; };