Merge "Make HAL default service run on crosshatch_car (VINTF registration)"
diff --git a/audio/6.0/IDevice.hal b/audio/6.0/IDevice.hal
index e885fe2..520e776 100644
--- a/audio/6.0/IDevice.hal
+++ b/audio/6.0/IDevice.hal
@@ -280,4 +280,15 @@
*/
setConnectedState(DeviceAddress address, bool connected)
generates (Result retval);
+
+ /**
+ * Called by the framework to deinitialize the device and free up
+ * all currently allocated resources. It is recommended to close
+ * the device on the client side as soon as it is becomes unused.
+ *
+ * @return retval OK in case the success.
+ * INVALID_STATE if the device was already closed.
+ */
+ @exit
+ close() generates (Result retval);
};
diff --git a/audio/6.0/IStream.hal b/audio/6.0/IStream.hal
index 451e116..d7d3c84 100644
--- a/audio/6.0/IStream.hal
+++ b/audio/6.0/IStream.hal
@@ -277,7 +277,7 @@
* @return retval OK in case the success.
* NOT_SUPPORTED on non mmap mode streams
* NOT_INITIALIZED in case of memory allocation error
- * INVALID_ARGUMENTS if the requested buffer size is too large
+ * INVALID_ARGUMENTS if the requested buffer size is invalid
* INVALID_STATE if called out of sequence
* @return info a MmapBufferInfo struct containing information on the MMMAP buffer created.
*/
@@ -300,13 +300,17 @@
/**
* Called by the framework to deinitialize the stream and free up
- * all the currently allocated resources. It is recommended to close
+ * all currently allocated resources. It is recommended to close
* the stream on the client side as soon as it is becomes unused.
*
+ * The client must ensure that this function is not called while
+ * audio data is being transferred through the stream's message queues.
+ *
* @return retval OK in case the success.
* NOT_SUPPORTED if called on IStream instead of input or
* output stream interface.
* INVALID_STATE if the stream was already closed.
*/
+ @exit
close() generates (Result retval);
};
diff --git a/audio/core/all-versions/default/Device.cpp b/audio/core/all-versions/default/Device.cpp
index 1a9df21..5ea4c8d 100644
--- a/audio/core/all-versions/default/Device.cpp
+++ b/audio/core/all-versions/default/Device.cpp
@@ -39,11 +39,10 @@
using ::android::hardware::audio::common::CPP_VERSION::implementation::HidlUtils;
-Device::Device(audio_hw_device_t* device) : mDevice(device) {}
+Device::Device(audio_hw_device_t* device) : mIsClosed(false), mDevice(device) {}
Device::~Device() {
- int status = audio_hw_device_close(mDevice);
- ALOGW_IF(status, "Error closing audio hw device %p: %s", mDevice, strerror(-status));
+ (void)doClose();
mDevice = nullptr;
}
@@ -383,6 +382,18 @@
}
#endif
+Result Device::doClose() {
+ if (mIsClosed) return Result::INVALID_STATE;
+ mIsClosed = true;
+ return analyzeStatus("close", audio_hw_device_close(mDevice));
+}
+
+#if MAJOR_VERSION >= 6
+Return<Result> Device::close() {
+ return doClose();
+}
+#endif
+
} // namespace implementation
} // namespace CPP_VERSION
} // namespace audio
diff --git a/audio/core/all-versions/default/PrimaryDevice.cpp b/audio/core/all-versions/default/PrimaryDevice.cpp
index 99590b0..3cf0932 100644
--- a/audio/core/all-versions/default/PrimaryDevice.cpp
+++ b/audio/core/all-versions/default/PrimaryDevice.cpp
@@ -31,7 +31,11 @@
PrimaryDevice::PrimaryDevice(audio_hw_device_t* device) : mDevice(new Device(device)) {}
-PrimaryDevice::~PrimaryDevice() {}
+PrimaryDevice::~PrimaryDevice() {
+ // Do not call mDevice->close here. If there are any unclosed streams,
+ // they only hold IDevice instance, not IPrimaryDevice, thus IPrimaryDevice
+ // "part" of a device can be destroyed before the streams.
+}
// Methods from ::android::hardware::audio::CPP_VERSION::IDevice follow.
Return<Result> PrimaryDevice::initCheck() {
@@ -160,6 +164,11 @@
return mDevice->setConnectedState(address, connected);
}
#endif
+#if MAJOR_VERSION >= 6
+Return<Result> PrimaryDevice::close() {
+ return mDevice->close();
+}
+#endif
// Methods from ::android::hardware::audio::CPP_VERSION::IPrimaryDevice follow.
Return<Result> PrimaryDevice::setVoiceVolume(float volume) {
diff --git a/audio/core/all-versions/default/StreamIn.cpp b/audio/core/all-versions/default/StreamIn.cpp
index d316f83..f1152ca 100644
--- a/audio/core/all-versions/default/StreamIn.cpp
+++ b/audio/core/all-versions/default/StreamIn.cpp
@@ -139,8 +139,7 @@
} // namespace
StreamIn::StreamIn(const sp<Device>& device, audio_stream_in_t* stream)
- : mIsClosed(false),
- mDevice(device),
+ : mDevice(device),
mStream(stream),
mStreamCommon(new Stream(&stream->common)),
mStreamMmap(new StreamMmap<audio_stream_in_t>(stream)),
@@ -159,7 +158,9 @@
status_t status = EventFlag::deleteEventFlag(&mEfGroup);
ALOGE_IF(status, "read MQ event flag deletion error: %s", strerror(-status));
}
+#if MAJOR_VERSION <= 5
mDevice->closeInputStream(mStream);
+#endif
mStream = nullptr;
}
@@ -303,14 +304,16 @@
}
Return<Result> StreamIn::close() {
- if (mIsClosed) return Result::INVALID_STATE;
- mIsClosed = true;
- if (mReadThread.get()) {
- mStopReadThread.store(true, std::memory_order_release);
+ if (mStopReadThread.load(std::memory_order_relaxed)) { // only this thread writes
+ return Result::INVALID_STATE;
}
+ mStopReadThread.store(true, std::memory_order_release);
if (mEfGroup) {
mEfGroup->wake(static_cast<uint32_t>(MessageQueueFlagBits::NOT_FULL));
}
+#if MAJOR_VERSION >= 6
+ mDevice->closeInputStream(mStream);
+#endif
return Result::OK;
}
diff --git a/audio/core/all-versions/default/StreamOut.cpp b/audio/core/all-versions/default/StreamOut.cpp
index 82cc408..396d354 100644
--- a/audio/core/all-versions/default/StreamOut.cpp
+++ b/audio/core/all-versions/default/StreamOut.cpp
@@ -138,8 +138,7 @@
} // namespace
StreamOut::StreamOut(const sp<Device>& device, audio_stream_out_t* stream)
- : mIsClosed(false),
- mDevice(device),
+ : mDevice(device),
mStream(stream),
mStreamCommon(new Stream(&stream->common)),
mStreamMmap(new StreamMmap<audio_stream_out_t>(stream)),
@@ -148,7 +147,7 @@
StreamOut::~StreamOut() {
ATRACE_CALL();
- close();
+ (void)close();
if (mWriteThread.get()) {
ATRACE_NAME("mWriteThread->join");
status_t status = mWriteThread->join();
@@ -159,10 +158,12 @@
ALOGE_IF(status, "write MQ event flag deletion error: %s", strerror(-status));
}
mCallback.clear();
+#if MAJOR_VERSION <= 5
mDevice->closeOutputStream(mStream);
// Closing the output stream in the HAL waits for the callback to finish,
// and joins the callback thread. Thus is it guaranteed that the callback
// thread will not be accessing our object anymore.
+#endif
mStream = nullptr;
}
@@ -291,14 +292,16 @@
#endif
Return<Result> StreamOut::close() {
- if (mIsClosed) return Result::INVALID_STATE;
- mIsClosed = true;
- if (mWriteThread.get()) {
- mStopWriteThread.store(true, std::memory_order_release);
+ if (mStopWriteThread.load(std::memory_order_relaxed)) { // only this thread writes
+ return Result::INVALID_STATE;
}
+ mStopWriteThread.store(true, std::memory_order_release);
if (mEfGroup) {
mEfGroup->wake(static_cast<uint32_t>(MessageQueueFlagBits::NOT_EMPTY));
}
+#if MAJOR_VERSION >= 6
+ mDevice->closeOutputStream(mStream);
+#endif
return Result::OK;
}
diff --git a/audio/core/all-versions/default/include/core/default/Device.h b/audio/core/all-versions/default/include/core/default/Device.h
index e64f00f..dc53a39 100644
--- a/audio/core/all-versions/default/include/core/default/Device.h
+++ b/audio/core/all-versions/default/include/core/default/Device.h
@@ -114,6 +114,9 @@
Return<void> getMicrophones(getMicrophones_cb _hidl_cb) override;
Return<Result> setConnectedState(const DeviceAddress& address, bool connected) override;
#endif
+#if MAJOR_VERSION >= 6
+ Return<Result> close() override;
+#endif
Return<void> debug(const hidl_handle& fd, const hidl_vec<hidl_string>& options) override;
@@ -124,11 +127,14 @@
void closeOutputStream(audio_stream_out_t* stream);
audio_hw_device_t* device() const { return mDevice; }
- private:
+ private:
+ bool mIsClosed;
audio_hw_device_t* mDevice;
virtual ~Device();
+ Result doClose();
+
// Methods from ParametersUtil.
char* halGetParameters(const char* keys) override;
int halSetParameters(const char* keysAndValues) override;
diff --git a/audio/core/all-versions/default/include/core/default/PrimaryDevice.h b/audio/core/all-versions/default/include/core/default/PrimaryDevice.h
index 9d69cb0..f5f3848 100644
--- a/audio/core/all-versions/default/include/core/default/PrimaryDevice.h
+++ b/audio/core/all-versions/default/include/core/default/PrimaryDevice.h
@@ -96,6 +96,9 @@
Return<void> getMicrophones(getMicrophones_cb _hidl_cb) override;
Return<Result> setConnectedState(const DeviceAddress& address, bool connected) override;
#endif
+#if MAJOR_VERSION >= 6
+ Return<Result> close() override;
+#endif
Return<void> debug(const hidl_handle& fd, const hidl_vec<hidl_string>& options) override;
diff --git a/audio/core/all-versions/default/include/core/default/StreamIn.h b/audio/core/all-versions/default/include/core/default/StreamIn.h
index 6209b8f..24f9944 100644
--- a/audio/core/all-versions/default/include/core/default/StreamIn.h
+++ b/audio/core/all-versions/default/include/core/default/StreamIn.h
@@ -120,7 +120,6 @@
uint64_t* time);
private:
- bool mIsClosed;
const sp<Device> mDevice;
audio_stream_in_t* mStream;
const sp<Stream> mStreamCommon;
diff --git a/audio/core/all-versions/default/include/core/default/StreamOut.h b/audio/core/all-versions/default/include/core/default/StreamOut.h
index b098005..6334785 100644
--- a/audio/core/all-versions/default/include/core/default/StreamOut.h
+++ b/audio/core/all-versions/default/include/core/default/StreamOut.h
@@ -126,7 +126,6 @@
TimeSpec* timeStamp);
private:
- bool mIsClosed;
const sp<Device> mDevice;
audio_stream_out_t* mStream;
const sp<Stream> mStreamCommon;
diff --git a/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalTest.cpp b/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalTest.cpp
index e267a5e..81f963d 100644
--- a/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalTest.cpp
+++ b/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalTest.cpp
@@ -22,18 +22,16 @@
GTEST_SKIP() << "No primary device on this factory"; // returns
}
- struct WaitExecutor {
- ~WaitExecutor() { DeviceManager::waitForInstanceDestruction(); }
- } waitExecutor; // Make sure we wait for the device destruction on exiting from the test.
- Result result;
- sp<IDevice> baseDevice;
- ASSERT_OK(getDevicesFactory()->openDevice("primary", returnIn(result, baseDevice)));
- ASSERT_OK(result);
- ASSERT_TRUE(baseDevice != nullptr);
-
- Return<sp<IPrimaryDevice>> primaryDevice = IPrimaryDevice::castFrom(baseDevice);
- ASSERT_TRUE(primaryDevice.isOk());
- ASSERT_TRUE(sp<IPrimaryDevice>(primaryDevice) != nullptr);
+ { // Scope for device SPs
+ sp<IDevice> baseDevice =
+ DeviceManager::getInstance().get(getFactoryName(), DeviceManager::kPrimaryDevice);
+ ASSERT_TRUE(baseDevice != nullptr);
+ Return<sp<IPrimaryDevice>> primaryDevice = IPrimaryDevice::castFrom(baseDevice);
+ EXPECT_TRUE(primaryDevice.isOk());
+ EXPECT_TRUE(sp<IPrimaryDevice>(primaryDevice) != nullptr);
+ }
+ EXPECT_TRUE(
+ DeviceManager::getInstance().reset(getFactoryName(), DeviceManager::kPrimaryDevice));
}
//////////////////////////////////////////////////////////////////////////////
@@ -113,10 +111,12 @@
ASSERT_NE(0U, activeMicrophones.size());
}
stream->close();
+#if MAJOR_VERSION <= 5
// Workaround for b/139329877. Ensures the stream gets closed on the audio hal side.
stream.clear();
IPCThreadState::self()->flushCommands();
usleep(1000);
+#endif
if (efGroup) {
EventFlag::deleteEventFlag(&efGroup);
}
diff --git a/audio/core/all-versions/vts/functional/6.0/AudioPrimaryHidlHalTest.cpp b/audio/core/all-versions/vts/functional/6.0/AudioPrimaryHidlHalTest.cpp
index 30f8a7a..7e931ff 100644
--- a/audio/core/all-versions/vts/functional/6.0/AudioPrimaryHidlHalTest.cpp
+++ b/audio/core/all-versions/vts/functional/6.0/AudioPrimaryHidlHalTest.cpp
@@ -100,7 +100,8 @@
special = true;
}
if ((flags & AUDIO_OUTPUT_FLAG_DIRECT) &&
- !(flags & AUDIO_OUTPUT_FLAG_HW_AV_SYNC)) {
+ !(flags &
+ (AUDIO_OUTPUT_FLAG_HW_AV_SYNC | AUDIO_OUTPUT_FLAG_MMAP_NOIRQ))) {
result.emplace_back(device, config,
AudioOutputFlag(AUDIO_OUTPUT_FLAG_DIRECT));
special = true;
diff --git a/audio/core/all-versions/vts/functional/AudioPrimaryHidlHalTest.h b/audio/core/all-versions/vts/functional/AudioPrimaryHidlHalTest.h
index 468f9b2..beea8e0 100644
--- a/audio/core/all-versions/vts/functional/AudioPrimaryHidlHalTest.h
+++ b/audio/core/all-versions/vts/functional/AudioPrimaryHidlHalTest.h
@@ -860,6 +860,7 @@
}
static void waitForStreamDestruction() {
+#if MAJOR_VERSION <= 5
// FIXME: there is no way to know when the remote IStream is being destroyed
// Binder does not support testing if an object is alive, thus
// wait for 100ms to let the binder destruction propagates and
@@ -868,6 +869,7 @@
// the latency between local and remote destruction.
IPCThreadState::self()->flushCommands();
usleep(100 * 1000);
+#endif
}
private:
@@ -1202,19 +1204,17 @@
waitForStreamDestruction())
// clang-format on
-static void testCreateTooBigMmapBuffer(IStream* stream) {
- MmapBufferInfo info;
- Result res;
- // Assume that int max is a value too big to be allocated
- // This is true currently with a 32bit media server, but might not when it
- // will run in 64 bit
- auto minSizeFrames = std::numeric_limits<int32_t>::max();
- ASSERT_OK(stream->createMmapBuffer(minSizeFrames, returnIn(res, info)));
- ASSERT_RESULT(invalidArgsOrNotSupported, res);
+static void testMmapBufferOfInvalidSize(IStream* stream) {
+ for (int32_t value : {-1, 0, std::numeric_limits<int32_t>::max()}) {
+ MmapBufferInfo info;
+ Result res;
+ EXPECT_OK(stream->createMmapBuffer(value, returnIn(res, info)));
+ EXPECT_RESULT(invalidArgsOrNotSupported, res) << "value=" << value;
+ }
}
-TEST_IO_STREAM(CreateTooBigMmapBuffer, "Create mmap buffer too big should fail",
- testCreateTooBigMmapBuffer(stream.get()))
+TEST_IO_STREAM(CreateTooBigMmapBuffer, "Create mmap buffer of invalid size must fail",
+ testMmapBufferOfInvalidSize(stream.get()))
static void testGetMmapPositionOfNonMmapedStream(IStream* stream) {
Result res;
diff --git a/audio/core/all-versions/vts/functional/DeviceManager.h b/audio/core/all-versions/vts/functional/DeviceManager.h
index b6e2db0..d849f85 100644
--- a/audio/core/all-versions/vts/functional/DeviceManager.h
+++ b/audio/core/all-versions/vts/functional/DeviceManager.h
@@ -22,25 +22,33 @@
template <class Derived, class Key, class Interface>
class InterfaceManager {
public:
+ sp<Interface> getExisting(const Key& name) {
+ auto existing = instances.find(name);
+ return existing != instances.end() ? existing->second : sp<Interface>();
+ }
+
sp<Interface> get(const Key& name) {
auto existing = instances.find(name);
if (existing != instances.end()) return existing->second;
auto [inserted, _] = instances.emplace(name, Derived::createInterfaceInstance(name));
if (inserted->second) {
- environment->registerTearDown([name]() { (void)Derived::getInstance().reset(name); });
+ environment->registerTearDown(
+ [name]() { (void)Derived::getInstance().reset(name, false); });
}
return inserted->second;
}
// The test must check that reset was successful. Reset failure means that the test code
// is holding a strong reference to the device.
- bool reset(const Key& name) __attribute__((warn_unused_result)) {
+ bool reset(const Key& name, bool waitForDestruction) __attribute__((warn_unused_result)) {
auto iter = instances.find(name);
if (iter == instances.end()) return true;
::android::wp<Interface> weak = iter->second;
instances.erase(iter);
if (weak.promote() != nullptr) return false;
- waitForInstanceDestruction();
+ if (waitForDestruction) {
+ waitForInstanceDestruction();
+ }
return true;
}
@@ -100,7 +108,15 @@
}
bool reset(const std::string& factoryName, const std::string& name)
__attribute__((warn_unused_result)) {
- return InterfaceManager::reset(std::make_tuple(factoryName, name));
+#if MAJOR_VERSION <= 5
+ return InterfaceManager::reset(std::make_tuple(factoryName, name), true);
+#elif MAJOR_VERSION >= 6
+ {
+ sp<IDevice> device = getExisting(std::make_tuple(factoryName, name));
+ if (device != nullptr) device->close();
+ }
+ return InterfaceManager::reset(std::make_tuple(factoryName, name), false);
+#endif
}
bool resetPrimary(const std::string& factoryName) __attribute__((warn_unused_result)) {
return reset(factoryName, kPrimaryDevice);
diff --git a/audio/effect/6.0/IEffect.hal b/audio/effect/6.0/IEffect.hal
index b35afee..f4c50a2 100644
--- a/audio/effect/6.0/IEffect.hal
+++ b/audio/effect/6.0/IEffect.hal
@@ -407,9 +407,12 @@
/**
* Called by the framework to deinitialize the effect and free up
- * all the currently allocated resources. It is recommended to close
+ * all currently allocated resources. It is recommended to close
* the effect on the client side as soon as it is becomes unused.
*
+ * The client must ensure that this function is not called while
+ * audio data is being transferred through the effect's message queues.
+ *
* @return retval OK in case the success.
* INVALID_STATE if the effect was already closed.
*/
diff --git a/audio/effect/all-versions/default/Effect.cpp b/audio/effect/all-versions/default/Effect.cpp
index 3c0d878..0afa779 100644
--- a/audio/effect/all-versions/default/Effect.cpp
+++ b/audio/effect/all-versions/default/Effect.cpp
@@ -138,11 +138,11 @@
const char* Effect::sContextCallFunction = sContextCallToCommand;
Effect::Effect(effect_handle_t handle)
- : mIsClosed(false), mHandle(handle), mEfGroup(nullptr), mStopProcessThread(false) {}
+ : mHandle(handle), mEfGroup(nullptr), mStopProcessThread(false) {}
Effect::~Effect() {
ATRACE_CALL();
- close();
+ (void)close();
if (mProcessThread.get()) {
ATRACE_NAME("mProcessThread->join");
status_t status = mProcessThread->join();
@@ -154,8 +154,10 @@
}
mInBuffer.clear();
mOutBuffer.clear();
+#if MAJOR_VERSION <= 5
int status = EffectRelease(mHandle);
ALOGW_IF(status, "Error releasing effect %p: %s", mHandle, strerror(-status));
+#endif
EffectMap::getInstance().remove(mHandle);
mHandle = 0;
}
@@ -699,15 +701,20 @@
}
Return<Result> Effect::close() {
- if (mIsClosed) return Result::INVALID_STATE;
- mIsClosed = true;
- if (mProcessThread.get()) {
- mStopProcessThread.store(true, std::memory_order_release);
+ if (mStopProcessThread.load(std::memory_order_relaxed)) { // only this thread modifies
+ return Result::INVALID_STATE;
}
+ mStopProcessThread.store(true, std::memory_order_release);
if (mEfGroup) {
mEfGroup->wake(static_cast<uint32_t>(MessageQueueFlagBits::REQUEST_QUIT));
}
+#if MAJOR_VERSION <= 5
return Result::OK;
+#elif MAJOR_VERSION >= 6
+ // No need to join the processing thread, it is part of the API contract that the client
+ // must finish processing before closing the effect.
+ return analyzeStatus("EffectRelease", "", sContextCallFunction, EffectRelease(mHandle));
+#endif
}
Return<void> Effect::debug(const hidl_handle& fd, const hidl_vec<hidl_string>& /* options */) {
diff --git a/audio/effect/all-versions/default/Effect.h b/audio/effect/all-versions/default/Effect.h
index 3d99a0e..181e542 100644
--- a/audio/effect/all-versions/default/Effect.h
+++ b/audio/effect/all-versions/default/Effect.h
@@ -170,7 +170,6 @@
static const char* sContextCallToCommand;
static const char* sContextCallFunction;
- bool mIsClosed;
effect_handle_t mHandle;
sp<AudioBufferWrapper> mInBuffer;
sp<AudioBufferWrapper> mOutBuffer;