Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 77eb2bda authored by Andy Hung's avatar Andy Hung
Browse files

SoundPool: Add clang-tidy and fix 

Test: soundpool_stress
Test: SoundPoolAacTest
Test: SoundPoolHapticTest
Test: SoundPoolMidiTest
Test: SoundPoolOggTest
Bug: 157501605
Change-Id: I7bba857a8b44b6255d423086127da0e2df4ce1c4
parent 71576c05

media/jni/soundpool/Android.bp

+87 −0
Original line number Diff line number Diff line 
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",

media/jni/soundpool/Sound.cpp

+4 −4
Original line number Diff line number Diff line
@@ -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<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 @@ static status_t decode(int fd, int64_t offset, int64_t length, 


            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 @@ 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

media/jni/soundpool/SoundDecoder.cpp

+1 −1
Original line number Diff line number Diff line
@@ -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);

media/jni/soundpool/SoundDecoder.h

+7 −6
Original line number Diff line number Diff line
@@ -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<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

media/jni/soundpool/SoundManager.cpp

+1 −1
Original line number Diff line number Diff line
@@ -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);