From 77eb2bda0e5758b7c350db2991d6fc5806680b57 Mon Sep 17 00:00:00 2001 From: Andy Hung Date: Tue, 19 May 2020 10:42:09 -0700 Subject: [PATCH] SoundPool: Add clang-tidy and fix Test: soundpool_stress Test: SoundPoolAacTest Test: SoundPoolHapticTest Test: SoundPoolMidiTest Test: SoundPoolOggTest Bug: 157501605 Change-Id: I7bba857a8b44b6255d423086127da0e2df4ce1c4 --- media/jni/soundpool/Android.bp | 87 +++++++++++++++++++ media/jni/soundpool/Sound.cpp | 8 +- media/jni/soundpool/SoundDecoder.cpp | 2 +- media/jni/soundpool/SoundDecoder.h | 13 +-- media/jni/soundpool/SoundManager.cpp | 2 +- media/jni/soundpool/SoundManager.h | 15 ++-- media/jni/soundpool/Stream.cpp | 15 ++-- media/jni/soundpool/Stream.h | 59 +++++++------ media/jni/soundpool/StreamManager.cpp | 11 ++- media/jni/soundpool/StreamManager.h | 39 +++++---- .../jni/soundpool/android_media_SoundPool.cpp | 75 ++++++++-------- 11 files changed, 222 insertions(+), 104 deletions(-) diff --git a/media/jni/soundpool/Android.bp b/media/jni/soundpool/Android.bp index aa3279321e903..6141308a8fdbf 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 0bbc3e46b0440..c3abdc22b19de 100644 --- a/media/jni/soundpool/Sound.cpp +++ b/media/jni/soundpool/Sound.cpp @@ -31,7 +31,7 @@ constexpr size_t kDefaultHeapSize = 1024 * 1024; // 1MB (compatible with low m 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 @@ Sound::~Sound() 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 heap, + audio_channel_mask_t *channelMask, const sp& heap, size_t *sizeInBytes) { ALOGV("%s(fd=%d, offset=%lld, length=%lld, ...)", __func__, fd, (long long)offset, (long long)length); @@ -81,7 +81,7 @@ static status_t decode(int fd, int64_t offset, int64_t length, bool sawInputEOS = false; bool sawOutputEOS = false; - uint8_t* writePos = static_cast(heap->getBase()); + auto writePos = static_cast(heap->getBase()); size_t available = heap->getSize(); size_t written = 0; format.reset(AMediaCodec_getOutputFormat(codec.get())); // update format. @@ -204,7 +204,7 @@ status_t Sound::doLoad() 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 12200ef03aadc..6614fdb5af531 100644 --- a/media/jni/soundpool/SoundDecoder.cpp +++ b/media/jni/soundpool/SoundDecoder.cpp @@ -57,7 +57,7 @@ void SoundDecoder::quit() 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 1288943e86e3b..7b62114483cf8 100644 --- a/media/jni/soundpool/SoundDecoder.h +++ b/media/jni/soundpool/SoundDecoder.h @@ -30,21 +30,22 @@ class SoundDecoder { 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 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 mSoundIDs; // GUARDED_BY(mLock); - bool mQuit = false; // GUARDED_BY(mLock); + std::deque 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 3c625bf3bb7f5..5b16174eef2bc 100644 --- a/media/jni/soundpool/SoundManager.cpp +++ b/media/jni/soundpool/SoundManager.cpp @@ -43,7 +43,7 @@ SoundManager::~SoundManager() 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 9201e78132f43..4a4e3b87be268 100644 --- a/media/jni/soundpool/SoundManager.h +++ b/media/jni/soundpool/SoundManager.h @@ -21,6 +21,8 @@ #include #include +#include + namespace android { class SoundPool; @@ -91,20 +93,21 @@ private: } 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 findSound_l(int32_t soundID) const; + std::shared_ptr 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 mDecoder; // has its own lock + CallbackHandler mCallbackHandler; // has its own lock + const std::unique_ptr mDecoder; // has its own lock - mutable std::mutex mSoundManagerLock; - std::unordered_map> mSounds; // GUARDED_BY(mSoundManagerLock) - int32_t mNextSoundID = 0; // GUARDED_BY(mSoundManagerLock) + mutable std::mutex mSoundManagerLock; + std::unordered_map> 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 e3152d6349aa5..a6d975851619e 100644 --- a/media/jni/soundpool/Stream.cpp +++ b/media/jni/soundpool/Stream.cpp @@ -105,7 +105,7 @@ void Stream::setRate(int32_t streamID, float rate) 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::stop_l() void Stream::clearAudioTrack() { + sp 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 @@ void Stream::play_l(const std::shared_ptr& sound, int32_t nextStreamID, 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 @@ void Stream::play_l(const std::shared_ptr& sound, int32_t nextStreamID, __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 @@ exit: /* static */ void Stream::staticCallback(int event, void* user, void* info) { - const uintptr_t userAsInt = (uintptr_t)user; - Stream* stream = reinterpret_cast(userAsInt & ~1); - stream->callback(event, info, userAsInt & 1, 0 /* tries */); + const auto userAsInt = (uintptr_t)user; + auto stream = reinterpret_cast(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 82d2690e2965c..fd929210eb463 100644 --- a/media/jni/soundpool/Stream.h +++ b/media/jni/soundpool/Stream.h @@ -18,6 +18,7 @@ #include "Sound.h" +#include #include #include @@ -104,47 +105,55 @@ public: // 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, int streamID, float leftVolume, float rightVolume, int priority, int loop, float rate, - sp releaseTracks[2]); - void stop_l(); - void setVolume_l(float leftVolume, float rightVolume); + sp 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 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 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 mAudioTrack; - int mToggle = 0; - int64_t mStopTimeNs = 0; // if nonzero, time to wait for stop. + sp 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 c61221892c280..9ff4254284dc9 100644 --- a/media/jni/soundpool/StreamManager.cpp +++ b/media/jni/soundpool/StreamManager.cpp @@ -55,7 +55,7 @@ StreamMap::StreamMap(int32_t streams) { streams = 1; } mStreamPoolSize = streams * 2; - mStreamPool.reset(new Stream[mStreamPoolSize]); + mStreamPool = std::make_unique(mStreamPoolSize); // create array of streams. // we use a perfect hash table with 2x size to map StreamIDs to Stream pointers. mPerfectHash = std::make_unique>(roundup(mStreamPoolSize * 2)); } @@ -69,7 +69,7 @@ Stream* StreamMap::findStream(int32_t streamID) const 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 @@ int32_t StreamMap::getNextIdForStream(Stream* stream) const { //////////// +// 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 @@ StreamManager::StreamManager( "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 30ad220db05b2..59ae2f9d108b3 100644 --- a/media/jni/soundpool/StreamManager.h +++ b/media/jni/soundpool/StreamManager.h @@ -183,9 +183,9 @@ private: std::atomic_size_t mActiveThreadCount = 0; std::mutex mThreadLock; - bool mQuit = false; // GUARDED_BY(mThreadLock) - int32_t mNextThreadId = 0; // GUARDED_BY(mThreadLock) - std::list> mThreads; // GUARDED_BY(mThreadLock) + bool mQuit GUARDED_BY(mThreadLock) = false; + int32_t mNextThreadId GUARDED_BY(mThreadLock) = 0; + std::list> mThreads GUARDED_BY(mThreadLock); }; /** @@ -263,7 +263,7 @@ private: mutable std::mutex mHashLock; const size_t mHashCapacity; // size of mK2V no lock needed. std::unique_ptr[]> mK2V; // no lock needed for read access. - K mNextKey{}; // GUARDED_BY(mHashLock) + K mNextKey GUARDED_BY(mHashLock) {}; }; /** @@ -392,7 +392,8 @@ public: // Returns positive streamID on success, 0 on failure. This is locked. int32_t queueForPlay(const std::shared_ptr &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 @@ public: 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 @@ private: } // 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 mThreadPool; // locked internally @@ -436,9 +439,9 @@ private: // 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 @@ private: // 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 - 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 mActiveStreams; // GUARDED_BY(mStreamManagerLock) + std::list mActiveStreams GUARDED_BY(mStreamManagerLock); // 3) mAvailableStreams: Streams that are inactive. // The paired stream will also be inactive. // No particular order. - std::unordered_set mAvailableStreams; // GUARDED_BY(mStreamManagerLock) + std::unordered_set 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 mProcessingStreams; // GUARDED_BY(mStreamManagerLock) + std::unordered_set 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 f6706369f3799..8f6df3db718b0 100644 --- a/media/jni/soundpool/android_media_SoundPool.cpp +++ b/media/jni/soundpool/android_media_SoundPool.cpp @@ -52,7 +52,7 @@ android_media_SoundPool_load_FD(JNIEnv *env, jobject thiz, jobject fileDescripto { 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 @@ static jboolean 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 @@ android_media_SoundPool_play(JNIEnv *env, jobject thiz, jint sampleID, { 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 @@ android_media_SoundPool_pause(JNIEnv *env, jobject thiz, jint channelID) { 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 @@ android_media_SoundPool_resume(JNIEnv *env, jobject thiz, jint channelID) { 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 @@ android_media_SoundPool_autoPause(JNIEnv *env, jobject thiz) { ALOGV("android_media_SoundPool_autoPause"); SoundPool *ap = MusterSoundPool(env, thiz); - if (ap == NULL) return; + if (ap == nullptr) return; ap->autoPause(); } @@ -108,7 +108,7 @@ android_media_SoundPool_autoResume(JNIEnv *env, jobject thiz) { ALOGV("android_media_SoundPool_autoResume"); SoundPool *ap = MusterSoundPool(env, thiz); - if (ap == NULL) return; + if (ap == nullptr) return; ap->autoResume(); } @@ -117,7 +117,7 @@ android_media_SoundPool_stop(JNIEnv *env, jobject thiz, jint channelID) { 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 @@ android_media_SoundPool_setVolume(JNIEnv *env, jobject thiz, jint channelID, { 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 @@ android_media_SoundPool_mute(JNIEnv *env, jobject thiz, jboolean muting) { 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 @@ android_media_SoundPool_setPriority(JNIEnv *env, jobject thiz, jint channelID, { 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 @@ android_media_SoundPool_setLoop(JNIEnv *env, jobject thiz, jint channelID, { 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 @@ android_media_SoundPool_setRate(JNIEnv *env, jobject thiz, jint channelID, { 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 @@ static void android_media_callback(SoundPoolEvent event, SoundPool* soundPool, v { 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 @@ android_media_SoundPool_native_setup(JNIEnv *env, jobject thiz, jobject weakRef, 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 @@ android_media_SoundPool_release(JNIEnv *env, jobject thiz) { 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 @@ static const char* const kClassPathName = "android/media/SoundPool"; jint JNI_OnLoad(JavaVM* vm, void* /* reserved */) { - JNIEnv* env = NULL; + JNIEnv* env = nullptr; jint result = -1; jclass clazz; @@ -317,23 +319,23 @@ jint JNI_OnLoad(JavaVM* vm, void* /* reserved */) 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 @@ jint JNI_OnLoad(JavaVM* vm, void* /* reserved */) // 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 @@ jint JNI_OnLoad(JavaVM* vm, void* /* reserved */) 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; }