AudioFlinger: Util clang-tidy fixes
Compile with AUDIO_WATCHDOG and TEE_SINK Configuration.h
defines set.
Test: ALLOW_LOCAL_TIDY_TRUE=1 mm -j .
Bug: 286457446
Merged-In: I7a7e67599c55c42ebbf513f3b7736e7831c94741
Change-Id: I7a7e67599c55c42ebbf513f3b7736e7831c94741
(cherry picked from commit 52d599cf8392662c71f5a9662349f13cb412b2e4)
diff --git a/services/audioflinger/afutils/AudioWatchdog.cpp b/services/audioflinger/afutils/AudioWatchdog.cpp
index 3116584..48a07a5 100644
--- a/services/audioflinger/afutils/AudioWatchdog.cpp
+++ b/services/audioflinger/afutils/AudioWatchdog.cpp
@@ -38,12 +38,12 @@
mUnderruns, mLogs, buf);
}
-bool AudioWatchdog::threadLoop()
+bool AudioWatchdog::threadLoop() NO_THREAD_SAFETY_ANALYSIS // unique_lock
{
{
- const AutoMutex _l(mMyLock);
+ std::unique_lock _l(mLock);
if (mPaused) {
- mMyCond.wait(mMyLock);
+ mCond.wait(_l);
// ignore previous timestamp after resume()
mOldTsValid = false;
// force an immediate log on first underrun after resume()
@@ -65,7 +65,7 @@
return true;
}
time_t sec = newTs.tv_sec - mOldTs.tv_sec;
- long nsec = newTs.tv_nsec - mOldTs.tv_nsec;
+ auto nsec = newTs.tv_nsec - mOldTs.tv_nsec;
if (nsec < 0) {
--sec;
nsec += 1000000000;
@@ -81,7 +81,8 @@
}
}
mLogTs.tv_sec += sec;
- if ((mLogTs.tv_nsec += nsec) >= 1000000000) {
+ mLogTs.tv_nsec += nsec;
+ if (mLogTs.tv_nsec >= 1000000000) {
mLogTs.tv_sec++;
mLogTs.tv_nsec -= 1000000000;
}
@@ -89,7 +90,7 @@
mDump->mUnderruns = ++mUnderruns;
if (mLogTs.tv_sec >= MIN_TIME_BETWEEN_LOGS_SEC) {
mDump->mLogs = ++mLogs;
- mDump->mMostRecent = time(NULL);
+ mDump->mMostRecent = time(nullptr /* tloc */);
ALOGW("Insufficient CPU for load: expected=%.1f actual=%.1f ms; underruns=%u logs=%u",
mPeriodNs * 1e-6, cycleNs * 1e-6, mUnderruns, mLogs);
mLogTs.tv_sec = 0;
@@ -99,7 +100,7 @@
struct timespec req;
req.tv_sec = 0;
req.tv_nsec = mPeriodNs;
- rc = nanosleep(&req, NULL);
+ rc = nanosleep(&req, nullptr /* remaining */);
if (!((rc == 0) || (rc == -1 && errno == EINTR))) {
pause();
return false;
@@ -116,22 +117,23 @@
void AudioWatchdog::pause()
{
- const AutoMutex _l(mMyLock);
+ const std::lock_guard _l(mLock);
mPaused = true;
}
void AudioWatchdog::resume()
{
- const AutoMutex _l(mMyLock);
+ const std::lock_guard _l(mLock);
if (mPaused) {
mPaused = false;
- mMyCond.signal();
+ mCond.notify_one();
}
}
void AudioWatchdog::setDump(AudioWatchdogDump *dump)
{
- mDump = dump != NULL ? dump : &mDummyDump;
+ const std::lock_guard _l(mLock);
+ mDump = dump != nullptr ? dump : &mDummyDump;
}
} // namespace android
diff --git a/services/audioflinger/afutils/AudioWatchdog.h b/services/audioflinger/afutils/AudioWatchdog.h
index 217761a..1f5dad4 100644
--- a/services/audioflinger/afutils/AudioWatchdog.h
+++ b/services/audioflinger/afutils/AudioWatchdog.h
@@ -21,6 +21,7 @@
#pragma once
+#include <mutex>
#include <time.h>
#include <utils/Thread.h>
@@ -41,7 +42,6 @@
explicit AudioWatchdog(unsigned periodMs = 50) : Thread(false /*canCallJava*/),
mPeriodNs(periodMs * 1000000), mMaxCycleNs(mPeriodNs * 2)
{
- constexpr int32_t MIN_TIME_BETWEEN_LOGS_SEC = 60;
// force an immediate log on first underrun
mLogTs.tv_sec = MIN_TIME_BETWEEN_LOGS_SEC;
mLogTs.tv_nsec = 0;
@@ -52,8 +52,8 @@
void requestExit() override;
// FIXME merge API and implementation with AudioTrackThread
- void pause(); // suspend thread from execution at next loop boundary
- void resume(); // allow thread to execute, if not requested to exit
+ void pause(); // suspend thread from execution at next loop boundary
+ void resume(); // allow thread to execute, if not requested to exit
// Where to store the dump, or NULL to not update
void setDump(AudioWatchdogDump* dump);
@@ -61,18 +61,21 @@
private:
bool threadLoop() override;
- Mutex mMyLock; // Thread::mLock is private
- Condition mMyCond; // Thread::mThreadExitedCondition is private
- bool mPaused = false; // whether thread is currently paused
+ static constexpr int32_t MIN_TIME_BETWEEN_LOGS_SEC = 60;
+ const uint32_t mPeriodNs; // nominal period
+ const uint32_t mMaxCycleNs; // maximum allowed time of one cycle before declaring underrun
- uint32_t mPeriodNs; // nominal period
- uint32_t mMaxCycleNs; // maximum allowed time of one cycle before declaring underrun
- struct timespec mOldTs; // monotonic time when threadLoop last ran
- struct timespec mLogTs; // time since last log (initialized in ctor).
- bool mOldTsValid = false; // whether mOldTs is valid
- uint32_t mUnderruns = 0; // total number of underruns
- uint32_t mLogs = 0; // total number of logs
- AudioWatchdogDump* mDump{&mDummyDump}; // where to store the dump, always non-NULL
+ mutable std::mutex mLock; // Thread::mLock is private
+ std::condition_variable mCond; // Thread::mThreadExitedCondition is private
+ bool mPaused GUARDED_BY(mLock) = false; // whether thread is currently paused
+ bool mOldTsValid GUARDED_BY(mLock) = false; // whether mOldTs is valid
+ struct timespec mOldTs GUARDED_BY(mLock); // monotonic time when threadLoop last ran
+ struct timespec mLogTs GUARDED_BY(mLock); // time since last log (ctor init).
+ uint32_t mUnderruns GUARDED_BY(mLock) = 0; // total number of underruns
+ uint32_t mLogs GUARDED_BY(mLock) = 0; // total number of logs
+
+ // where to store the dump, always non-NULL
+ AudioWatchdogDump* mDump GUARDED_BY(mLock) = &mDummyDump;
AudioWatchdogDump mDummyDump; // default area for dump in case setDump() is not called
};
diff --git a/services/audioflinger/afutils/BufLog.cpp b/services/audioflinger/afutils/BufLog.cpp
index 47efe00..508022f 100644
--- a/services/audioflinger/afutils/BufLog.cpp
+++ b/services/audioflinger/afutils/BufLog.cpp
@@ -58,26 +58,14 @@
// BufLog
// ------------------------------
-BufLog::BufLog() {
- memset(mStreams, 0, sizeof(mStreams));
-}
-
BufLog::~BufLog() {
- const AutoMutex autoLock(mLock);
-
- for (unsigned int id = 0; id < BUFLOG_MAXSTREAMS; id++) { // NOLINT(modernize-loop-convert)
- BufLogStream *pBLStream = mStreams[id];
- if (pBLStream != nullptr) {
- delete pBLStream ;
- mStreams[id] = nullptr;
- }
- }
+ reset();
}
size_t BufLog::write(int streamid, const char *tag, int format, int channels,
int samplingRate, size_t maxBytes, const void *buf, size_t size) {
const unsigned int id = streamid % BUFLOG_MAXSTREAMS;
- const AutoMutex autoLock(mLock);
+ const std::lock_guard autoLock(mLock);
BufLogStream *pBLStream = mStreams[id];
@@ -90,15 +78,12 @@
}
void BufLog::reset() {
- const AutoMutex autoLock(mLock);
- ALOGV("Resetting all BufLogs");
+ const std::lock_guard autoLock(mLock);
int count = 0;
-
- for (unsigned int id = 0; id < BUFLOG_MAXSTREAMS; id++) { // NOLINT(modernize-loop-convert)
- BufLogStream *pBLStream = mStreams[id];
+ for (auto &pBLStream : mStreams) {
if (pBLStream != nullptr) {
delete pBLStream;
- mStreams[id] = nullptr;
+ pBLStream = nullptr;
count++;
}
}
@@ -116,8 +101,6 @@
unsigned int samplingRate,
size_t maxBytes = 0) : mId(id), mFormat(format), mChannels(channels),
mSamplingRate(samplingRate), mMaxBytes(maxBytes) {
- mByteCount = 0;
- mPaused = false;
if (tag != nullptr) {
(void)audio_utils_strlcpy(mTag, tag);
} else {
@@ -157,7 +140,7 @@
BufLogStream::~BufLogStream() {
ALOGV("Destroying BufLogStream id:%d tag:%s", mId, mTag);
- const AutoMutex autoLock(mLock);
+ const std::lock_guard autoLock(mLock);
closeStream_l();
}
@@ -166,7 +149,7 @@
size_t bytes = 0;
if (!mPaused && mFile != nullptr) {
if (size > 0 && buf != nullptr) {
- const AutoMutex autoLock(mLock);
+ const std::lock_guard autoLock(mLock);
if (mMaxBytes > 0) {
size = MIN(size, mMaxBytes - mByteCount);
}
@@ -192,7 +175,7 @@
}
void BufLogStream::finalize() {
- const AutoMutex autoLock(mLock);
+ const std::lock_guard autoLock(mLock);
closeStream_l();
}
diff --git a/services/audioflinger/afutils/BufLog.h b/services/audioflinger/afutils/BufLog.h
index 0bb16d6..a58d073 100644
--- a/services/audioflinger/afutils/BufLog.h
+++ b/services/audioflinger/afutils/BufLog.h
@@ -98,10 +98,10 @@
BufLogSingleton::instance()->reset(); } } while (0)
#endif
+#include <mutex>
#include <stdint.h>
#include <stdio.h>
#include <sys/types.h>
-#include <utils/Mutex.h>
//BufLog configuration
#define BUFLOGSTREAM_MAX_TAGSIZE 32
@@ -135,26 +135,24 @@
void finalize();
private:
- bool mPaused;
const unsigned int mId;
- char mTag[BUFLOGSTREAM_MAX_TAGSIZE + 1];
const unsigned int mFormat;
const unsigned int mChannels;
const unsigned int mSamplingRate;
const size_t mMaxBytes;
- size_t mByteCount;
- FILE *mFile;
- mutable android::Mutex mLock;
+ char mTag[BUFLOGSTREAM_MAX_TAGSIZE + 1]; // const, set in ctor.
+
+ mutable std::mutex mLock;
+ bool mPaused = false;
+ size_t mByteCount = 0;
+ FILE *mFile; // set in ctor
void closeStream_l();
};
-
class BufLog {
public:
- BufLog();
~BufLog();
- BufLog(BufLog const&) {};
// streamid: int [0:BUFLOG_MAXSTREAMS-1] buffer id.
// If a buffer doesn't exist, it is created the first time is referenced
@@ -181,9 +179,9 @@
void reset();
protected:
- static const unsigned int BUFLOG_MAXSTREAMS = 16;
- BufLogStream *mStreams[BUFLOG_MAXSTREAMS];
- mutable android::Mutex mLock;
+ static constexpr size_t BUFLOG_MAXSTREAMS = 16;
+ mutable std::mutex mLock;
+ BufLogStream *mStreams[BUFLOG_MAXSTREAMS]{};
};
class BufLogSingleton {
diff --git a/services/audioflinger/afutils/NBAIO_Tee.cpp b/services/audioflinger/afutils/NBAIO_Tee.cpp
index d0682b5..49057ce 100644
--- a/services/audioflinger/afutils/NBAIO_Tee.cpp
+++ b/services/audioflinger/afutils/NBAIO_Tee.cpp
@@ -91,8 +91,8 @@
/** returns filename of created audio file, else empty string on failure. */
std::string create(
- std::function<ssize_t /* frames_read */
- (void * /* buffer */, size_t /* size_in_frames */)> reader,
+ const std::function<ssize_t /* frames_read */
+ (void * /* buffer */, size_t /* size_in_frames */)>& reader,
uint32_t sampleRate,
uint32_t channelCount,
audio_format_t format,
@@ -109,8 +109,8 @@
/** creates an audio file from a reader functor passed in. */
status_t createInternal(
- std::function<ssize_t /* frames_read */
- (void * /* buffer */, size_t /* size_in_frames */)> reader,
+ const std::function<ssize_t /* frames_read */
+ (void * /* buffer */, size_t /* size_in_frames */)>& reader,
uint32_t sampleRate,
uint32_t channelCount,
audio_format_t format,
@@ -159,30 +159,29 @@
// yet another ThreadPool implementation.
class ThreadPool {
public:
- ThreadPool(size_t size)
+ explicit ThreadPool(size_t size)
: mThreadPoolSize(size)
{ }
/** launches task "name" with associated function "func".
if the threadpool is exhausted, it will launch on calling function */
- status_t launch(const std::string &name, std::function<status_t()> func);
+ status_t launch(const std::string &name, const std::function<status_t()>& func);
private:
+ const size_t mThreadPoolSize;
std::mutex mLock;
std::list<std::pair<
- std::string, std::future<status_t>>> mFutures; // GUARDED_BY(mLock)
-
- const size_t mThreadPoolSize;
+ std::string, std::future<status_t>>> mFutures; // GUARDED_BY(mLock);
} mThreadPool;
- const std::string mPrefix;
- std::mutex mLock;
- std::string mDirectory; // GUARDED_BY(mLock)
- std::deque<std::string> mFiles; // GUARDED_BY(mLock) sorted list of files by creation time
-
static constexpr size_t FRAMES_PER_READ = 1024;
static constexpr size_t MAX_FILES_READ = 1024;
static constexpr size_t MAX_FILES_KEEP = 32;
+
+ const std::string mPrefix;
+ std::mutex mLock;
+ std::string mDirectory; // GUARDED_BY(mLock);
+ std::deque<std::string> mFiles; // GUARDED_BY(mLock); // sorted list of files by creation time
};
/* static */
@@ -200,7 +199,7 @@
const NBAIO_Format format = source->format();
bool firstRead = true;
- std::string filename = audioFileHandler.create(
+ const std::string filename = audioFileHandler.create(
// this functor must not hold references to stack
[firstRead, sinkSource] (void *buffer, size_t frames) mutable {
auto &source = sinkSource.second;
@@ -253,14 +252,14 @@
}
std::string AudioFileHandler::create(
- std::function<ssize_t /* frames_read */
- (void * /* buffer */, size_t /* size_in_frames */)> reader,
+ const std::function<ssize_t /* frames_read */
+ (void * /* buffer */, size_t /* size_in_frames */)>& reader,
uint32_t sampleRate,
uint32_t channelCount,
audio_format_t format,
const std::string &suffix)
{
- const std::string filename = generateFilename(suffix);
+ std::string filename = generateFilename(suffix);
if (mThreadPool.launch(std::string("create ") + filename,
[=]() { return createInternal(reader, sampleRate, channelCount, format, filename); })
@@ -314,7 +313,7 @@
std::sort(files.begin() + toRemove, files.end());
{
- std::lock_guard<std::mutex> _l(mLock);
+ const std::lock_guard<std::mutex> _l(mLock);
mDirectory = directory;
mFiles = std::move(files);
@@ -332,13 +331,13 @@
std::vector<std::string> filesToRemove;
std::string dir;
{
- std::lock_guard<std::mutex> _l(mLock);
+ const std::lock_guard<std::mutex> _l(mLock);
if (!isDirectoryValid(mDirectory)) return NO_INIT;
dir = mDirectory;
if (mFiles.size() > MAX_FILES_KEEP) {
- size_t toRemove = mFiles.size() - MAX_FILES_KEEP;
+ const size_t toRemove = mFiles.size() - MAX_FILES_KEEP;
// use move and erase to efficiently transfer std::string
std::move(mFiles.begin(),
@@ -348,7 +347,7 @@
}
}
- std::string dirp = dir + "/";
+ const std::string dirp = dir + "/";
// remove files outside of lock for better concurrency.
for (const auto &file : filesToRemove) {
(void)unlink((dirp + file).c_str());
@@ -362,14 +361,14 @@
}
status_t AudioFileHandler::ThreadPool::launch(
- const std::string &name, std::function<status_t()> func)
+ const std::string &name, const std::function<status_t()>& func)
{
if (mThreadPoolSize > 1) {
- std::lock_guard<std::mutex> _l(mLock);
+ const std::lock_guard<std::mutex> _l(mLock);
if (mFutures.size() >= mThreadPoolSize) {
for (auto it = mFutures.begin(); it != mFutures.end();) {
const std::string &filename = it->first;
- std::future<status_t> &future = it->second;
+ const std::future<status_t> &future = it->second;
if (!future.valid() ||
future.wait_for(std::chrono::seconds(0)) == std::future_status::ready) {
ALOGV("%s: future %s ready", __func__, filename.c_str());
@@ -391,8 +390,8 @@
}
status_t AudioFileHandler::createInternal(
- std::function<ssize_t /* frames_read */
- (void * /* buffer */, size_t /* size_in_frames */)> reader,
+ const std::function<ssize_t /* frames_read */
+ (void * /* buffer */, size_t /* size_in_frames */)>& reader,
uint32_t sampleRate,
uint32_t channelCount,
audio_format_t format,
@@ -431,9 +430,9 @@
}
std::string directory;
- status_t status = clean(&directory);
+ const status_t status = clean(&directory);
if (status != NO_ERROR) return status;
- std::string dirPrefix = directory + "/";
+ const std::string dirPrefix = directory + "/";
const std::string path = dirPrefix + filename;
@@ -505,7 +504,7 @@
// Success: add our name to managed files.
{
- std::lock_guard<std::mutex> _l(mLock);
+ const std::lock_guard<std::mutex> _l(mLock);
// weak synchronization - only update mFiles if the directory hasn't changed.
if (mDirectory == directory) {
mFiles.emplace_back(filename); // add to the end to preserve sort.
diff --git a/services/audioflinger/afutils/NBAIO_Tee.h b/services/audioflinger/afutils/NBAIO_Tee.h
index 918cca2..17b6175 100644
--- a/services/audioflinger/afutils/NBAIO_Tee.h
+++ b/services/audioflinger/afutils/NBAIO_Tee.h
@@ -216,7 +216,7 @@
// Note: as mentioned in NBAIO_Tee::set(), don't call set() while write() is
// ongoing.
if (enabled) {
- std::lock_guard<std::mutex> _l(mLock);
+ const std::lock_guard<std::mutex> _l(mLock);
mFlags = flags;
mFormat = format; // could get this from the Sink.
mFrames = frames;
@@ -228,7 +228,7 @@
}
void setId(const std::string &id) {
- std::lock_guard<std::mutex> _l(mLock);
+ const std::lock_guard<std::mutex> _l(mLock);
mId = id;
}
@@ -237,7 +237,7 @@
std::string suffix;
NBAIO_SinkSource sinkSource;
{
- std::lock_guard<std::mutex> _l(mLock);
+ const std::lock_guard<std::mutex> _l(mLock);
suffix = mId + reason;
sinkSource = mSinkSource;
}
@@ -281,13 +281,13 @@
class RunningTees {
public:
void add(const std::shared_ptr<NBAIO_TeeImpl> &tee) {
- std::lock_guard<std::mutex> _l(mLock);
+ const std::lock_guard<std::mutex> _l(mLock);
ALOGW_IF(!mTees.emplace(tee).second,
"%s: %p already exists in mTees", __func__, tee.get());
}
void remove(const std::shared_ptr<NBAIO_TeeImpl> &tee) {
- std::lock_guard<std::mutex> _l(mLock);
+ const std::lock_guard<std::mutex> _l(mLock);
ALOGW_IF(mTees.erase(tee) != 1,
"%s: %p doesn't exist in mTees", __func__, tee.get());
}
@@ -295,7 +295,7 @@
void dump(int fd, const std::string &reason) {
std::vector<std::shared_ptr<NBAIO_TeeImpl>> tees; // safe snapshot of tees
{
- std::lock_guard<std::mutex> _l(mLock);
+ const std::lock_guard<std::mutex> _l(mLock);
tees.insert(tees.end(), mTees.begin(), mTees.end());
}
for (const auto &tee : tees) {
@@ -320,4 +320,6 @@
const std::shared_ptr<NBAIO_TeeImpl> mTee;
}; // NBAIO_Tee
+} // namespace android
+
#endif // TEE_SINK