From fb99828e30302f2c615e74926dbe62a647def972 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Fri, 15 Jul 2016 17:09:27 -0700 Subject: [PATCH] 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();