Merge changes I1b3a4173,I1200c9fd into sc-v2-dev
* changes:
Do not hold the lock while invoking callbacks
Ensure correct destruction of SensorPoseProviderImpl
diff --git a/media/libheadtracking/SensorPoseProvider.cpp b/media/libheadtracking/SensorPoseProvider.cpp
index 8e6c2ff..da1d8cb 100644
--- a/media/libheadtracking/SensorPoseProvider.cpp
+++ b/media/libheadtracking/SensorPoseProvider.cpp
@@ -101,6 +101,8 @@
}
~SensorPoseProviderImpl() override {
+ // Disable all active sensors.
+ mEnabledSensors.clear();
ALooper_wake(mLooper);
mThread.join();
}
@@ -109,12 +111,12 @@
int32_t handle = ASensor_getHandle(sensor);
// Enable the sensor.
- if (ASensorEventQueue_registerSensor(mQueue->get(), sensor, samplingPeriod.count(), 0)) {
+ if (ASensorEventQueue_registerSensor(mQueue, sensor, samplingPeriod.count(), 0)) {
ALOGE("Failed to enable sensor");
return INVALID_HANDLE;
}
- mEnabledSensors.emplace(handle, SensorEnableGuard(mQueue->get(), sensor));
+ mEnabledSensors.emplace(handle, SensorEnableGuard(mQueue, sensor));
return handle;
}
@@ -123,9 +125,10 @@
private:
ALooper* mLooper;
Listener* const mListener;
+
std::thread mThread;
std::map<int32_t, SensorEnableGuard> mEnabledSensors;
- std::unique_ptr<EventQueueGuard> mQueue;
+ ASensorEventQueue* mQueue;
// We must do some of the initialization operations on the worker thread, because the API relies
// on the thread-local looper. In addition, as a matter of convenience, we store some of the
@@ -159,16 +162,15 @@
}
// Create event queue.
- ASensorEventQueue* queue =
- ASensorManager_createEventQueue(sensor_manager, mLooper, kIdent, nullptr, nullptr);
+ mQueue = ASensorManager_createEventQueue(sensor_manager, mLooper, kIdent, nullptr, nullptr);
- if (queue == nullptr) {
+ if (mQueue == nullptr) {
ALOGE("Failed to create a sensor event queue");
initFinished(false);
return;
}
- mQueue.reset(new EventQueueGuard(sensor_manager, queue));
+ EventQueueGuard eventQueueGuard(sensor_manager, mQueue);
initFinished(true);
@@ -190,7 +192,7 @@
// Process an event.
ASensorEvent event;
- ssize_t size = ASensorEventQueue_getEvents(queue, &event, 1);
+ ssize_t size = ASensorEventQueue_getEvents(mQueue, &event, 1);
if (size < 0 || size > 1) {
ALOGE("Unexpected return value from ASensorEventQueue_getEvents: %zd", size);
break;
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