Merge changes from topic "upstream-close-fixes"

* changes:
  audio: Add check to IDevice.close for currently opened streams
  audio: Factor out IStream operations into a helper class
  audio: Cleanup VTS tests
  Audio VTS: Fix MMAP tests
  Audio V6 wrapper: IDevice|IStream|IEffect.close releases HAL resource
This commit is contained in:
Treehugger Robot
2019-11-22 18:10:58 +00:00
committed by Gerrit Code Review
18 changed files with 258 additions and 146 deletions

View File

@@ -280,4 +280,19 @@ interface IDevice {
*/
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.
*
* Note that all streams must be closed by the client before
* attempting to close the device they belong to.
*
* @return retval OK in case the success.
* INVALID_STATE if the device was already closed
* or there are streams currently opened.
*/
@exit
close() generates (Result retval);
};

View File

@@ -277,7 +277,7 @@ interface IStream {
* @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 @@ interface IStream {
/**
* 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);
};

View File

@@ -39,11 +39,10 @@ namespace implementation {
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;
}
@@ -54,10 +53,14 @@ Result Device::analyzeStatus(const char* funcName, int status,
void Device::closeInputStream(audio_stream_in_t* stream) {
mDevice->close_input_stream(mDevice, stream);
LOG_ALWAYS_FATAL_IF(mOpenedStreamsCount == 0, "mOpenedStreamsCount is already 0");
--mOpenedStreamsCount;
}
void Device::closeOutputStream(audio_stream_out_t* stream) {
mDevice->close_output_stream(mDevice, stream);
LOG_ALWAYS_FATAL_IF(mOpenedStreamsCount == 0, "mOpenedStreamsCount is already 0");
--mOpenedStreamsCount;
}
char* Device::halGetParameters(const char* keys) {
@@ -159,6 +162,7 @@ std::tuple<Result, sp<IStreamOut>> Device::openOutputStreamImpl(int32_t ioHandle
sp<IStreamOut> streamOut;
if (status == OK) {
streamOut = new StreamOut(this, halStream);
++mOpenedStreamsCount;
}
HidlUtils::audioConfigFromHal(halConfig, suggestedConfig);
return {analyzeStatus("open_output_stream", status, {EINVAL} /*ignore*/), streamOut};
@@ -185,6 +189,7 @@ std::tuple<Result, sp<IStreamIn>> Device::openInputStreamImpl(
sp<IStreamIn> streamIn;
if (status == OK) {
streamIn = new StreamIn(this, halStream);
++mOpenedStreamsCount;
}
HidlUtils::audioConfigFromHal(halConfig, suggestedConfig);
return {analyzeStatus("open_input_stream", status, {EINVAL} /*ignore*/), streamIn};
@@ -383,6 +388,18 @@ Return<Result> Device::setConnectedState(const DeviceAddress& address, bool conn
}
#endif
Result Device::doClose() {
if (mIsClosed || mOpenedStreamsCount != 0) 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

View File

@@ -31,7 +31,11 @@ namespace implementation {
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<Result> PrimaryDevice::setConnectedState(const DeviceAddress& address, bo
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) {

View File

@@ -139,8 +139,7 @@ bool ReadThread::threadLoop() {
} // 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 @@ StreamIn::~StreamIn() {
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<void> StreamIn::getMmapPosition(getMmapPosition_cb _hidl_cb) {
}
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;
}

View File

@@ -138,8 +138,7 @@ bool WriteThread::threadLoop() {
} // 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(const sp<Device>& device, audio_stream_out_t* stream)
StreamOut::~StreamOut() {
ATRACE_CALL();
close();
(void)close();
if (mWriteThread.get()) {
ATRACE_NAME("mWriteThread->join");
status_t status = mWriteThread->join();
@@ -159,10 +158,12 @@ StreamOut::~StreamOut() {
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 @@ Return<Result> StreamOut::setParameters(const hidl_vec<ParameterValue>& context,
#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;
}

View File

@@ -114,6 +114,9 @@ struct Device : public IDevice, public ParametersUtil {
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,15 @@ struct Device : public IDevice, public ParametersUtil {
void closeOutputStream(audio_stream_out_t* stream);
audio_hw_device_t* device() const { return mDevice; }
private:
private:
bool mIsClosed;
audio_hw_device_t* mDevice;
int mOpenedStreamsCount = 0;
virtual ~Device();
Result doClose();
// Methods from ParametersUtil.
char* halGetParameters(const char* keys) override;
int halSetParameters(const char* keysAndValues) override;

View File

@@ -96,6 +96,9 @@ struct PrimaryDevice : public IPrimaryDevice {
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;

View File

@@ -120,7 +120,6 @@ struct StreamIn : public IStreamIn {
uint64_t* time);
private:
bool mIsClosed;
const sp<Device> mDevice;
audio_stream_in_t* mStream;
const sp<Stream> mStreamCommon;

View File

@@ -126,7 +126,6 @@ struct StreamOut : public IStreamOut {
TimeSpec* timeStamp);
private:
bool mIsClosed;
const sp<Device> mDevice;
audio_stream_out_t* mStream;
const sp<Stream> mStreamCommon;

View File

@@ -22,18 +22,16 @@ TEST_P(AudioHidlTest, OpenPrimaryDeviceUsingGetDevice) {
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));
}
//////////////////////////////////////////////////////////////////////////////
@@ -54,7 +52,6 @@ TEST_P(AudioHidlDeviceTest, GetMicrophonesTest) {
doc::test(
"Make sure getMicrophones always succeeds"
"and getActiveMicrophones always succeeds when recording from these microphones.");
AudioIoHandle ioHandle = (AudioIoHandle)AudioHandleConsts::AUDIO_IO_HANDLE_NONE;
AudioConfig config{};
config.channelMask = mkEnumBitfield(AudioChannelMask::IN_MONO);
config.sampleRateHz = 8000;
@@ -67,18 +64,14 @@ TEST_P(AudioHidlDeviceTest, GetMicrophonesTest) {
continue;
}
sp<IStreamIn> stream;
StreamHelper<IStreamIn> helper(stream);
AudioConfig suggestedConfig{};
ASSERT_OK(getDevice()->openInputStream(ioHandle, microphone.deviceAddress, config,
flags, initMetadata,
returnIn(res, stream, suggestedConfig)));
if (res != Result::OK) {
ASSERT_TRUE(stream == nullptr);
AudioConfig suggestedConfigRetry{};
ASSERT_OK(getDevice()->openInputStream(
ioHandle, microphone.deviceAddress, suggestedConfig, flags, initMetadata,
returnIn(res, stream, suggestedConfigRetry)));
}
ASSERT_OK(res);
ASSERT_NO_FATAL_FAILURE(helper.open(
[&](AudioIoHandle handle, AudioConfig config, auto cb) {
return getDevice()->openInputStream(handle, microphone.deviceAddress,
config, flags, initMetadata, cb);
},
config, &res, &suggestedConfig));
hidl_vec<MicrophoneInfo> activeMicrophones;
Result readRes;
typedef MessageQueue<IStreamIn::ReadParameters, kSynchronizedReadWrite> CommandMQ;
@@ -112,11 +105,8 @@ TEST_P(AudioHidlDeviceTest, GetMicrophonesTest) {
ASSERT_OK(res);
ASSERT_NE(0U, activeMicrophones.size());
}
stream->close();
// Workaround for b/139329877. Ensures the stream gets closed on the audio hal side.
stream.clear();
IPCThreadState::self()->flushCommands();
usleep(1000);
helper.close(true /*clear*/, &res);
ASSERT_OK(res);
if (efGroup) {
EventFlag::deleteEventFlag(&efGroup);
}

View File

@@ -100,7 +100,8 @@ const std::vector<DeviceConfigParameter>& getOutputDeviceConfigParameters() {
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;
@@ -144,3 +145,49 @@ const std::vector<DeviceConfigParameter>& getInputDeviceConfigParameters() {
}();
return parameters;
}
TEST_P(AudioHidlDeviceTest, CloseDeviceWithOpenedOutputStreams) {
doc::test("Verify that a device can't be closed if there are streams opened");
DeviceAddress address{.device = AudioDevice::OUT_DEFAULT};
AudioConfig config{};
auto flags = hidl_bitfield<AudioOutputFlag>(AudioOutputFlag::NONE);
SourceMetadata initMetadata = {{{AudioUsage::MEDIA, AudioContentType::MUSIC, 1 /* gain */}}};
sp<IStreamOut> stream;
StreamHelper<IStreamOut> helper(stream);
AudioConfig suggestedConfig{};
ASSERT_NO_FATAL_FAILURE(helper.open(
[&](AudioIoHandle handle, AudioConfig config, auto cb) {
return getDevice()->openOutputStream(handle, address, config, flags, initMetadata,
cb);
},
config, &res, &suggestedConfig));
ASSERT_RESULT(Result::INVALID_STATE, getDevice()->close());
ASSERT_NO_FATAL_FAILURE(helper.close(true /*clear*/, &res));
ASSERT_OK(getDevice()->close());
ASSERT_TRUE(resetDevice());
}
TEST_P(AudioHidlDeviceTest, CloseDeviceWithOpenedInputStreams) {
doc::test("Verify that a device can't be closed if there are streams opened");
auto module = getCachedPolicyConfig().getModuleFromName(getDeviceName());
if (module->getInputProfiles().empty()) {
GTEST_SKIP() << "Device doesn't have input profiles";
}
DeviceAddress address{.device = AudioDevice::IN_DEFAULT};
AudioConfig config{};
auto flags = hidl_bitfield<AudioInputFlag>(AudioInputFlag::NONE);
SinkMetadata initMetadata = {{{.source = AudioSource::MIC, .gain = 1}}};
sp<IStreamIn> stream;
StreamHelper<IStreamIn> helper(stream);
AudioConfig suggestedConfig{};
ASSERT_NO_FATAL_FAILURE(helper.open(
[&](AudioIoHandle handle, AudioConfig config, auto cb) {
return getDevice()->openInputStream(handle, address, config, flags, initMetadata,
cb);
},
config, &res, &suggestedConfig));
ASSERT_RESULT(Result::INVALID_STATE, getDevice()->close());
ASSERT_NO_FATAL_FAILURE(helper.close(true /*clear*/, &res));
ASSERT_OK(getDevice()->close());
ASSERT_TRUE(resetDevice());
}

View File

@@ -717,12 +717,6 @@ INSTANTIATE_TEST_CASE_P(
::testing::ValuesIn(ConfigHelper::getRequiredSupportCaptureAudioConfig()),
::testing::Values(AudioInputFlag::NONE)),
&DeviceConfigParameterToString);
INSTANTIATE_TEST_CASE_P(
SupportedInputBufferSize, RequiredInputBufferSizeTest,
::testing::Combine(::testing::ValuesIn(getDeviceParameters()),
::testing::ValuesIn(ConfigHelper::getSupportedCaptureAudioConfig()),
::testing::Values(AudioInputFlag::NONE)),
&DeviceConfigParameterToString);
INSTANTIATE_TEST_CASE_P(
RecommendedCaptureAudioConfigSupport, OptionalInputBufferSizeTest,
::testing::Combine(
@@ -815,60 +809,84 @@ TEST_P(AudioHidlDeviceTest, DebugDumpInvalidArguments) {
////////////////////////// open{Output,Input}Stream //////////////////////////
//////////////////////////////////////////////////////////////////////////////
// This class is also used by some device tests.
template <class Stream>
class OpenStreamTest : public AudioHidlTestWithDeviceConfigParameter {
protected:
class StreamHelper {
public:
// StreamHelper doesn't own the stream, this is for simpler stream lifetime management.
explicit StreamHelper(sp<Stream>& stream) : mStream(stream) {}
template <class Open>
void testOpen(Open openStream, const AudioConfig& config) {
void open(Open openStream, const AudioConfig& config, Result* res,
AudioConfig* suggestedConfigPtr) {
// FIXME: Open a stream without an IOHandle
// This is not required to be accepted by hal implementations
AudioIoHandle ioHandle = (AudioIoHandle)AudioHandleConsts::AUDIO_IO_HANDLE_NONE;
AudioConfig suggestedConfig{};
ASSERT_OK(openStream(ioHandle, config, returnIn(res, stream, suggestedConfig)));
// TODO: only allow failure for RecommendedPlaybackAudioConfig
switch (res) {
bool retryWithSuggestedConfig = true;
if (suggestedConfigPtr == nullptr) {
suggestedConfigPtr = &suggestedConfig;
retryWithSuggestedConfig = false;
}
ASSERT_OK(openStream(ioHandle, config, returnIn(*res, mStream, *suggestedConfigPtr)));
switch (*res) {
case Result::OK:
ASSERT_TRUE(stream != nullptr);
audioConfig = config;
ASSERT_TRUE(mStream != nullptr);
*suggestedConfigPtr = config;
break;
case Result::INVALID_ARGUMENTS:
ASSERT_TRUE(stream == nullptr);
AudioConfig suggestedConfigRetry;
// Could not open stream with config, try again with the
// suggested one
ASSERT_OK(openStream(ioHandle, suggestedConfig,
returnIn(res, stream, suggestedConfigRetry)));
// This time it must succeed
ASSERT_OK(res);
ASSERT_TRUE(stream != nullptr);
audioConfig = suggestedConfig;
ASSERT_TRUE(mStream == nullptr);
if (retryWithSuggestedConfig) {
AudioConfig suggestedConfigRetry;
ASSERT_OK(openStream(ioHandle, *suggestedConfigPtr,
returnIn(*res, mStream, suggestedConfigRetry)));
ASSERT_OK(*res);
ASSERT_TRUE(mStream != nullptr);
}
break;
default:
FAIL() << "Invalid return status: " << ::testing::PrintToString(res);
FAIL() << "Invalid return status: " << ::testing::PrintToString(*res);
}
}
void close(bool clear, Result* res) {
auto ret = mStream->close();
EXPECT_TRUE(ret.isOk());
*res = ret;
if (clear) {
mStream.clear();
#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
// the remote device has the time to be destroyed.
// flushCommand makes sure all local command are sent, thus should reduce
// the latency between local and remote destruction.
IPCThreadState::self()->flushCommands();
usleep(100 * 1000);
#endif
}
}
private:
sp<Stream>& mStream;
};
template <class Stream>
class OpenStreamTest : public AudioHidlTestWithDeviceConfigParameter {
protected:
OpenStreamTest() : AudioHidlTestWithDeviceConfigParameter(), helper(stream) {}
template <class Open>
void testOpen(Open openStream, const AudioConfig& config) {
// TODO: only allow failure for RecommendedPlaybackAudioConfig
ASSERT_NO_FATAL_FAILURE(helper.open(openStream, config, &res, &audioConfig));
open = true;
}
Return<Result> closeStream() {
Result closeStream(bool clear = true) {
open = false;
auto res = stream->close();
stream.clear();
waitForStreamDestruction();
helper.close(clear, &res);
return res;
}
static void waitForStreamDestruction() {
// 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
// the remote device has the time to be destroyed.
// flushCommand makes sure all local command are sent, thus should reduce
// the latency between local and remote destruction.
IPCThreadState::self()->flushCommands();
usleep(100 * 1000);
}
private:
void TearDown() override {
if (open) {
@@ -881,6 +899,7 @@ class OpenStreamTest : public AudioHidlTestWithDeviceConfigParameter {
AudioConfig audioConfig;
DeviceAddress address = {};
sp<Stream> stream;
StreamHelper<Stream> helper;
bool open = false;
};
@@ -928,12 +947,6 @@ INSTANTIATE_TEST_CASE_P(
::testing::ValuesIn(ConfigHelper::getRequiredSupportPlaybackAudioConfig()),
::testing::Values(AudioOutputFlag::NONE)),
&DeviceConfigParameterToString);
INSTANTIATE_TEST_CASE_P(
SupportedOutputStreamConfig, OutputStreamTest,
::testing::Combine(::testing::ValuesIn(getDeviceParameters()),
::testing::ValuesIn(ConfigHelper::getSupportedPlaybackAudioConfig()),
::testing::Values(AudioOutputFlag::NONE)),
&DeviceConfigParameterToString);
INSTANTIATE_TEST_CASE_P(
RecommendedOutputStreamConfigSupport, OutputStreamTest,
::testing::Combine(
@@ -944,7 +957,7 @@ INSTANTIATE_TEST_CASE_P(
#elif MAJOR_VERSION >= 6
// For V6 and above test according to the audio policy manager configuration.
// This is more correct as CDD is written from the apps perspective.
// Audio system provides necessary format conversions for the missing configurations.
// Audio system provides necessary format conversions for missing configurations.
INSTANTIATE_TEST_CASE_P(DeclaredOutputStreamConfigSupport, OutputStreamTest,
::testing::ValuesIn(getOutputDeviceConfigParameters()),
&DeviceConfigParameterToString);
@@ -989,12 +1002,6 @@ INSTANTIATE_TEST_CASE_P(
::testing::ValuesIn(ConfigHelper::getRequiredSupportCaptureAudioConfig()),
::testing::Values(AudioInputFlag::NONE)),
&DeviceConfigParameterToString);
INSTANTIATE_TEST_CASE_P(
SupportedInputStreamConfig, InputStreamTest,
::testing::Combine(::testing::ValuesIn(getDeviceParameters()),
::testing::ValuesIn(ConfigHelper::getSupportedCaptureAudioConfig()),
::testing::Values(AudioInputFlag::NONE)),
&DeviceConfigParameterToString);
INSTANTIATE_TEST_CASE_P(
RecommendedInputStreamConfigSupport, InputStreamTest,
::testing::Combine(
@@ -1005,7 +1012,7 @@ INSTANTIATE_TEST_CASE_P(
#elif MAJOR_VERSION >= 6
// For V6 and above test according to the audio policy manager configuration.
// This is more correct as CDD is written from the apps perspective.
// Audio system provides necessary format conversions for the missing configurations.
// Audio system provides necessary format conversions for missing configurations.
INSTANTIATE_TEST_CASE_P(DeclaredInputStreamConfigSupport, InputStreamTest,
::testing::ValuesIn(getInputDeviceConfigParameters()),
&DeviceConfigParameterToString);
@@ -1194,26 +1201,21 @@ TEST_IO_STREAM(getMmapPositionNoMmap, "Get a stream Mmap position before mapping
TEST_IO_STREAM(close, "Make sure a stream can be closed", ASSERT_OK(closeStream()))
// clang-format off
TEST_IO_STREAM(closeTwice, "Make sure a stream can not be closed twice",
auto streamCopy = stream;
ASSERT_OK(closeStream());
ASSERT_RESULT(Result::INVALID_STATE, streamCopy->close());
streamCopy.clear();
waitForStreamDestruction())
ASSERT_OK(closeStream(false /*clear*/));
ASSERT_EQ(Result::INVALID_STATE, closeStream()))
// 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;

View File

@@ -57,12 +57,6 @@ struct ConfigHelper {
{24000, 48000}, {AudioFormat::PCM_16_BIT});
}
static const vector<AudioConfig> getSupportedPlaybackAudioConfig() {
// TODO: retrieve audio config supported by the platform
// as declared in the policy configuration
return {};
}
static const vector<AudioConfig> getRequiredSupportCaptureAudioConfig() {
if (!primaryHasMic()) return {};
return combineAudioConfig({AudioChannelMask::IN_MONO}, {8000, 11025, 16000, 44100},
@@ -73,11 +67,6 @@ struct ConfigHelper {
return combineAudioConfig({AudioChannelMask::IN_STEREO}, {22050, 48000},
{AudioFormat::PCM_16_BIT});
}
static const vector<AudioConfig> getSupportedCaptureAudioConfig() {
// TODO: retrieve audio config supported by the platform
// as declared in the policy configuration
return {};
}
static vector<AudioConfig> combineAudioConfig(vector<audio_channel_mask_t> channelMasks,
vector<uint32_t> sampleRates,

View File

@@ -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 @@ class DeviceManager : public InterfaceManager<DeviceManager, FactoryAndDevice, I
}
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);

View File

@@ -407,9 +407,12 @@ interface IEffect {
/**
* 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.
*/

View File

@@ -138,11 +138,11 @@ const char* Effect::sContextCallToCommand = "error";
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 @@ Effect::~Effect() {
}
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::setCurrentConfigForFeature(uint32_t featureId,
}
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 */) {

View File

@@ -170,7 +170,6 @@ struct Effect : public IEffect {
static const char* sContextCallToCommand;
static const char* sContextCallFunction;
bool mIsClosed;
effect_handle_t mHandle;
sp<AudioBufferWrapper> mInBuffer;
sp<AudioBufferWrapper> mOutBuffer;