Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Binaural Monitoring fixes #183

Merged
merged 14 commits into from
Feb 17, 2022
Merged

Binaural Monitoring fixes #183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
7103e61
Safer weak_ptr locking
rsjbailey Feb 11, 2022
3392468
remove pointless weak_ptr copy
rsjbailey Feb 16, 2022
6591ed5
Initial inversion implementation
firthm01 Feb 15, 2022
f7cbf5d
Save to config file (needs mac path test!)
firthm01 Feb 15, 2022
70eff83
Config File path good on OSX
firthm01 Feb 15, 2022
dc8bfff
More helpful connection error message
firthm01 Feb 15, 2022
55c8f83
Correct order of config restore to ensure enacted
firthm01 Feb 15, 2022
851acf2
Patch JUCE so Win can detect UDP port open fail
firthm01 Feb 15, 2022
bf16341
Don't crash if bin mon receives 0.6.0 metadata
firthm01 Feb 15, 2022
cf8e699
Don't need to rely on message receipt to confirm successful listen
firthm01 Feb 15, 2022
dbd232f
isValidId function and pass const to vectorContains
firthm01 Feb 16, 2022
bfa9e21
Save version to config file and store in userApplicationDataDirectory
firthm01 Feb 16, 2022
01068f1
public Inversions struct and accept as setInverts arg
firthm01 Feb 16, 2022
d4eca1f
Merge branch 'main' into bin-mon-fixes
firthm01 Feb 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions ...roduction-suite-plugins/lib/include/ui/binaural_monitoring_frontend_backend_connector.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ class EAR_PLUGIN_BASE_EXPORT BinauralMonitoringFrontendBackendConnector {
ROLL,
OSC_ENABLE,
OSC_PORT,
OSC_INVERT_YAW,
OSC_INVERT_PITCH,
OSC_INVERT_ROLL,
OSC_INVERT_QUAT_W,
OSC_INVERT_QUAT_X,
OSC_INVERT_QUAT_Y,
OSC_INVERT_QUAT_Z,
};

using ParameterChangedCallback =
Expand Down
27 changes: 23 additions & 4 deletions ear-production-suite-plugins/lib/src/binaural_monitoring_backend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,17 @@
using std::placeholders::_1;
using std::placeholders::_2;

namespace {
template<typename T>
bool vectorContains(std::vector<T> const& v, T k) {
return std::find(v.begin(), v.end(), k) != v.end();
}

bool isValidId(std::string const& id) {
return !(id.empty() || id == "00000000-0000-0000-0000-000000000000");
}
}

namespace ear {
namespace plugin {

Expand Down Expand Up @@ -200,7 +211,14 @@ void BinauralMonitoringBackend::onSceneReceived(proto::SceneStore store) {
size_t totalObjChannels = 0;
size_t totalHoaChannels = 0;

std::vector<ConnId> availableItemIds;
availableItemIds.reserve(store.all_available_items_size());

for (const auto& item : store.all_available_items()) {
if(item.has_connection_id() &&
isValidId(item.connection_id())) {
availableItemIds.push_back(item.connection_id());
}
if (item.has_ds_metadata()) {
totalDsChannels += item.ds_metadata().speakers_size();
}
Expand Down Expand Up @@ -231,10 +249,11 @@ void BinauralMonitoringBackend::onSceneReceived(proto::SceneStore store) {

for (const auto& item : store.monitoring_items()) {
if (item.has_connection_id() &&
item.connection_id() != "00000000-0000-0000-0000-000000000000" &&
item.connection_id() != "") {
bool newItem = std::find(allActiveIds.begin(), allActiveIds.end(),
item.connection_id()) == allActiveIds.end();
isValidId(item.connection_id()) &&
vectorContains(availableItemIds, item.connection_id())) {

bool newItem = !vectorContains(allActiveIds, item.connection_id());

// clang-format off
if (item.has_hoa_metadata()) {
if(newItem || item.changed()) {
Expand Down
202 changes: 200 additions & 2 deletions ...-suite-plugins/plugins/binaural_monitoring/src/binaural_monitoring_frontend_connector.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,27 @@ BinauralMonitoringJuceFrontendConnector::
if (auto oscControl = oscPortControl_.lock()) {
oscControl->removeListener(this);
}
if (auto oscControl = oscInvertYawButton_.lock()) {
oscControl->removeListener(this);
}
if (auto oscControl = oscInvertPitchButton_.lock()) {
oscControl->removeListener(this);
}
if (auto oscControl = oscInvertRollButton_.lock()) {
oscControl->removeListener(this);
}
if (auto oscControl = oscInvertQuatWButton_.lock()) {
oscControl->removeListener(this);
}
if (auto oscControl = oscInvertQuatXButton_.lock()) {
oscControl->removeListener(this);
}
if (auto oscControl = oscInvertQuatYButton_.lock()) {
oscControl->removeListener(this);
}
if (auto oscControl = oscInvertQuatZButton_.lock()) {
oscControl->removeListener(this);
}

if (listenerOrientation) {
listenerOrientation->removeListener(this);
Expand Down Expand Up @@ -102,6 +123,55 @@ void BinauralMonitoringJuceFrontendConnector::setOscPortControl(
setOscPort(cachedOscPort_);
}

void BinauralMonitoringJuceFrontendConnector::setOscInvertYawButton(
std::shared_ptr<ToggleButton> button) {
button->addListener(this);
oscInvertYawButton_ = button;
setOscInvertYaw(cachedOscInvertYaw_);
Comment on lines +127 to +130
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpicking again, could std::move all the passed buttons into their member vars to avoid copying the shared_ptr twice (once in pass-by-value, once in the copy-assign), e.g.
oscInvertPitchButton_ = std::move(button);
In practice probably doesn't matter as these methods won't get called often.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a C+P of the existing pattern in other plugins so we should change this everywhere if we're going to tackle it here.

}

void BinauralMonitoringJuceFrontendConnector::setOscInvertPitchButton(
std::shared_ptr<ToggleButton> button) {
button->addListener(this);
oscInvertPitchButton_ = button;
setOscInvertPitch(cachedOscInvertPitch_);
}

void BinauralMonitoringJuceFrontendConnector::setOscInvertRollButton(
std::shared_ptr<ToggleButton> button) {
button->addListener(this);
oscInvertRollButton_ = button;
setOscInvertRoll(cachedOscInvertRoll_);
}

void BinauralMonitoringJuceFrontendConnector::setOscInvertQuatWButton(
std::shared_ptr<ToggleButton> button) {
button->addListener(this);
oscInvertQuatWButton_ = button;
setOscInvertQuatW(cachedOscInvertQuatW_);
}

void BinauralMonitoringJuceFrontendConnector::setOscInvertQuatXButton(
std::shared_ptr<ToggleButton> button) {
button->addListener(this);
oscInvertQuatXButton_ = button;
setOscInvertQuatX(cachedOscInvertQuatX_);
}

void BinauralMonitoringJuceFrontendConnector::setOscInvertQuatYButton(
std::shared_ptr<ToggleButton> button) {
button->addListener(this);
oscInvertQuatYButton_ = button;
setOscInvertQuatY(cachedOscInvertQuatY_);
}

void BinauralMonitoringJuceFrontendConnector::setOscInvertQuatZButton(
std::shared_ptr<ToggleButton> button) {
button->addListener(this);
oscInvertQuatZButton_ = button;
setOscInvertQuatZ(cachedOscInvertQuatZ_);
}

void BinauralMonitoringJuceFrontendConnector::setListenerOrientationInstance(
std::shared_ptr<ListenerOrientation> lo) {
listenerOrientation = lo;
Expand Down Expand Up @@ -137,6 +207,7 @@ void BinauralMonitoringJuceFrontendConnector::setEuler(
if (listenerOrientation) {
listenerOrientation->setEuler(euler);
}

}

void BinauralMonitoringJuceFrontendConnector::setQuaternion(
Expand All @@ -160,6 +231,55 @@ void BinauralMonitoringJuceFrontendConnector::setOscPort(int port) {
cachedOscPort_ = port;
}

void BinauralMonitoringJuceFrontendConnector::setOscInvertYaw(bool invert) {
if (auto oscInvertButtonLocked = oscInvertYawButton_.lock()) {
oscInvertButtonLocked->setToggleState(invert, dontSendNotification);
}
cachedOscInvertYaw_ = invert;
}

void BinauralMonitoringJuceFrontendConnector::setOscInvertPitch(bool invert) {
if (auto oscInvertButtonLocked = oscInvertPitchButton_.lock()) {
oscInvertButtonLocked->setToggleState(invert, dontSendNotification);
}
cachedOscInvertPitch_ = invert;
}

void BinauralMonitoringJuceFrontendConnector::setOscInvertRoll(bool invert) {
if (auto oscInvertButtonLocked = oscInvertRollButton_.lock()) {
oscInvertButtonLocked->setToggleState(invert, dontSendNotification);
}
cachedOscInvertRoll_ = invert;
}

void BinauralMonitoringJuceFrontendConnector::setOscInvertQuatW(bool invert) {
if (auto oscInvertButtonLocked = oscInvertQuatWButton_.lock()) {
oscInvertButtonLocked->setToggleState(invert, dontSendNotification);
}
cachedOscInvertQuatW_ = invert;
}

void BinauralMonitoringJuceFrontendConnector::setOscInvertQuatX(bool invert) {
if (auto oscInvertButtonLocked = oscInvertQuatXButton_.lock()) {
oscInvertButtonLocked->setToggleState(invert, dontSendNotification);
}
cachedOscInvertQuatX_ = invert;
}

void BinauralMonitoringJuceFrontendConnector::setOscInvertQuatY(bool invert) {
if (auto oscInvertButtonLocked = oscInvertQuatYButton_.lock()) {
oscInvertButtonLocked->setToggleState(invert, dontSendNotification);
}
cachedOscInvertQuatY_ = invert;
}

void BinauralMonitoringJuceFrontendConnector::setOscInvertQuatZ(bool invert) {
if (auto oscInvertButtonLocked = oscInvertQuatZButton_.lock()) {
oscInvertButtonLocked->setToggleState(invert, dontSendNotification);
}
cachedOscInvertQuatZ_ = invert;
}

void BinauralMonitoringJuceFrontendConnector::orientationChange(
ear::plugin::ListenerOrientation::Euler euler) {
// ListenerOrientation callback from backend
Expand Down Expand Up @@ -220,6 +340,56 @@ void BinauralMonitoringJuceFrontendConnector::parameterValueChanged(
notifyParameterChanged(ParameterId::OSC_PORT, p_->getOscPort()->get());
setOscPort(p_->getOscPort()->get());
});

} else if (parameterIndex == (int)ParameterId::OSC_INVERT_YAW) {
updater_.callOnMessageThread([this, parameterIndex, newValue]() {
notifyParameterChanged(ParameterId::OSC_INVERT_YAW,
p_->getOscInvertYaw()->get());
setOscInvertYaw(p_->getOscInvertYaw()->get());
});
Comment on lines +344 to +349
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mentioned I'd noticed an odd pattern when I was going through these classes - this is it.
Not sure why we are capturing newValue, then retrieving it from the parameter rather then just using the value? Looks like a common pattern pretty much everywhere.

I guess we should either use newValue rather than doing p_->xxx->get(), or remove it from the lambda captures and have it as an unused/unnamed parameter in the parameterValueChanged function?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good point. I wonder if it's because it's on a different thread and it's better to just get the "current" value than the one passed in?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is another C+P of an existing pattern - should be tidied everywhere


} else if (parameterIndex == (int)ParameterId::OSC_INVERT_PITCH) {
updater_.callOnMessageThread([this, parameterIndex, newValue]() {
notifyParameterChanged(ParameterId::OSC_INVERT_PITCH,
p_->getOscInvertPitch()->get());
setOscInvertPitch(p_->getOscInvertPitch()->get());
});

} else if (parameterIndex == (int)ParameterId::OSC_INVERT_ROLL) {
updater_.callOnMessageThread([this, parameterIndex, newValue]() {
notifyParameterChanged(ParameterId::OSC_INVERT_ROLL,
p_->getOscInvertRoll()->get());
setOscInvertRoll(p_->getOscInvertRoll()->get());
});

} else if (parameterIndex == (int)ParameterId::OSC_INVERT_QUAT_W) {
updater_.callOnMessageThread([this, parameterIndex, newValue]() {
notifyParameterChanged(ParameterId::OSC_INVERT_QUAT_W,
p_->getOscInvertQuatW()->get());
setOscInvertQuatW(p_->getOscInvertQuatW()->get());
});

} else if (parameterIndex == (int)ParameterId::OSC_INVERT_QUAT_X) {
updater_.callOnMessageThread([this, parameterIndex, newValue]() {
notifyParameterChanged(ParameterId::OSC_INVERT_QUAT_X,
p_->getOscInvertQuatX()->get());
setOscInvertQuatX(p_->getOscInvertQuatX()->get());
});

} else if (parameterIndex == (int)ParameterId::OSC_INVERT_QUAT_Y) {
updater_.callOnMessageThread([this, parameterIndex, newValue]() {
notifyParameterChanged(ParameterId::OSC_INVERT_QUAT_Y,
p_->getOscInvertQuatY()->get());
setOscInvertQuatY(p_->getOscInvertQuatY()->get());
});

} else if (parameterIndex == (int)ParameterId::OSC_INVERT_QUAT_Z) {
updater_.callOnMessageThread([this, parameterIndex, newValue]() {
notifyParameterChanged(ParameterId::OSC_INVERT_QUAT_Z,
p_->getOscInvertQuatZ()->get());
setOscInvertQuatZ(p_->getOscInvertQuatZ()->get());
});

}
}

Expand Down Expand Up @@ -270,12 +440,40 @@ void BinauralMonitoringJuceFrontendConnector::orientationDragEnded(
}

void BinauralMonitoringJuceFrontendConnector::buttonClicked(Button* button) {
// note: getToggleState still has the old value for toggle types when this is called
// due to setClickingTogglesState on the button intentionally being false
if (auto oscButton = lockIfSame(oscEnableButton_, button)) {
// note: getToggleState still has the old value when this is called
// due to setClickingTogglesState on the button being false
bool newState = !oscButton->getToggleState();
*(p_->getOscEnable()) = newState;
}
if (auto oscButton = lockIfSame(oscInvertYawButton_, button)) {
bool newState = !oscButton->getToggleState();
*(p_->getOscInvertYaw()) = newState;
}
if (auto oscButton = lockIfSame(oscInvertPitchButton_, button)) {
bool newState = !oscButton->getToggleState();
*(p_->getOscInvertPitch()) = newState;
}
if (auto oscButton = lockIfSame(oscInvertRollButton_, button)) {
bool newState = !oscButton->getToggleState();
*(p_->getOscInvertRoll()) = newState;
}
if (auto oscButton = lockIfSame(oscInvertQuatWButton_, button)) {
bool newState = !oscButton->getToggleState();
*(p_->getOscInvertQuatW()) = newState;
}
if (auto oscButton = lockIfSame(oscInvertQuatXButton_, button)) {
bool newState = !oscButton->getToggleState();
*(p_->getOscInvertQuatX()) = newState;
}
if (auto oscButton = lockIfSame(oscInvertQuatYButton_, button)) {
bool newState = !oscButton->getToggleState();
*(p_->getOscInvertQuatY()) = newState;
}
if (auto oscButton = lockIfSame(oscInvertQuatZButton_, button)) {
bool newState = !oscButton->getToggleState();
*(p_->getOscInvertQuatZ()) = newState;
}
}

void BinauralMonitoringJuceFrontendConnector::sliderValueChanged(
Expand Down
29 changes: 29 additions & 0 deletions ...-suite-plugins/plugins/binaural_monitoring/src/binaural_monitoring_frontend_connector.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ class BinauralMonitoringJuceFrontendConnector
// OSC Controls
void setOscEnableButton(std::shared_ptr<EarButton> button);
void setOscPortControl(std::shared_ptr<EarSlider> slider);
void setOscInvertYawButton(std::shared_ptr<ToggleButton> button);
void setOscInvertPitchButton(std::shared_ptr<ToggleButton> button);
void setOscInvertRollButton(std::shared_ptr<ToggleButton> button);
void setOscInvertQuatWButton(std::shared_ptr<ToggleButton> button);
void setOscInvertQuatXButton(std::shared_ptr<ToggleButton> button);
void setOscInvertQuatYButton(std::shared_ptr<ToggleButton> button);
void setOscInvertQuatZButton(std::shared_ptr<ToggleButton> button);

// Listener Orientation Object
void setListenerOrientationInstance(std::shared_ptr<ListenerOrientation> lo);
Expand All @@ -57,6 +64,13 @@ class BinauralMonitoringJuceFrontendConnector

void setOscEnable(bool enable);
void setOscPort(int port);
void setOscInvertYaw(bool invert);
void setOscInvertPitch(bool invert);
void setOscInvertRoll(bool invert);
void setOscInvertQuatW(bool invert);
void setOscInvertQuatX(bool invert);
void setOscInvertQuatY(bool invert);
void setOscInvertQuatZ(bool invert);

protected:
// Orientation::Listener
Expand Down Expand Up @@ -90,10 +104,25 @@ class BinauralMonitoringJuceFrontendConnector
// OSC Controls
std::weak_ptr<EarButton> oscEnableButton_;
std::weak_ptr<EarSlider> oscPortControl_;
std::weak_ptr<ToggleButton> oscInvertYawButton_;
std::weak_ptr<ToggleButton> oscInvertPitchButton_;
std::weak_ptr<ToggleButton> oscInvertRollButton_;
std::weak_ptr<ToggleButton> oscInvertQuatWButton_;
std::weak_ptr<ToggleButton> oscInvertQuatXButton_;
std::weak_ptr<ToggleButton> oscInvertQuatYButton_;
std::weak_ptr<ToggleButton> oscInvertQuatZButton_;

// Values
bool cachedOscEnable_;
int cachedOscPort_;
bool cachedOscInvertYaw_;
bool cachedOscInvertPitch_;
bool cachedOscInvertRoll_;
bool cachedOscInvertQuatW_;
bool cachedOscInvertQuatX_;
bool cachedOscInvertQuatY_;
bool cachedOscInvertQuatZ_;

/// Listener Orientation Object
std::shared_ptr<ListenerOrientation> listenerOrientation;

Expand Down
Loading