SoundPool: Add clang-tidy and fix

Test: soundpool_stress
Test: SoundPoolAacTest
Test: SoundPoolHapticTest
Test: SoundPoolMidiTest
Test: SoundPoolOggTest
Bug: 157501605
Change-Id: I7bba857a8b44b6255d423086127da0e2df4ce1c4
diff --git a/media/jni/soundpool/Android.bp b/media/jni/soundpool/Android.bp
index aa32793..6141308 100644
--- a/media/jni/soundpool/Android.bp
+++ b/media/jni/soundpool/Android.bp
@@ -1,5 +1,92 @@
+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",  // 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)
+    "-google-build-using-namespace", // Reenable and fix later.
+]
+
+cc_defaults {
+    name: "soundpool_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",
+        "-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",
+    ],
+    // https://clang.llvm.org/extra/clang-tidy/
+    tidy: true,
+    tidy_checks: tidy_errors,
+    tidy_checks_as_errors: tidy_errors,
+    tidy_flags: [
+      "-format-style='file'",
+      "--header-filter='frameworks/base/media/jni/soundpool'",
+    ],
+}
+
 cc_library_shared {
     name: "libsoundpool",
+    defaults: [
+        "soundpool_flags_defaults",
+    ],
 
     srcs: [
         "android_media_SoundPool.cpp",
diff --git a/media/jni/soundpool/Sound.cpp b/media/jni/soundpool/Sound.cpp
index 0bbc3e4..c3abdc2 100644
--- a/media/jni/soundpool/Sound.cpp
+++ b/media/jni/soundpool/Sound.cpp
@@ -31,7 +31,7 @@
 
 Sound::Sound(int32_t soundID, int fd, int64_t offset, int64_t length)
     : mSoundID(soundID)
-    , mFd(dup(fd))
+    , mFd(fcntl(fd, F_DUPFD_CLOEXEC)) // like dup(fd) but closes on exec to prevent leaks.
     , mOffset(offset)
     , mLength(length)
 {
@@ -47,7 +47,7 @@
 
 static status_t decode(int fd, int64_t offset, int64_t length,
         uint32_t *rate, int32_t *channelCount, audio_format_t *audioFormat,
-        audio_channel_mask_t *channelMask, sp<MemoryHeapBase> heap,
+        audio_channel_mask_t *channelMask, const sp<MemoryHeapBase>& heap,
         size_t *sizeInBytes) {
     ALOGV("%s(fd=%d, offset=%lld, length=%lld, ...)",
             __func__, fd, (long long)offset, (long long)length);
@@ -81,7 +81,7 @@
 
             bool sawInputEOS = false;
             bool sawOutputEOS = false;
-            uint8_t* writePos = static_cast<uint8_t*>(heap->getBase());
+            auto writePos = static_cast<uint8_t*>(heap->getBase());
             size_t available = heap->getSize();
             size_t written = 0;
             format.reset(AMediaCodec_getOutputFormat(codec.get())); // update format.
@@ -204,7 +204,7 @@
         int32_t channelCount;
         audio_format_t format;
         audio_channel_mask_t channelMask;
-        status_t status = decode(mFd.get(), mOffset, mLength, &sampleRate, &channelCount, &format,
+        status = decode(mFd.get(), mOffset, mLength, &sampleRate, &channelCount, &format,
                         &channelMask, mHeap, &mSizeInBytes);
         ALOGV("%s: close(%d)", __func__, mFd.get());
         mFd.reset();  // close
diff --git a/media/jni/soundpool/SoundDecoder.cpp b/media/jni/soundpool/SoundDecoder.cpp
index 12200ef..6614fdb 100644
--- a/media/jni/soundpool/SoundDecoder.cpp
+++ b/media/jni/soundpool/SoundDecoder.cpp
@@ -57,7 +57,7 @@
     mThreadPool->quit();
 }
 
-void SoundDecoder::run(int32_t id __unused /* ALOGV only */)
+void SoundDecoder::run(int32_t id)
 {
     ALOGV("%s(%d): entering", __func__, id);
     std::unique_lock lock(mLock);
diff --git a/media/jni/soundpool/SoundDecoder.h b/media/jni/soundpool/SoundDecoder.h
index 1288943..7b62114 100644
--- a/media/jni/soundpool/SoundDecoder.h
+++ b/media/jni/soundpool/SoundDecoder.h
@@ -30,21 +30,22 @@
 public:
     SoundDecoder(SoundManager* soundManager, size_t threads);
     ~SoundDecoder();
-    void loadSound(int32_t soundID);
+    void loadSound(int32_t soundID) NO_THREAD_SAFETY_ANALYSIS; // uses unique_lock
     void quit();
 
 private:
-    void run(int32_t id);                       // The decode thread function.
+    // The decode thread function.
+    void run(int32_t id) NO_THREAD_SAFETY_ANALYSIS; // uses unique_lock
 
     SoundManager* const     mSoundManager;      // set in constructor, has own lock
     std::unique_ptr<ThreadPool> mThreadPool;    // set in constructor, has own lock
 
     std::mutex              mLock;
-    std::condition_variable mQueueSpaceAvailable;
-    std::condition_variable mQueueDataAvailable;
+    std::condition_variable mQueueSpaceAvailable GUARDED_BY(mLock);
+    std::condition_variable mQueueDataAvailable GUARDED_BY(mLock);
 
-    std::deque<int32_t>     mSoundIDs;            // GUARDED_BY(mLock);
-    bool                    mQuit = false;        // GUARDED_BY(mLock);
+    std::deque<int32_t>     mSoundIDs GUARDED_BY(mLock);
+    bool                    mQuit GUARDED_BY(mLock) = false;
 };
 
 } // end namespace android::soundpool
diff --git a/media/jni/soundpool/SoundManager.cpp b/media/jni/soundpool/SoundManager.cpp
index 3c625bf..5b16174 100644
--- a/media/jni/soundpool/SoundManager.cpp
+++ b/media/jni/soundpool/SoundManager.cpp
@@ -43,7 +43,7 @@
     mSounds.clear();
 }
 
-int32_t SoundManager::load(int fd, int64_t offset, int64_t length, int32_t priority __unused)
+int32_t SoundManager::load(int fd, int64_t offset, int64_t length, int32_t priority)
 {
     ALOGV("%s(fd=%d, offset=%lld, length=%lld, priority=%d)",
             __func__, fd, (long long)offset, (long long)length, priority);
diff --git a/media/jni/soundpool/SoundManager.h b/media/jni/soundpool/SoundManager.h
index 9201e78..4a4e3b8 100644
--- a/media/jni/soundpool/SoundManager.h
+++ b/media/jni/soundpool/SoundManager.h
@@ -21,6 +21,8 @@
 #include <mutex>
 #include <unordered_map>
 
+#include <android-base/thread_annotations.h>
+
 namespace android {
 
 class SoundPool;
@@ -91,20 +93,21 @@
         }
     private:
         mutable std::recursive_mutex  mCallbackLock; // allow mCallback to setCallback().
+                                          // No thread-safety checks in R for recursive_mutex.
         SoundPool*          mSoundPool = nullptr; // GUARDED_BY(mCallbackLock)
         SoundPoolCallback*  mCallback = nullptr;  // GUARDED_BY(mCallbackLock)
         void*               mUserData = nullptr;  // GUARDED_BY(mCallbackLock)
     };
 
-    std::shared_ptr<Sound> findSound_l(int32_t soundID) const;
+    std::shared_ptr<Sound> findSound_l(int32_t soundID) const REQUIRES(mSoundManagerLock);
 
     // The following variables are initialized in constructor and can be accessed anytime.
-    CallbackHandler         mCallbackHandler;              // has its own lock
-    const std::unique_ptr<SoundDecoder> mDecoder;          // has its own lock
+    CallbackHandler mCallbackHandler;              // has its own lock
+    const std::unique_ptr<SoundDecoder> mDecoder;  // has its own lock
 
-    mutable std::mutex      mSoundManagerLock;
-    std::unordered_map<int, std::shared_ptr<Sound>> mSounds; // GUARDED_BY(mSoundManagerLock)
-    int32_t                 mNextSoundID = 0;    // GUARDED_BY(mSoundManagerLock)
+    mutable std::mutex mSoundManagerLock;
+    std::unordered_map<int, std::shared_ptr<Sound>> mSounds GUARDED_BY(mSoundManagerLock);
+    int32_t mNextSoundID GUARDED_BY(mSoundManagerLock) = 0;
 };
 
 } // namespace android::soundpool
diff --git a/media/jni/soundpool/Stream.cpp b/media/jni/soundpool/Stream.cpp
index e3152d6..a6d9758 100644
--- a/media/jni/soundpool/Stream.cpp
+++ b/media/jni/soundpool/Stream.cpp
@@ -105,7 +105,7 @@
     if (streamID == mStreamID) {
         mRate = rate;
         if (mAudioTrack != nullptr && mSound != nullptr) {
-            const uint32_t sampleRate = uint32_t(float(mSound->getSampleRate()) * rate + 0.5);
+            const auto sampleRate = (uint32_t)lround(double(mSound->getSampleRate()) * rate);
             mAudioTrack->setSampleRate(sampleRate);
         }
     }
@@ -214,8 +214,11 @@
 
 void Stream::clearAudioTrack()
 {
+    sp<AudioTrack> release;  // release outside of lock.
+    std::lock_guard lock(mLock);
     // This will invoke the destructor which waits for the AudioTrack thread to join,
     // and is currently the only safe way to ensure there are no callbacks afterwards.
+    release = mAudioTrack;  // or std::swap if we had move semantics.
     mAudioTrack.clear();
 }
 
@@ -288,7 +291,7 @@
         const audio_stream_type_t streamType =
                 AudioSystem::attributesToStreamType(*mStreamManager->getAttributes());
         const int32_t channelCount = sound->getChannelCount();
-        const uint32_t sampleRate = uint32_t(float(sound->getSampleRate()) * rate + 0.5);
+        const auto sampleRate = (uint32_t)lround(double(sound->getSampleRate()) * rate);
         size_t frameCount = 0;
 
         if (loop) {
@@ -307,7 +310,7 @@
                         __func__, mAudioTrack.get(), sound->getSoundID());
             }
         }
-        if (newTrack == 0) {
+        if (newTrack == nullptr) {
             // mToggle toggles each time a track is started on a given stream.
             // The toggle is concatenated with the Stream address and passed to AudioTrack
             // as callback user data. This enables the detection of callbacks received from the old
@@ -380,9 +383,9 @@
 /* static */
 void Stream::staticCallback(int event, void* user, void* info)
 {
-    const uintptr_t userAsInt = (uintptr_t)user;
-    Stream* stream = reinterpret_cast<Stream*>(userAsInt & ~1);
-    stream->callback(event, info, userAsInt & 1, 0 /* tries */);
+    const auto userAsInt = (uintptr_t)user;
+    auto stream = reinterpret_cast<Stream*>(userAsInt & ~1);
+    stream->callback(event, info, int(userAsInt & 1), 0 /* tries */);
 }
 
 void Stream::callback(int event, void* info, int toggle, int tries)
diff --git a/media/jni/soundpool/Stream.h b/media/jni/soundpool/Stream.h
index 82d2690..fd92921 100644
--- a/media/jni/soundpool/Stream.h
+++ b/media/jni/soundpool/Stream.h
@@ -18,6 +18,7 @@
 
 #include "Sound.h"
 
+#include <android-base/thread_annotations.h>
 #include <audio_utils/clock.h>
 #include <media/AudioTrack.h>
 
@@ -104,47 +105,55 @@
 
     // The following getters are not locked and have weak consistency.
     // These are considered advisory only - being stale is of nuisance.
-    int32_t getPriority() const { return mPriority; }
-    int32_t getPairPriority() const { return getPairStream()->getPriority(); }
-    int64_t getStopTimeNs() const { return mStopTimeNs; }
+    int32_t getPriority() const NO_THREAD_SAFETY_ANALYSIS { return mPriority; }
+    int32_t getPairPriority() const NO_THREAD_SAFETY_ANALYSIS {
+        return getPairStream()->getPriority();
+    }
+    int64_t getStopTimeNs() const NO_THREAD_SAFETY_ANALYSIS { return mStopTimeNs; }
 
-    int32_t getStreamID() const { return mStreamID; }  // Can change with setPlay()
-    int32_t getSoundID() const { return mSoundID; }    // Can change with play_l()
-    bool hasSound() const { return mSound.get() != nullptr; }
+    // Can change with setPlay()
+    int32_t getStreamID() const NO_THREAD_SAFETY_ANALYSIS { return mStreamID; }
 
-    Stream* getPairStream() const;  // this never changes.  See top of header.
+    // Can change with play_l()
+    int32_t getSoundID() const NO_THREAD_SAFETY_ANALYSIS { return mSoundID; }
+
+    bool hasSound() const NO_THREAD_SAFETY_ANALYSIS { return mSound.get() != nullptr; }
+
+    // This never changes.  See top of header.
+    Stream* getPairStream() const;
 
 private:
     void play_l(const std::shared_ptr<Sound>& sound, int streamID,
             float leftVolume, float rightVolume, int priority, int loop, float rate,
-            sp<AudioTrack> releaseTracks[2]);
-    void stop_l();
-    void setVolume_l(float leftVolume, float rightVolume);
+            sp<AudioTrack> releaseTracks[2]) REQUIRES(mLock);
+    void stop_l() REQUIRES(mLock);
+    void setVolume_l(float leftVolume, float rightVolume) REQUIRES(mLock);
 
     // For use with AudioTrack callback.
     static void staticCallback(int event, void* user, void* info);
-    void callback(int event, void* info, int toggle, int tries);
+    void callback(int event, void* info, int toggle, int tries)
+            NO_THREAD_SAFETY_ANALYSIS; // uses unique_lock
 
     // StreamManager should be set on construction and not changed.
     // release mLock before calling into StreamManager
     StreamManager*     mStreamManager = nullptr;
 
     mutable std::mutex  mLock;
-    std::atomic_int32_t mStreamID = 0;          // Note: valid streamIDs are always positive.
-    int                 mState = IDLE;
-    std::shared_ptr<Sound> mSound;              // Non-null if playing.
-    int32_t             mSoundID = 0;           // The sound ID associated with the AudioTrack.
-    float               mLeftVolume = 0.f;
-    float               mRightVolume = 0.f;
-    int32_t             mPriority = INT32_MIN;
-    int32_t             mLoop = 0;
-    float               mRate = 0.f;
-    bool                mAutoPaused = false;
-    bool                mMuted = false;
+    std::atomic_int32_t mStreamID GUARDED_BY(mLock) = 0; // Valid streamIDs are always positive.
+    int                 mState GUARDED_BY(mLock) = IDLE;
+    std::shared_ptr<Sound> mSound GUARDED_BY(mLock);    // Non-null if playing.
+    int32_t             mSoundID GUARDED_BY(mLock) = 0; // SoundID associated with AudioTrack.
+    float               mLeftVolume GUARDED_BY(mLock) = 0.f;
+    float               mRightVolume GUARDED_BY(mLock) = 0.f;
+    int32_t             mPriority GUARDED_BY(mLock) = INT32_MIN;
+    int32_t             mLoop GUARDED_BY(mLock) = 0;
+    float               mRate GUARDED_BY(mLock) = 0.f;
+    bool                mAutoPaused GUARDED_BY(mLock) = false;
+    bool                mMuted GUARDED_BY(mLock) = false;
 
-    sp<AudioTrack>      mAudioTrack;
-    int                 mToggle = 0;
-    int64_t             mStopTimeNs = 0;        // if nonzero, time to wait for stop.
+    sp<AudioTrack>      mAudioTrack GUARDED_BY(mLock);
+    int                 mToggle GUARDED_BY(mLock) = 0;
+    int64_t             mStopTimeNs GUARDED_BY(mLock) = 0;  // if nonzero, time to wait for stop.
 };
 
 } // namespace android::soundpool
diff --git a/media/jni/soundpool/StreamManager.cpp b/media/jni/soundpool/StreamManager.cpp
index c612218..9ff4254 100644
--- a/media/jni/soundpool/StreamManager.cpp
+++ b/media/jni/soundpool/StreamManager.cpp
@@ -55,7 +55,7 @@
         streams = 1;
     }
     mStreamPoolSize = streams * 2;
-    mStreamPool.reset(new Stream[mStreamPoolSize]);
+    mStreamPool = std::make_unique<Stream[]>(mStreamPoolSize); // create array of streams.
     // we use a perfect hash table with 2x size to map StreamIDs to Stream pointers.
     mPerfectHash = std::make_unique<PerfectHash<int32_t, Stream *>>(roundup(mStreamPoolSize * 2));
 }
@@ -69,7 +69,7 @@
 size_t StreamMap::streamPosition(const Stream* stream) const
 {
     ptrdiff_t index = stream - mStreamPool.get();
-    LOG_ALWAYS_FATAL_IF(index < 0 || index >= mStreamPoolSize,
+    LOG_ALWAYS_FATAL_IF(index < 0 || (size_t)index >= mStreamPoolSize,
             "%s: stream position out of range: %td", __func__, index);
     return (size_t)index;
 }
@@ -92,6 +92,11 @@
 
 ////////////
 
+// Thread safety analysis is supposed to be disabled for constructors and destructors
+// but clang in R seems to have a bug.  We use pragma to disable.
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wthread-safety-analysis"
+
 StreamManager::StreamManager(
         int32_t streams, size_t threads, const audio_attributes_t* attributes)
     : StreamMap(streams)
@@ -110,6 +115,8 @@
             "SoundPool_");
 }
 
+#pragma clang diagnostic pop
+
 StreamManager::~StreamManager()
 {
     ALOGV("%s", __func__);
diff --git a/media/jni/soundpool/StreamManager.h b/media/jni/soundpool/StreamManager.h
index 30ad220..59ae2f9 100644
--- a/media/jni/soundpool/StreamManager.h
+++ b/media/jni/soundpool/StreamManager.h
@@ -183,9 +183,9 @@
     std::atomic_size_t      mActiveThreadCount = 0;
 
     std::mutex              mThreadLock;
-    bool                    mQuit = false;           // GUARDED_BY(mThreadLock)
-    int32_t                 mNextThreadId = 0;       // GUARDED_BY(mThreadLock)
-    std::list<std::unique_ptr<JavaThread>> mThreads; // GUARDED_BY(mThreadLock)
+    bool                    mQuit GUARDED_BY(mThreadLock) = false;
+    int32_t                 mNextThreadId GUARDED_BY(mThreadLock) = 0;
+    std::list<std::unique_ptr<JavaThread>> mThreads GUARDED_BY(mThreadLock);
 };
 
 /**
@@ -263,7 +263,7 @@
     mutable std::mutex          mHashLock;
     const size_t                mHashCapacity; // size of mK2V no lock needed.
     std::unique_ptr<std::atomic<V>[]> mK2V;    // no lock needed for read access.
-    K                           mNextKey{};    // GUARDED_BY(mHashLock)
+    K                           mNextKey GUARDED_BY(mHashLock) {};
 };
 
 /**
@@ -392,7 +392,8 @@
     // Returns positive streamID on success, 0 on failure.  This is locked.
     int32_t queueForPlay(const std::shared_ptr<Sound> &sound,
             int32_t soundID, float leftVolume, float rightVolume,
-            int32_t priority, int32_t loop, float rate);
+            int32_t priority, int32_t loop, float rate)
+            NO_THREAD_SAFETY_ANALYSIS; // uses unique_lock
 
     ///////////////////////////////////////////////////////////////////////
     // Called from soundpool::Stream
@@ -407,11 +408,11 @@
 
 private:
 
-    void run(int32_t id);                        // worker thread, takes lock internally.
+    void run(int32_t id) NO_THREAD_SAFETY_ANALYSIS; // worker thread, takes unique_lock.
     void dump() const;                           // no lock needed
 
     // returns true if more worker threads are needed.
-    bool needMoreThreads_l() {
+    bool needMoreThreads_l() REQUIRES(mStreamManagerLock) {
         return mRestartStreams.size() > 0 &&
                 (mThreadPool->getActiveThreadCount() == 0
                 || std::distance(mRestartStreams.begin(),
@@ -420,14 +421,16 @@
     }
 
     // returns true if the stream was added.
-    bool moveToRestartQueue_l(Stream* stream, int32_t activeStreamIDToMatch = 0);
+    bool moveToRestartQueue_l(
+            Stream* stream, int32_t activeStreamIDToMatch = 0) REQUIRES(mStreamManagerLock);
     // returns number of queues the stream was removed from (should be 0 or 1);
     // a special code of -1 is returned if activeStreamIDToMatch is > 0 and
     // the stream wasn't found on the active queue.
-    ssize_t removeFromQueues_l(Stream* stream, int32_t activeStreamIDToMatch = 0);
-    void addToRestartQueue_l(Stream *stream);
-    void addToActiveQueue_l(Stream *stream);
-    void sanityCheckQueue_l() const;
+    ssize_t removeFromQueues_l(
+            Stream* stream, int32_t activeStreamIDToMatch = 0) REQUIRES(mStreamManagerLock);
+    void addToRestartQueue_l(Stream *stream) REQUIRES(mStreamManagerLock);
+    void addToActiveQueue_l(Stream *stream) REQUIRES(mStreamManagerLock);
+    void sanityCheckQueue_l() const REQUIRES(mStreamManagerLock);
 
     const audio_attributes_t mAttributes;
     std::unique_ptr<ThreadPool> mThreadPool;                  // locked internally
@@ -436,9 +439,9 @@
     // 4 stream queues by the Manager Thread or by the user initiated play().
     // A stream pair has exactly one stream on exactly one of the queues.
     std::mutex                  mStreamManagerLock;
-    std::condition_variable     mStreamManagerCondition;
+    std::condition_variable     mStreamManagerCondition GUARDED_BY(mStreamManagerLock);
 
-    bool                        mQuit = false;      // GUARDED_BY(mStreamManagerLock)
+    bool                        mQuit GUARDED_BY(mStreamManagerLock) = false;
 
     // There are constructor arg "streams" pairs of streams, only one of each
     // pair on the 4 stream queues below.  The other stream in the pair serves as
@@ -452,24 +455,24 @@
     // The paired stream may be active (but with no AudioTrack), and will be restarted
     // with an active AudioTrack when the current stream is stopped.
     std::multimap<int64_t /* stopTimeNs */, Stream*>
-                                mRestartStreams;    // GUARDED_BY(mStreamManagerLock)
+                                mRestartStreams GUARDED_BY(mStreamManagerLock);
 
     // 2) mActiveStreams: Streams that are active.
     // The paired stream will be inactive.
     // This is in order of specified by kStealActiveStream_OldestFirst
-    std::list<Stream*>          mActiveStreams;     // GUARDED_BY(mStreamManagerLock)
+    std::list<Stream*>          mActiveStreams GUARDED_BY(mStreamManagerLock);
 
     // 3) mAvailableStreams: Streams that are inactive.
     // The paired stream will also be inactive.
     // No particular order.
-    std::unordered_set<Stream*> mAvailableStreams;  // GUARDED_BY(mStreamManagerLock)
+    std::unordered_set<Stream*> mAvailableStreams GUARDED_BY(mStreamManagerLock);
 
     // 4) mProcessingStreams: Streams that are being processed by the ManagerThreads
     // When on this queue, the stream and its pair are not available for stealing.
     // Each ManagerThread will have at most one stream on the mProcessingStreams queue.
     // The paired stream may be active or restarting.
     // No particular order.
-    std::unordered_set<Stream*> mProcessingStreams; // GUARDED_BY(mStreamManagerLock)
+    std::unordered_set<Stream*> mProcessingStreams GUARDED_BY(mStreamManagerLock);
 };
 
 } // namespace android::soundpool
diff --git a/media/jni/soundpool/android_media_SoundPool.cpp b/media/jni/soundpool/android_media_SoundPool.cpp
index f670636..8f6df3d 100644
--- a/media/jni/soundpool/android_media_SoundPool.cpp
+++ b/media/jni/soundpool/android_media_SoundPool.cpp
@@ -52,7 +52,7 @@
 {
     ALOGV("android_media_SoundPool_load_FD");
     SoundPool *ap = MusterSoundPool(env, thiz);
-    if (ap == NULL) return 0;
+    if (ap == nullptr) return 0;
     return (jint) ap->load(jniGetFDFromFileDescriptor(env, fileDescriptor),
             int64_t(offset), int64_t(length), int(priority));
 }
@@ -61,7 +61,7 @@
 android_media_SoundPool_unload(JNIEnv *env, jobject thiz, jint sampleID) {
     ALOGV("android_media_SoundPool_unload\n");
     SoundPool *ap = MusterSoundPool(env, thiz);
-    if (ap == NULL) return JNI_FALSE;
+    if (ap == nullptr) return JNI_FALSE;
     return ap->unload(sampleID) ? JNI_TRUE : JNI_FALSE;
 }
 
@@ -72,7 +72,7 @@
 {
     ALOGV("android_media_SoundPool_play\n");
     SoundPool *ap = MusterSoundPool(env, thiz);
-    if (ap == NULL) return 0;
+    if (ap == nullptr) return 0;
     return (jint) ap->play(sampleID, leftVolume, rightVolume, priority, loop, rate);
 }
 
@@ -81,7 +81,7 @@
 {
     ALOGV("android_media_SoundPool_pause");
     SoundPool *ap = MusterSoundPool(env, thiz);
-    if (ap == NULL) return;
+    if (ap == nullptr) return;
     ap->pause(channelID);
 }
 
@@ -90,7 +90,7 @@
 {
     ALOGV("android_media_SoundPool_resume");
     SoundPool *ap = MusterSoundPool(env, thiz);
-    if (ap == NULL) return;
+    if (ap == nullptr) return;
     ap->resume(channelID);
 }
 
@@ -99,7 +99,7 @@
 {
     ALOGV("android_media_SoundPool_autoPause");
     SoundPool *ap = MusterSoundPool(env, thiz);
-    if (ap == NULL) return;
+    if (ap == nullptr) return;
     ap->autoPause();
 }
 
@@ -108,7 +108,7 @@
 {
     ALOGV("android_media_SoundPool_autoResume");
     SoundPool *ap = MusterSoundPool(env, thiz);
-    if (ap == NULL) return;
+    if (ap == nullptr) return;
     ap->autoResume();
 }
 
@@ -117,7 +117,7 @@
 {
     ALOGV("android_media_SoundPool_stop");
     SoundPool *ap = MusterSoundPool(env, thiz);
-    if (ap == NULL) return;
+    if (ap == nullptr) return;
     ap->stop(channelID);
 }
 
@@ -127,7 +127,7 @@
 {
     ALOGV("android_media_SoundPool_setVolume");
     SoundPool *ap = MusterSoundPool(env, thiz);
-    if (ap == NULL) return;
+    if (ap == nullptr) return;
     ap->setVolume(channelID, (float) leftVolume, (float) rightVolume);
 }
 
@@ -136,7 +136,7 @@
 {
     ALOGV("android_media_SoundPool_mute(%d)", muting);
     SoundPool *ap = MusterSoundPool(env, thiz);
-    if (ap == NULL) return;
+    if (ap == nullptr) return;
     ap->mute(muting == JNI_TRUE);
 }
 
@@ -146,7 +146,7 @@
 {
     ALOGV("android_media_SoundPool_setPriority");
     SoundPool *ap = MusterSoundPool(env, thiz);
-    if (ap == NULL) return;
+    if (ap == nullptr) return;
     ap->setPriority(channelID, (int) priority);
 }
 
@@ -156,7 +156,7 @@
 {
     ALOGV("android_media_SoundPool_setLoop");
     SoundPool *ap = MusterSoundPool(env, thiz);
-    if (ap == NULL) return;
+    if (ap == nullptr) return;
     ap->setLoop(channelID, loop);
 }
 
@@ -166,7 +166,7 @@
 {
     ALOGV("android_media_SoundPool_setRate");
     SoundPool *ap = MusterSoundPool(env, thiz);
-    if (ap == NULL) return;
+    if (ap == nullptr) return;
     ap->setRate(channelID, (float) rate);
 }
 
@@ -174,24 +174,26 @@
 {
     ALOGV("callback: (%d, %d, %d, %p, %p)", event.mMsg, event.mArg1, event.mArg2, soundPool, user);
     JNIEnv *env = AndroidRuntime::getJNIEnv();
-    env->CallStaticVoidMethod(fields.mSoundPoolClass, fields.mPostEvent, user, event.mMsg, event.mArg1, event.mArg2, NULL);
+    env->CallStaticVoidMethod(
+            fields.mSoundPoolClass, fields.mPostEvent, user, event.mMsg, event.mArg1, event.mArg2,
+            nullptr /* object */);
 }
 
 static jint
 android_media_SoundPool_native_setup(JNIEnv *env, jobject thiz, jobject weakRef,
         jint maxChannels, jobject jaa)
 {
-    if (jaa == 0) {
+    if (jaa == nullptr) {
         ALOGE("Error creating SoundPool: invalid audio attributes");
         return -1;
     }
 
-    audio_attributes_t *paa = NULL;
+    audio_attributes_t *paa = nullptr;
     // read the AudioAttributes values
     paa = (audio_attributes_t *) calloc(1, sizeof(audio_attributes_t));
-    const jstring jtags =
+    const auto jtags =
             (jstring) env->GetObjectField(jaa, javaAudioAttrFields.fieldFormattedTags);
-    const char* tags = env->GetStringUTFChars(jtags, NULL);
+    const char* tags = env->GetStringUTFChars(jtags, nullptr);
     // copying array size -1, char array for tags was calloc'd, no need to NULL-terminate it
     strncpy(paa->tags, tags, AUDIO_ATTRIBUTES_TAGS_MAX_SIZE - 1);
     env->ReleaseStringUTFChars(jtags, tags);
@@ -201,8 +203,8 @@
     paa->flags = env->GetIntField(jaa, javaAudioAttrFields.fieldFlags);
 
     ALOGV("android_media_SoundPool_native_setup");
-    SoundPool *ap = new SoundPool(maxChannels, paa);
-    if (ap == NULL) {
+    auto *ap = new SoundPool(maxChannels, paa);
+    if (ap == nullptr) {
         return -1;
     }
 
@@ -224,12 +226,12 @@
 {
     ALOGV("android_media_SoundPool_release");
     SoundPool *ap = MusterSoundPool(env, thiz);
-    if (ap != NULL) {
+    if (ap != nullptr) {
 
         // release weak reference and clear callback
-        jobject weakRef = (jobject) ap->getUserData();
-        ap->setCallback(NULL, NULL);
-        if (weakRef != NULL) {
+        auto weakRef = (jobject) ap->getUserData();
+        ap->setCallback(nullptr /* callback */, nullptr /* user */);
+        if (weakRef != nullptr) {
             env->DeleteGlobalRef(weakRef);
         }
 
@@ -309,7 +311,7 @@
 
 jint JNI_OnLoad(JavaVM* vm, void* /* reserved */)
 {
-    JNIEnv* env = NULL;
+    JNIEnv* env = nullptr;
     jint result = -1;
     jclass clazz;
 
@@ -317,23 +319,23 @@
         ALOGE("ERROR: GetEnv failed\n");
         return result;
     }
-    assert(env != NULL);
+    assert(env != nullptr);
 
     clazz = env->FindClass(kClassPathName);
-    if (clazz == NULL) {
+    if (clazz == nullptr) {
         ALOGE("Can't find %s", kClassPathName);
         return result;
     }
 
     fields.mNativeContext = env->GetFieldID(clazz, "mNativeContext", "J");
-    if (fields.mNativeContext == NULL) {
+    if (fields.mNativeContext == nullptr) {
         ALOGE("Can't find SoundPool.mNativeContext");
         return result;
     }
 
     fields.mPostEvent = env->GetStaticMethodID(clazz, "postEventFromNative",
                                                "(Ljava/lang/Object;IIILjava/lang/Object;)V");
-    if (fields.mPostEvent == NULL) {
+    if (fields.mPostEvent == nullptr) {
         ALOGE("Can't find android/media/SoundPool.postEventFromNative");
         return result;
     }
@@ -342,16 +344,18 @@
     // since it's a static object.
     fields.mSoundPoolClass = (jclass) env->NewGlobalRef(clazz);
 
-    if (AndroidRuntime::registerNativeMethods(env, kClassPathName, gMethods, NELEM(gMethods)) < 0)
+    if (AndroidRuntime::registerNativeMethods(
+                env, kClassPathName, gMethods, NELEM(gMethods)) < 0) {
         return result;
+    }
 
     // Get the AudioAttributes class and fields
     jclass audioAttrClass = env->FindClass(kAudioAttributesClassPathName);
-    if (audioAttrClass == NULL) {
+    if (audioAttrClass == nullptr) {
         ALOGE("Can't find %s", kAudioAttributesClassPathName);
         return result;
     }
-    jclass audioAttributesClassRef = (jclass)env->NewGlobalRef(audioAttrClass);
+    auto audioAttributesClassRef = (jclass)env->NewGlobalRef(audioAttrClass);
     javaAudioAttrFields.fieldUsage = env->GetFieldID(audioAttributesClassRef, "mUsage", "I");
     javaAudioAttrFields.fieldContentType
                                    = env->GetFieldID(audioAttributesClassRef, "mContentType", "I");
@@ -359,9 +363,10 @@
     javaAudioAttrFields.fieldFormattedTags =
             env->GetFieldID(audioAttributesClassRef, "mFormattedTags", "Ljava/lang/String;");
     env->DeleteGlobalRef(audioAttributesClassRef);
-    if (javaAudioAttrFields.fieldUsage == NULL || javaAudioAttrFields.fieldContentType == NULL
-            || javaAudioAttrFields.fieldFlags == NULL
-            || javaAudioAttrFields.fieldFormattedTags == NULL) {
+    if (javaAudioAttrFields.fieldUsage == nullptr
+            || javaAudioAttrFields.fieldContentType == nullptr
+            || javaAudioAttrFields.fieldFlags == nullptr
+            || javaAudioAttrFields.fieldFormattedTags == nullptr) {
         ALOGE("Can't initialize AudioAttributes fields");
         return result;
     }