Merge "AudioFlinger: Add clang tidy checks on build" (42f97ed1) · Commits · e / os / android_frameworks_av

include/media/Interpolator.h

+2 −2

Original line number	Diff line number	Diff line
		@@ -122,7 +122,7 @@ public:
		// monotonic computation.
		// we use lazy computation here - if we precompute in
		// a single pass, duplicate secant computations may be avoided.
		S sec, sec0, sec1;
		S sec{}, sec0{}, sec1{}; // initialization not needed, used for clang-tidy
		if (!catmullRom \|\| monotonic) {
		sec = (high->second - low->second) / interval;
		sec0 = low2 != this->end()
		@@ -269,7 +269,7 @@ public:

		// Note: We don't need to check size is within some bounds as
		// the Parcel read will fail if size is incorrectly specified too large.
		float lastx;
		float lastx = 0.f; // initialization not needed, used for clang tidy
		for (uint32_t i = 0; i < size; ++i) {
		float x = config.xy[i * 2];
		float y = config.xy[i * 2 + 1];

media/audioaidlconversion/include/media/AidlConversionUtil-impl.h

+3 −3

Original line number	Diff line number	Diff line
		@@ -58,15 +58,15 @@ template<typename To, typename From>
		ConversionResult<To> convertIntegral(From from) {
		// Special handling is required for signed / vs. unsigned comparisons, since otherwise we may
		// have the signed converted to unsigned and produce wrong results.
		if (std::is_signed_v<From> && !std::is_signed_v<To>) {
		if constexpr (std::is_signed_v<From> && !std::is_signed_v<To>) {
		if (from < 0 \|\| from > std::numeric_limits<To>::max()) {
		return ::android::base::unexpected(::android::BAD_VALUE);
		}
		} else if (std::is_signed_v<To> && !std::is_signed_v<From>) {
		} else if constexpr (std::is_signed_v<To> && !std::is_signed_v<From>) {
		if (from > std::numeric_limits<To>::max()) {
		return ::android::base::unexpected(::android::BAD_VALUE);
		}
		} else {
		} else /* constexpr */ {
		if (from < std::numeric_limits<To>::min() \|\| from > std::numeric_limits<To>::max()) {
		return ::android::base::unexpected(::android::BAD_VALUE);
		}

media/utils/include/mediautils/MethodStatistics.h

+1 −1

Original line number	Diff line number	Diff line
		@@ -59,7 +59,7 @@ public:
		void event(C&& code, FloatType executeMs) {
		std::lock_guard lg(mLock);
		auto it = mStatisticsMap.lower_bound(code);
		if (it != mStatisticsMap.end() && it->first == code) {
		if (it != mStatisticsMap.end() && it->first == static_cast<Code>(code)) {
		it->second.add(executeMs);
		} else {
		// StatsType ctor takes an optional array of data for initialization.

services/audioflinger/Android.bp

+116 −0

Original line number	Diff line number	Diff line
		@@ -19,11 +19,127 @@ license {
		],
		}

		tidy_errors = [
		// https://clang.llvm.org/extra/clang-tidy/checks/list.html
		// For many categories, the checks are too many to specify individually.
		// Feel free to disable as needed - as warnings are generally ignored,
		// we treat warnings as errors.
		"android-*",
		"bugprone-*",
		"cert-*",
		"clang-analyzer-security*",
		"google-*",
		"misc-*",
		//"modernize-*", // explicitly list the modernize as they can be subjective.
		"modernize-avoid-bind",
		//"modernize-avoid-c-arrays", // std::array<> can be verbose
		"modernize-concat-nested-namespaces",
		//"modernize-deprecated-headers", // C headers still ok even if there is C++ equivalent.
		"modernize-deprecated-ios-base-aliases",
		"modernize-loop-convert",
		"modernize-make-shared",
		"modernize-make-unique",
		// "modernize-pass-by-value",
		"modernize-raw-string-literal",
		"modernize-redundant-void-arg",
		"modernize-replace-auto-ptr",
		"modernize-replace-random-shuffle",
		"modernize-return-braced-init-list",
		"modernize-shrink-to-fit",
		"modernize-unary-static-assert",
		// "modernize-use-auto", // found in MediaMetricsService.h, debatable - auto can obscure type
		"modernize-use-bool-literals",
		"modernize-use-default-member-init",
		"modernize-use-emplace",
		"modernize-use-equals-default",
		"modernize-use-equals-delete",
		// "modernize-use-nodiscard",
		"modernize-use-noexcept",
		"modernize-use-nullptr",
		"modernize-use-override",
		//"modernize-use-trailing-return-type", // not necessarily more readable
		"modernize-use-transparent-functors",
		"modernize-use-uncaught-exceptions",
		"modernize-use-using",
		"performance-*",

		// Remove some pedantic stylistic requirements.
		"-google-readability-casting", // C++ casts not always necessary and may be verbose
		"-google-readability-todo", // do not require TODO(info)

		"-bugprone-unhandled-self-assignment",
		"-bugprone-suspicious-string-compare",
		"-cert-oop54-cpp", // found in TransactionLog.h
		"-bugprone-narrowing-conversions", // b/182410845

		// TODO(b/275642749) Reenable these warnings
		"-bugprone-assignment-in-if-condition",
		"-bugprone-forward-declaration-namespace",
		"-bugprone-parent-virtual-call",
		"-cert-dcl59-cpp",
		"-cert-err34-c",
		"-google-build-namespaces",
		"-google-build-using-namespace",
		"-google-default-arguments",
		"-google-runtime-int",
		"-misc-const-correctness",
		"-misc-non-private-member-variables-in-classes",
		"-modernize-concat-nested-namespaces",
		"-modernize-loop-convert",
		"-modernize-use-default-member-init",
		"-modernize-use-equals-default",
		"-modernize-use-nullptr",
		"-modernize-use-override",
		"-modernize-use-using",
		"-performance-no-int-to-ptr",
		]

		// Eventually use common tidy defaults
		cc_defaults {
		name: "audioflinger_flags_defaults",
		// https://clang.llvm.org/docs/UsersManual.html#command-line-options
		// https://clang.llvm.org/docs/DiagnosticsReference.html
		cflags: [
		"-Wall",
		"-Wdeprecated",
		"-Werror",
		"-Werror=implicit-fallthrough",
		"-Werror=sometimes-uninitialized",
		"-Werror=conditional-uninitialized",
		"-Wextra",

		// suppress some warning chatter.
		"-Wno-deprecated-copy-with-dtor",
		"-Wno-deprecated-copy-with-user-provided-dtor",

		"-Wredundant-decls",
		"-Wshadow",
		"-Wstrict-aliasing",
		"-fstrict-aliasing",
		"-Wthread-safety",
		//"-Wthread-safety-negative", // experimental - looks broken in R.
		"-Wunreachable-code",
		"-Wunreachable-code-break",
		"-Wunreachable-code-return",
		"-Wunused",
		"-Wused-but-marked-unused",
		"-D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS",
		],
		// https://clang.llvm.org/extra/clang-tidy/
		tidy: true,
		tidy_checks: tidy_errors,
		tidy_checks_as_errors: tidy_errors,
		tidy_flags: [
		"-format-style=file",
		],
		}

		cc_library_shared {
		name: "libaudioflinger",

		defaults: [
		"latest_android_media_audio_common_types_cpp_shared",
		"audioflinger_flags_defaults",
		],

		srcs: [

services/audioflinger/AudioFlinger.cpp

+31 −22

Original line number	Diff line number	Diff line
		@@ -714,7 +714,7 @@ void AudioFlinger::onExternalVibrationStop(const sp<os::ExternalVibration>& exte
		}

		status_t AudioFlinger::addEffectToHal(audio_port_handle_t deviceId,
		audio_module_handle_t hwModuleId, sp<EffectHalInterface> effect) {
		audio_module_handle_t hwModuleId, const sp<EffectHalInterface>& effect) {
		AutoMutex lock(mHardwareLock);
		AudioHwDevice *audioHwDevice = mAudioHwDevs.valueFor(hwModuleId);
		if (audioHwDevice == nullptr) {
		@@ -724,7 +724,7 @@ status_t AudioFlinger::addEffectToHal(audio_port_handle_t deviceId,
		}

		status_t AudioFlinger::removeEffectFromHal(audio_port_handle_t deviceId,
		audio_module_handle_t hwModuleId, sp<EffectHalInterface> effect) {
		audio_module_handle_t hwModuleId, const sp<EffectHalInterface>& effect) {
		AutoMutex lock(mHardwareLock);
		AudioHwDevice *audioHwDevice = mAudioHwDevs.valueFor(hwModuleId);
		if (audioHwDevice == nullptr) {
		@@ -842,6 +842,7 @@ bool AudioFlinger::dumpTryLock(Mutex& mutex)
		}

		status_t AudioFlinger::dump(int fd, const Vector<String16>& args)
		NO_THREAD_SAFETY_ANALYSIS // conditional try lock
		{
		if (!dumpAllowed()) {
		dumpPermissionDenial(fd, args);
		@@ -944,7 +945,6 @@ status_t AudioFlinger::dump(int fd, const Vector<String16>& args)
		// to lookup the service if it's not running, as it will block for a second
		if (sMediaLogServiceAsBinder != 0) {
		dprintf(fd, "\nmedia.log:\n");
		Vector<String16> args;
		sMediaLogServiceAsBinder->dump(fd, args);
		}

		@@ -1441,9 +1441,10 @@ status_t AudioFlinger::setMode(audio_mode_t mode)
		if (NO_ERROR == ret) {
		Mutex::Autolock _l(mLock);
		mMode = mode;
		for (size_t i = 0; i < mPlaybackThreads.size(); i++)
		for (size_t i = 0; i < mPlaybackThreads.size(); i++) {
		mPlaybackThreads.valueAt(i)->setMode(mode);
		}
		}

		mediametrics::LogItem(mMetricsId)
		.set(AMEDIAMETRICS_PROP_EVENT, AMEDIAMETRICS_PROP_EVENT_VALUE_SETMODE)
		@@ -1780,7 +1781,7 @@ void AudioFlinger::updateOutDevicesForRecordThreads_l(const DeviceDescriptorBase
		// forwardAudioHwSyncToDownstreamPatches_l() must be called with AudioFlinger::mLock held
		void AudioFlinger::forwardParametersToDownstreamPatches_l(
		audio_io_handle_t upStream, const String8& keyValuePairs,
		std::function<bool(const sp<PlaybackThread>&)> useThread)
		const std::function<bool(const sp<PlaybackThread>&)>& useThread)
		{
		std::vector<PatchPanel::SoftwarePatch> swPatches;
		if (mPatchPanel.getDownstreamSoftwarePatches(upStream, &swPatches) != OK) return;
		@@ -1796,7 +1797,7 @@ void AudioFlinger::forwardParametersToDownstreamPatches_l(

		// Update downstream patches for all playback threads attached to an MSD module
		void AudioFlinger::updateDownStreamPatches_l(const struct audio_patch *patch,
		const std::set<audio_io_handle_t> streams)
		const std::set<audio_io_handle_t>& streams)
		{
		for (const audio_io_handle_t stream : streams) {
		PlaybackThread *playbackThread = checkPlaybackThread_l(stream);
		@@ -2002,24 +2003,29 @@ size_t AudioFlinger::getInputBufferSize(uint32_t sampleRate, audio_format_t form
		mHardwareStatus = AUDIO_HW_GET_INPUT_BUFFER_SIZE;

		sp<DeviceHalInterface> dev = mPrimaryHardwareDev->hwDevice();

		std::vector<audio_channel_mask_t> channelMasks = {channelMask};
		if (channelMask != AUDIO_CHANNEL_IN_MONO)
		if (channelMask != AUDIO_CHANNEL_IN_MONO) {
		channelMasks.push_back(AUDIO_CHANNEL_IN_MONO);
		if (channelMask != AUDIO_CHANNEL_IN_STEREO)
		}
		if (channelMask != AUDIO_CHANNEL_IN_STEREO) {
		channelMasks.push_back(AUDIO_CHANNEL_IN_STEREO);
		}

		std::vector<audio_format_t> formats = {format};
		if (format != AUDIO_FORMAT_PCM_16_BIT)
		if (format != AUDIO_FORMAT_PCM_16_BIT) {
		formats.push_back(AUDIO_FORMAT_PCM_16_BIT);
		}

		std::vector<uint32_t> sampleRates = {sampleRate};
		static const uint32_t SR_44100 = 44100;
		static const uint32_t SR_48000 = 48000;

		if (sampleRate != SR_48000)
		if (sampleRate != SR_48000) {
		sampleRates.push_back(SR_48000);
		if (sampleRate != SR_44100)
		}
		if (sampleRate != SR_44100) {
		sampleRates.push_back(SR_44100);
		}

		mHardwareStatus = AUDIO_HW_IDLE;

		@@ -2488,7 +2494,7 @@ status_t AudioFlinger::createRecord(const media::CreateRecordRequest& _input,
		// session and move it to this thread.
		sp<EffectChain> chain = getOrphanEffectChain_l(sessionId);
		if (chain != 0) {
		Mutex::Autolock _l(thread->mLock);
		Mutex::Autolock _l2(thread->mLock);
		thread->addEffectChain_l(chain);
		}
		break;
		@@ -2868,7 +2874,7 @@ void AudioFlinger::setAudioHwSyncForSession_l(PlaybackThread *thread, audio_sess
		sp<AudioFlinger::ThreadBase> AudioFlinger::openOutput_l(audio_module_handle_t module,
		audio_io_handle_t *output,
		audio_config_t *halConfig,
		audio_config_base_t *mixerConfig __unused,
		audio_config_base_t *mixerConfig,
		audio_devices_t deviceType,
		const String8& address,
		audio_output_flags_t flags)
		@@ -2990,7 +2996,6 @@ status_t AudioFlinger::openOutput(const media::OpenOutputRequest& request,
		aidl2legacy_int32_t_audio_output_flags_t_mask(request.flags));

		audio_io_handle_t output;
		uint32_t latencyMs;

		ALOGI("openOutput() this %p, module %d Device %s, SamplingRate %d, Format %#08x, "
		"Channels %#x, flags %#x",
		@@ -3013,6 +3018,7 @@ status_t AudioFlinger::openOutput(const media::OpenOutputRequest& request,
		sp<ThreadBase> thread = openOutput_l(module, &output, &halConfig,
		&mixerConfig, deviceType, address, flags);
		if (thread != 0) {
		uint32_t latencyMs = 0;
		if ((flags & AUDIO_OUTPUT_FLAG_MMAP_NOIRQ) == 0) {
		PlaybackThread playbackThread = (PlaybackThread )thread.get();
		latencyMs = playbackThread->latency();
		@@ -3361,7 +3367,7 @@ status_t AudioFlinger::closeInput_nonvirtual(audio_io_handle_t input)
		continue;
		}
		if (t->hasAudioSession(chain->sessionId()) != 0) {
		Mutex::Autolock _l(t->mLock);
		Mutex::Autolock _l2(t->mLock);
		ALOGV("closeInput() found thread %d for effect session %d",
		t->id(), chain->sessionId());
		t->addEffectChain_l(chain);
		@@ -3566,7 +3572,8 @@ std::vector<sp<AudioFlinger::EffectModule>> AudioFlinger::purgeStaleEffects_l()
		}

		for (size_t i = 0; i < chains.size(); i++) {
		sp<EffectChain> ec = chains[i];
		// clang-tidy suggests const ref
		sp<EffectChain> ec = chains[i]; // NOLINT(performance-unnecessary-copy-initialization)
		int sessionid = ec->sessionId();
		sp<ThreadBase> t = ec->thread().promote();
		if (t == 0) {
		@@ -3735,7 +3742,7 @@ DeviceTypeSet AudioFlinger::primaryOutputDevice_l() const
		PlaybackThread *thread = primaryPlaybackThread_l();

		if (thread == NULL) {
		return DeviceTypeSet();
		return {};
		}

		return thread->outDeviceTypes();
		@@ -4247,7 +4254,7 @@ status_t AudioFlinger::createEffect(const media::CreateEffectRequest& request,
		// session and used it instead of creating a new one.
		sp<EffectChain> chain = getOrphanEffectChain_l(sessionId);
		if (chain != 0) {
		Mutex::Autolock _l(thread->mLock);
		Mutex::Autolock _l2(thread->mLock);
		thread->addEffectChain_l(chain);
		}
		}
		@@ -4365,6 +4372,7 @@ void AudioFlinger::setEffectSuspended(int effectId,
		status_t AudioFlinger::moveEffectChain_l(audio_session_t sessionId,
		AudioFlinger::PlaybackThread *srcThread,
		AudioFlinger::PlaybackThread *dstThread)
		NO_THREAD_SAFETY_ANALYSIS // requires srcThread and dstThread locks
		{
		ALOGV("moveEffectChain_l() session %d from thread %p to thread %p",
		sessionId, srcThread, dstThread);
		@@ -4394,11 +4402,12 @@ status_t AudioFlinger::moveEffectChain_l(audio_session_t sessionId,
		// transfer all effects one by one so that new effect chain is created on new thread with
		// correct buffer sizes and audio parameters and effect engines reconfigured accordingly
		sp<EffectChain> dstChain;
		sp<EffectModule> effect = chain->getEffectFromId_l(0);
		Vector< sp<EffectModule> > removed;
		status_t status = NO_ERROR;
		std::string errorString;
		while (effect != nullptr) {
		// process effects one by one.
		for (sp<EffectModule> effect = chain->getEffectFromId_l(0); effect != nullptr;
		effect = chain->getEffectFromId_l(0)) {
		srcThread->removeEffect_l(effect);
		removed.add(effect);
		status = dstThread->addEffect_l(effect);
		@@ -4419,7 +4428,6 @@ status_t AudioFlinger::moveEffectChain_l(audio_session_t sessionId,
		break;
		}
		}
		effect = chain->getEffectFromId_l(0);
		}

		size_t restored = 0;
		@@ -4523,6 +4531,7 @@ Exit:
		}

		bool AudioFlinger::isNonOffloadableGlobalEffectEnabled_l()
		NO_THREAD_SAFETY_ANALYSIS // thread lock for getEffectChain_l.
		{
		if (mGlobalEffectEnableTime != 0 &&
		((systemTime() - mGlobalEffectEnableTime) < kMinGlobalEffectEnabletimeNs)) {