Do not hold the lock while invoking callbacks
Holding the lock creates a burden on the client, so easier to handle
here.
Test: Manual verification using logs.
Bug: 188502620
Change-Id: I1b3a417315bd05bd421b8d643fd15f832faace02
diff --git a/services/audiopolicy/service/SpatializerPoseController.cpp b/services/audiopolicy/service/SpatializerPoseController.cpp
index 3d79c9d..91eb47f 100644
--- a/services/audiopolicy/service/SpatializerPoseController.cpp
+++ b/services/audiopolicy/service/SpatializerPoseController.cpp
@@ -34,10 +34,10 @@
namespace {
// This is how fast, in m/s, we allow position to shift during rate-limiting.
-constexpr auto kMaxTranslationalVelocity = 2 ;
+constexpr auto kMaxTranslationalVelocity = 2;
// This is how fast, in rad/s, we allow rotation angle to shift during rate-limiting.
-constexpr auto kMaxRotationalVelocity = 4 * M_PI ;
+constexpr auto kMaxRotationalVelocity = 4 * M_PI;
// This should be set to the typical time scale that the translation sensors used drift in. This
// means, loosely, for how long we can trust the reading to be "accurate enough". This would
@@ -73,8 +73,8 @@
} // namespace
SpatializerPoseController::SpatializerPoseController(Listener* listener,
- std::chrono::microseconds sensorPeriod,
- std::chrono::microseconds maxUpdatePeriod)
+ std::chrono::microseconds sensorPeriod,
+ std::chrono::microseconds maxUpdatePeriod)
: mListener(listener),
mSensorPeriod(sensorPeriod),
mPoseProvider(SensorPoseProvider::create("headtracker", this)),
@@ -88,6 +88,8 @@
})),
mThread([this, maxUpdatePeriod] {
while (true) {
+ Pose3f headToStage;
+ std::optional<HeadTrackingMode> modeIfChanged;
{
std::unique_lock lock(mMutex);
mCondVar.wait_for(lock, maxUpdatePeriod,
@@ -96,7 +98,19 @@
ALOGV("Exiting thread");
return;
}
- calculate_l();
+
+ // Calculate.
+ std::tie(headToStage, modeIfChanged) = calculate_l();
+ }
+
+ // Invoke the callbacks outside the lock.
+ mListener->onHeadToStagePose(headToStage);
+ if (modeIfChanged) {
+ mListener->onActualModeChange(modeIfChanged.value());
+ }
+
+ {
+ std::lock_guard lock(mMutex);
if (!mCalculated) {
mCalculated = true;
mCondVar.notify_all();
@@ -185,17 +199,20 @@
mCondVar.wait(lock, [this] { return mCalculated; });
}
-void SpatializerPoseController::calculate_l() {
+std::tuple<media::Pose3f, std::optional<media::HeadTrackingMode>>
+SpatializerPoseController::calculate_l() {
Pose3f headToStage;
HeadTrackingMode mode;
+ std::optional<media::HeadTrackingMode> modeIfChanged;
+
mProcessor->calculate(elapsedRealtimeNano());
headToStage = mProcessor->getHeadToStagePose();
mode = mProcessor->getActualMode();
- mListener->onHeadToStagePose(headToStage);
if (!mActualMode.has_value() || mActualMode.value() != mode) {
mActualMode = mode;
- mListener->onActualModeChange(mode);
+ modeIfChanged = mode;
}
+ return std::make_tuple(headToStage, modeIfChanged);
}
void SpatializerPoseController::recenter() {
@@ -204,7 +221,7 @@
}
void SpatializerPoseController::onPose(int64_t timestamp, int32_t sensor, const Pose3f& pose,
- const std::optional<Twist3f>& twist) {
+ const std::optional<Twist3f>& twist) {
std::lock_guard lock(mMutex);
if (sensor == mHeadSensor) {
mProcessor->setWorldToHeadPose(timestamp, pose, twist.value_or(Twist3f()));
diff --git a/services/audiopolicy/service/SpatializerPoseController.h b/services/audiopolicy/service/SpatializerPoseController.h
index 619dc7b..fcb7a46 100644
--- a/services/audiopolicy/service/SpatializerPoseController.h
+++ b/services/audiopolicy/service/SpatializerPoseController.h
@@ -38,15 +38,13 @@
* - By setting a timeout in the ctor, a calculation will be triggered after the timeout elapsed
* from the last calculateAsync() call.
*
- * This class is thread-safe. Callbacks are invoked with the lock held, so it is illegal to call
- * into this module from the callbacks.
+ * This class is thread-safe.
*/
class SpatializerPoseController : private media::SensorPoseProvider::Listener {
public:
/**
* Listener interface for getting pose and mode updates.
- * Methods will always be invoked from a designated thread. Calling into the parent class from
- * within the callbacks is disallowed (will result in a deadlock).
+ * Methods will always be invoked from a designated thread.
*/
class Listener {
public:
@@ -109,7 +107,7 @@
/**
* Blocks until calculation and invocation of the respective callbacks has happened at least
- * once.
+ * once. Do not call from within callbacks.
*/
void waitUntilCalculated();
@@ -131,7 +129,11 @@
void onPose(int64_t timestamp, int32_t sensor, const media::Pose3f& pose,
const std::optional<media::Twist3f>& twist) override;
- void calculate_l();
+ /**
+ * Calculates the new outputs and updates internal state. Must be called with the lock held.
+ * Returns values that should be passed to the respective callbacks.
+ */
+ std::tuple<media::Pose3f, std::optional<media::HeadTrackingMode>> calculate_l();
};
} // namespace android