From 87edc66e1f7423bb604128c6feb6d35379a63f15 Mon Sep 17 00:00:00 2001 From: Andy Hung Date: Wed, 16 Jun 2021 09:40:07 -0700 Subject: [PATCH 1/4] Audio: Add memory leak checking for HAL $ adb shell setprop libc.debug.malloc.program android.hardware.audio.service $ adb shell setprop libc.debug.malloc.options backtrace=8 $ adb shell setenforce 0 $ adb shell pkill audioserver $ adb shell dumpsys media.audio_flinger Test: Check the audio flinger dumpsys as above. Bug: 186054996 Bug: 187462632 Change-Id: I2e8db14b816cc4cd7e1420c538505bf71fa58c97 (cherry picked from commit 8f836b94992576a85b15d9666f4da193008e0597) Merged-In: I2e8db14b816cc4cd7e1420c538505bf71fa58c97 --- audio/core/all-versions/default/Android.bp | 2 ++ audio/core/all-versions/default/Device.cpp | 29 ++++++++++++++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/audio/core/all-versions/default/Android.bp b/audio/core/all-versions/default/Android.bp index 86380a5d40..df688fd909 100644 --- a/audio/core/all-versions/default/Android.bp +++ b/audio/core/all-versions/default/Android.bp @@ -48,6 +48,8 @@ cc_defaults { "libhidlbase", "liblog", "libmedia_helper", + "libmediautils_vendor", + "libmemunreachable", "libutils", "android.hardware.audio.common-util", ], diff --git a/audio/core/all-versions/default/Device.cpp b/audio/core/all-versions/default/Device.cpp index 98a59b9ff2..ac5a3ba8ef 100644 --- a/audio/core/all-versions/default/Device.cpp +++ b/audio/core/all-versions/default/Device.cpp @@ -30,6 +30,8 @@ #include #include +#include +#include #include @@ -501,9 +503,32 @@ Return Device::debugDump(const hidl_handle& fd) { } #endif -Return Device::debug(const hidl_handle& fd, const hidl_vec& /* options */) { +Return Device::debug(const hidl_handle& fd, const hidl_vec& options) { if (fd.getNativeHandle() != nullptr && fd->numFds == 1) { - analyzeStatus("dump", mDevice->dump(mDevice, fd->data[0])); + const int fd0 = fd->data[0]; + bool dumpMem = false; + bool unreachableMemory = false; + for (const auto& option : options) { + if (option == "-m") { + dumpMem = true; + } else if (option == "--unreachable") { + unreachableMemory = true; + } + } + + if (dumpMem) { + dprintf(fd0, "\nDumping memory:\n"); + std::string s = dumpMemoryAddresses(100 /* limit */); + write(fd0, s.c_str(), s.size()); + } + if (unreachableMemory) { + dprintf(fd0, "\nDumping unreachable memory:\n"); + // TODO - should limit be an argument parameter? + std::string s = GetUnreachableMemoryString(true /* contents */, 100 /* limit */); + write(fd0, s.c_str(), s.size()); + } + + analyzeStatus("dump", mDevice->dump(mDevice, fd0)); } return Void(); } From 22dc9b9aa4d39829d3858450272e5eb02f3d45e0 Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Sat, 29 Jan 2022 00:39:13 +0000 Subject: [PATCH 2/4] audio: Add recommended mute duration attribute to APM XSD Add 'recommendedMuteDurationMs' optional attribute to 'mixPort' element. This attribute specifies customized mute duration while switching device's audio path to avoid truncated sound at the beginning. The framework is already aware of this attribute, only need to allow it in the XSD. Bug: 183344508 Bug: 197823000 Test: m CTS-Coverage-bug: 215647214 Change-Id: I3be36009d1b5db218c846efe96f078e858283992 (cherry picked from commit b6800fc7154c51f462a80d7d202e6646e50ef666) Merged-In: I3be36009d1b5db218c846efe96f078e858283992 --- audio/7.1/config/api/current.txt | 2 ++ audio/7.1/config/audio_policy_configuration.xsd | 1 + 2 files changed, 3 insertions(+) diff --git a/audio/7.1/config/api/current.txt b/audio/7.1/config/api/current.txt index 0a0eb02e19..2963904fea 100644 --- a/audio/7.1/config/api/current.txt +++ b/audio/7.1/config/api/current.txt @@ -462,6 +462,7 @@ package android.audio.policy.configuration.V7_1 { method @Nullable public String getName(); method @Nullable public java.util.List getPreferredUsage(); method @Nullable public java.util.List getProfile(); + method @Nullable public long getRecommendedMuteDurationMs(); method @Nullable public android.audio.policy.configuration.V7_1.Role getRole(); method public void setFlags(@Nullable java.util.List); method public void setGains(@Nullable android.audio.policy.configuration.V7_1.Gains); @@ -469,6 +470,7 @@ package android.audio.policy.configuration.V7_1 { method public void setMaxOpenCount(@Nullable long); method public void setName(@Nullable String); method public void setPreferredUsage(@Nullable java.util.List); + method public void setRecommendedMuteDurationMs(@Nullable long); method public void setRole(@Nullable android.audio.policy.configuration.V7_1.Role); } diff --git a/audio/7.1/config/audio_policy_configuration.xsd b/audio/7.1/config/audio_policy_configuration.xsd index defb50651d..414552e89f 100644 --- a/audio/7.1/config/audio_policy_configuration.xsd +++ b/audio/7.1/config/audio_policy_configuration.xsd @@ -226,6 +226,7 @@ + From 930799246605041e50aec13d5c326f7f4d640bb6 Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Mon, 31 Jan 2022 22:40:16 +0000 Subject: [PATCH 3/4] audio: Add IDevice.setConnectedState_7_1 method This is an updated version of IDevice.setConnectedState which accepts a full AudioPort so that additional data like extra audio descriptors can be passed to the audio HAL. Bug: 211601178 Test: atest VtsHalAudioV7_1TargetTest Change-Id: Id746caa32122dabfb83feb5b515bed7717bcb67c (cherry picked from commit 533f78f411b4e9c5156d6b252c99415df9441cb1) Merged-In: Id746caa32122dabfb83feb5b515bed7717bcb67c --- audio/7.1/IDevice.hal | 12 +++++++ audio/core/all-versions/default/Device.cpp | 15 +++++++++ .../default/include/core/default/Device.h | 3 ++ .../7.1/AudioPrimaryHidlHalTest.cpp | 31 +++++++++++++++++++ 4 files changed, 61 insertions(+) diff --git a/audio/7.1/IDevice.hal b/audio/7.1/IDevice.hal index 373d17f4f9..e0b1e92359 100644 --- a/audio/7.1/IDevice.hal +++ b/audio/7.1/IDevice.hal @@ -85,4 +85,16 @@ interface IDevice extends @7.0::IDevice { Result retval, IStreamIn inStream, AudioConfig suggestedConfig); + + /** + * Notifies the device module about the connection state of an input/output + * device attached to it. The devicePort identifies the device and may also + * provide extra information such as raw audio descriptors. + * + * @param devicePort audio device port. + * @param connected whether the device is connected. + * @return retval operation completion status. + */ + setConnectedState_7_1(AudioPort devicePort, bool connected) + generates (Result retval); }; diff --git a/audio/core/all-versions/default/Device.cpp b/audio/core/all-versions/default/Device.cpp index ac5a3ba8ef..ca8c03df36 100644 --- a/audio/core/all-versions/default/Device.cpp +++ b/audio/core/all-versions/default/Device.cpp @@ -616,6 +616,21 @@ Return Device::updateAudioPatch(int32_t previousPatch, #endif +#if MAJOR_VERSION == 7 && MINOR_VERSION == 1 +Return Device::setConnectedState_7_1(const AudioPort& devicePort, bool connected) { + if (version() >= AUDIO_DEVICE_API_VERSION_3_2 && + mDevice->set_device_connected_state_v7 != nullptr) { + audio_port_v7 halPort; + if (status_t status = HidlUtils::audioPortToHal(devicePort, &halPort); status != NO_ERROR) { + return analyzeStatus("audioPortToHal", status); + } + return analyzeStatus("set_device_connected_state_v7", + mDevice->set_device_connected_state_v7(mDevice, &halPort, connected)); + } + return Result::NOT_SUPPORTED; +} +#endif + } // namespace implementation } // namespace CPP_VERSION } // namespace audio 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 0aeb6b32cb..8cde3e0a06 100644 --- a/audio/core/all-versions/default/include/core/default/Device.h +++ b/audio/core/all-versions/default/include/core/default/Device.h @@ -162,6 +162,9 @@ struct Device : public IDevice, public ParametersUtil { Return updateAudioPatch(int32_t previousPatch, const hidl_vec& sources, const hidl_vec& sinks, createAudioPatch_cb _hidl_cb) override; +#endif +#if MAJOR_VERSION == 7 && MINOR_VERSION == 1 + Return setConnectedState_7_1(const AudioPort& devicePort, bool connected) override; #endif Return debug(const hidl_handle& fd, const hidl_vec& options) override; diff --git a/audio/core/all-versions/vts/functional/7.1/AudioPrimaryHidlHalTest.cpp b/audio/core/all-versions/vts/functional/7.1/AudioPrimaryHidlHalTest.cpp index b750f56ca2..d82d4adb51 100644 --- a/audio/core/all-versions/vts/functional/7.1/AudioPrimaryHidlHalTest.cpp +++ b/audio/core/all-versions/vts/functional/7.1/AudioPrimaryHidlHalTest.cpp @@ -16,3 +16,34 @@ // pull in all the <= 7.0 tests #include "7.0/AudioPrimaryHidlHalTest.cpp" + +TEST_P(AudioHidlDeviceTest, SetConnectedState_7_1) { + doc::test("Check that the HAL can be notified of device connection and disconnection"); + using AD = xsd::AudioDevice; + for (auto deviceType : {AD::AUDIO_DEVICE_OUT_HDMI, AD::AUDIO_DEVICE_OUT_WIRED_HEADPHONE, + AD::AUDIO_DEVICE_IN_USB_HEADSET}) { + SCOPED_TRACE("device=" + toString(deviceType)); + for (bool state : {true, false}) { + SCOPED_TRACE("state=" + ::testing::PrintToString(state)); + DeviceAddress address = {}; + address.deviceType = toString(deviceType); + if (deviceType == AD::AUDIO_DEVICE_IN_USB_HEADSET) { + address.address.alsa({0, 0}); + } + AudioPort devicePort; + devicePort.ext.device(address); + auto ret = getDevice()->setConnectedState_7_1(devicePort, state); + ASSERT_TRUE(ret.isOk()); + if (ret == Result::NOT_SUPPORTED) { + doc::partialTest("setConnectedState_7_1 is not supported"); + break; // other deviceType might be supported + } + ASSERT_OK(ret); + } + } + + // Because there is no way of knowing if the devices were connected before + // calling setConnectedState, there is no way to restore the HAL to its + // initial state. To workaround this, destroy the HAL at the end of this test. + ASSERT_TRUE(resetDevice()); +} From 4a016add9598cc2bd47c5072cb6e6ebde884028f Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Fri, 4 Feb 2022 00:00:11 +0000 Subject: [PATCH 4/4] audio: Remove IStreamIn@7.1 This has turned out to be an empty interface. By removing it we save about 100 kB in the interface code. Bug: 217752112 Test: atest VtsHalAudioV7_1TargetTest Change-Id: I2522653600a254ddcf2c4eac8bed1df69ac11d3d (cherry picked from commit 893ea216812749d3e22a9d94a8f4d92288431458) Merged-In: I2522653600a254ddcf2c4eac8bed1df69ac11d3d --- audio/7.1/Android.bp | 1 - audio/7.1/IDevice.hal | 32 ------------------- audio/7.1/IStreamIn.hal | 22 ------------- audio/core/all-versions/default/Device.cpp | 10 ------ .../default/include/core/default/Device.h | 4 --- .../default/include/core/default/StreamIn.h | 4 ++- .../functional/4.0/AudioPrimaryHidlHalUtils.h | 4 +-- .../vts/functional/AudioPrimaryHidlHalTest.h | 20 ++++++------ 8 files changed, 14 insertions(+), 83 deletions(-) delete mode 100644 audio/7.1/IStreamIn.hal diff --git a/audio/7.1/Android.bp b/audio/7.1/Android.bp index 52980e2626..cede72a321 100644 --- a/audio/7.1/Android.bp +++ b/audio/7.1/Android.bp @@ -17,7 +17,6 @@ hidl_interface { "IDevice.hal", "IDevicesFactory.hal", "IPrimaryDevice.hal", - "IStreamIn.hal", "IStreamOut.hal", "IStreamOutLatencyModeCallback.hal", ], diff --git a/audio/7.1/IDevice.hal b/audio/7.1/IDevice.hal index e0b1e92359..c158e7ec79 100644 --- a/audio/7.1/IDevice.hal +++ b/audio/7.1/IDevice.hal @@ -20,7 +20,6 @@ import android.hardware.audio.common@7.0; import @7.0::AudioInOutFlag; import @7.0::IDevice; import @7.0::Result; -import IStreamIn; import IStreamOut; interface IDevice extends @7.0::IDevice { @@ -55,37 +54,6 @@ interface IDevice extends @7.0::IDevice { IStreamOut outStream, AudioConfig suggestedConfig); - /** - * This method creates and opens the audio hardware input stream. - * If the stream can not be opened with the proposed audio config, - * HAL must provide suggested values for the audio config. - * - * Note: INVALID_ARGUMENTS is returned both in the case when the - * HAL can not use the provided config and in the case when - * the value of any argument is invalid. In the latter case the - * HAL must provide a default initialized suggested config. - * - * @param ioHandle handle assigned by AudioFlinger. - * @param device device type and (if needed) address. - * @param config stream configuration. - * @param flags additional flags. - * @param sinkMetadata Description of the audio that is suggested by the client. - * May be used by implementations to configure processing effects. - * @return retval operation completion status. - * @return inStream in case of success, created input stream. - * @return suggestedConfig in the case of rejection of the proposed config, - * a config suggested by the HAL. - */ - openInputStream_7_1( - AudioIoHandle ioHandle, - DeviceAddress device, - AudioConfig config, - vec flags, - SinkMetadata sinkMetadata) generates ( - Result retval, - IStreamIn inStream, - AudioConfig suggestedConfig); - /** * Notifies the device module about the connection state of an input/output * device attached to it. The devicePort identifies the device and may also diff --git a/audio/7.1/IStreamIn.hal b/audio/7.1/IStreamIn.hal deleted file mode 100644 index abebe6a062..0000000000 --- a/audio/7.1/IStreamIn.hal +++ /dev/null @@ -1,22 +0,0 @@ -/* - * Copyright (C) 2022 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package android.hardware.audio@7.1; - -import @7.0::IStreamIn; - -interface IStreamIn extends @7.0::IStreamIn { -}; diff --git a/audio/core/all-versions/default/Device.cpp b/audio/core/all-versions/default/Device.cpp index ca8c03df36..b954fcd6f8 100644 --- a/audio/core/all-versions/default/Device.cpp +++ b/audio/core/all-versions/default/Device.cpp @@ -348,16 +348,6 @@ Return Device::openOutputStream_7_1(int32_t ioHandle, const DeviceAddress& _hidl_cb(result, streamOut, suggestedConfig); return Void(); } - -Return Device::openInputStream_7_1(int32_t ioHandle, const DeviceAddress& device, - const AudioConfig& config, const AudioInputFlags& flags, - const SinkMetadata& sinkMetadata, - openInputStream_7_1_cb _hidl_cb) { - auto [result, streamIn, suggestedConfig] = - openInputStreamImpl(ioHandle, device, config, flags, sinkMetadata); - _hidl_cb(result, streamIn, suggestedConfig); - return Void(); -} #endif // V7.1 Return Device::supportsAudioPatches() { 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 8cde3e0a06..0696f97983 100644 --- a/audio/core/all-versions/default/include/core/default/Device.h +++ b/audio/core/all-versions/default/include/core/default/Device.h @@ -123,10 +123,6 @@ struct Device : public IDevice, public ParametersUtil { const AudioConfig& config, const AudioOutputFlags& flags, const SourceMetadata& sourceMetadata, openOutputStream_7_1_cb _hidl_cb) override; - Return openInputStream_7_1(int32_t ioHandle, const DeviceAddress& device, - const AudioConfig& config, const AudioInputFlags& flags, - const SinkMetadata& sinkMetadata, - openInputStream_7_1_cb _hidl_cb) override; #endif Return supportsAudioPatches() 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 a6346e5db4..4627eecae6 100644 --- a/audio/core/all-versions/default/include/core/default/StreamIn.h +++ b/audio/core/all-versions/default/include/core/default/StreamIn.h @@ -17,7 +17,9 @@ #ifndef ANDROID_HARDWARE_AUDIO_STREAMIN_H #define ANDROID_HARDWARE_AUDIO_STREAMIN_H -#include PATH(android/hardware/audio/FILE_VERSION/IStreamIn.h) +// clang-format off +#include PATH(android/hardware/audio/CORE_TYPES_FILE_VERSION/IStreamIn.h) +// clang-format on #include "Device.h" #include "Stream.h" diff --git a/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalUtils.h b/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalUtils.h index a567cf90e2..83ca9eba17 100644 --- a/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalUtils.h +++ b/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalUtils.h @@ -15,9 +15,9 @@ */ // clang-format off -#include PATH(android/hardware/audio/FILE_VERSION/IStreamIn.h) -#include PATH(android/hardware/audio/FILE_VERSION/IStreamOut.h) +#include PATH(android/hardware/audio/CORE_TYPES_FILE_VERSION/IStreamIn.h) #include PATH(android/hardware/audio/CORE_TYPES_FILE_VERSION/types.h) +#include PATH(android/hardware/audio/FILE_VERSION/IStreamOut.h) #include PATH(android/hardware/audio/common/COMMON_TYPES_FILE_VERSION/types.h) // clang-format on #include diff --git a/audio/core/all-versions/vts/functional/AudioPrimaryHidlHalTest.h b/audio/core/all-versions/vts/functional/AudioPrimaryHidlHalTest.h index 404532a6fa..fa3ee7fdb5 100644 --- a/audio/core/all-versions/vts/functional/AudioPrimaryHidlHalTest.h +++ b/audio/core/all-versions/vts/functional/AudioPrimaryHidlHalTest.h @@ -91,8 +91,9 @@ using ::android::hardware::details::toHexString; using namespace ::android::hardware::audio::common::COMMON_TYPES_CPP_VERSION; using namespace ::android::hardware::audio::common::test::utility; using namespace ::android::hardware::audio::CPP_VERSION; -using ReadParameters = ::android::hardware::audio::CPP_VERSION::IStreamIn::ReadParameters; -using ReadStatus = ::android::hardware::audio::CPP_VERSION::IStreamIn::ReadStatus; +using ReadParameters = + ::android::hardware::audio::CORE_TYPES_CPP_VERSION::IStreamIn::ReadParameters; +using ReadStatus = ::android::hardware::audio::CORE_TYPES_CPP_VERSION::IStreamIn::ReadStatus; using WriteCommand = ::android::hardware::audio::CPP_VERSION::IStreamOut::WriteCommand; using WriteStatus = ::android::hardware::audio::CPP_VERSION::IStreamOut::WriteStatus; #if MAJOR_VERSION >= 7 @@ -1089,7 +1090,7 @@ GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(OutputStreamTest); class StreamReader : public StreamWorker { public: - using IStreamIn = ::android::hardware::audio::CPP_VERSION::IStreamIn; + using IStreamIn = ::android::hardware::audio::CORE_TYPES_CPP_VERSION::IStreamIn; StreamReader(IStreamIn* stream, size_t bufferSize) : mStream(stream), mBufferSize(bufferSize), mData(mBufferSize) {} @@ -1204,7 +1205,8 @@ class StreamReader : public StreamWorker { EventFlag* mEfGroup = nullptr; }; -class InputStreamTest : public OpenStreamTest<::android::hardware::audio::CPP_VERSION::IStreamIn> { +class InputStreamTest + : public OpenStreamTest<::android::hardware::audio::CORE_TYPES_CPP_VERSION::IStreamIn> { void SetUp() override { ASSERT_NO_FATAL_FAILURE(OpenStreamTest::SetUp()); // setup base #if MAJOR_VERSION <= 6 @@ -1226,13 +1228,8 @@ class InputStreamTest : public OpenStreamTest<::android::hardware::audio::CPP_VE auto flags = getInputFlags(); testOpen( [&](AudioIoHandle handle, AudioConfig config, auto cb) { -#if MAJOR_VERSION < 7 || (MAJOR_VERSION == 7 && MINOR_VERSION == 0) return getDevice()->openInputStream(handle, address, config, flags, initMetadata, cb); -#elif MAJOR_VERSION == 7 && MINOR_VERSION == 1 - return getDevice()->openInputStream_7_1(handle, address, config, flags, - initMetadata, cb); -#endif }, config); } @@ -1605,8 +1602,9 @@ TEST_P(InputStreamTest, SetGain) { "InputStream::setGain"); } -static void testPrepareForReading(::android::hardware::audio::CPP_VERSION::IStreamIn* stream, - uint32_t frameSize, uint32_t framesCount) { +static void testPrepareForReading( + ::android::hardware::audio::CORE_TYPES_CPP_VERSION::IStreamIn* stream, uint32_t frameSize, + uint32_t framesCount) { Result res; // Ignore output parameters as the call should fail ASSERT_OK(stream->prepareForReading(frameSize, framesCount,