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

Commit 41243dc2 authored by Treehugger Robot's avatar Treehugger Robot Committed by Gerrit Code Review
Browse files

Merge "audio: Ensure proper priority and scheduler for service threads" into main

parents 203ca985 4bf6899c
Loading
Loading
Loading
Loading
+3 −0
Original line number Diff line number Diff line
@@ -106,6 +106,9 @@ void ThreadController::workerThread() {
        std::lock_guard<std::mutex> lock(mWorkerLock);
        mWorkerState = error.empty() ? WorkerState::RUNNING : WorkerState::STOPPED;
        mError = error;
#if defined(__ANDROID__)
        mTid = pthread_gettid_np(pthread_self());
#endif
    }
    mWorkerCv.notify_one();
    if (!error.empty()) return;
+8 −0
Original line number Diff line number Diff line
@@ -16,6 +16,8 @@

#pragma once

#include <sys/types.h>

#include <atomic>
#include <condition_variable>
#include <mutex>
@@ -52,6 +54,10 @@ class ThreadController {
        std::lock_guard<std::mutex> lock(mWorkerLock);
        return mError;
    }
    pid_t getTid() {
        std::lock_guard<std::mutex> lock(mWorkerLock);
        return mTid;
    }
    void stop();
    // Direct use of 'join' assumes that the StreamLogic is not intended
    // to run forever, and is guaranteed to exit by itself. This normally
@@ -78,6 +84,7 @@ class ThreadController {
    std::condition_variable mWorkerCv;
    WorkerState mWorkerState GUARDED_BY(mWorkerLock) = WorkerState::INITIAL;
    std::string mError GUARDED_BY(mWorkerLock);
    pid_t mTid GUARDED_BY(mWorkerLock) = -1;
    // The atomic lock-free variable is used to prevent priority inversions
    // that can occur when a high priority worker tries to acquire the lock
    // which has been taken by a lower priority control thread which in its turn
@@ -143,6 +150,7 @@ class StreamWorker : public LogicImpl {
    void resume() { mThread.resume(); }
    bool hasError() { return mThread.hasError(); }
    std::string getError() { return mThread.getError(); }
    pid_t getTid() { return mThread.getTid(); }
    void stop() { mThread.stop(); }
    void join() { mThread.join(); }
    bool waitForAtLeastOneCycle() { return mThread.waitForAtLeastOneCycle(); }
+8 −0
Original line number Diff line number Diff line
@@ -87,6 +87,7 @@ class StreamWorkerInvalidTest : public testing::TestWithParam<bool> {
TEST_P(StreamWorkerInvalidTest, Uninitialized) {
    EXPECT_FALSE(worker.hasWorkerCycleCalled());
    EXPECT_FALSE(worker.hasError());
    EXPECT_LE(worker.getTid(), 0);
}

TEST_P(StreamWorkerInvalidTest, UninitializedPauseIgnored) {
@@ -105,6 +106,9 @@ TEST_P(StreamWorkerInvalidTest, Start) {
    EXPECT_FALSE(worker.start());
    EXPECT_FALSE(worker.hasWorkerCycleCalled());
    EXPECT_TRUE(worker.hasError());
#if defined(__ANDROID__)
    EXPECT_GT(worker.getTid(), 0);
#endif
}

TEST_P(StreamWorkerInvalidTest, PauseIgnored) {
@@ -136,12 +140,16 @@ static constexpr unsigned kWorkerIdleCheckTime = 50 * 1000;
TEST_P(StreamWorkerTest, Uninitialized) {
    EXPECT_FALSE(worker.hasWorkerCycleCalled());
    EXPECT_FALSE(worker.hasError());
    EXPECT_LE(worker.getTid(), 0);
}

TEST_P(StreamWorkerTest, Start) {
    ASSERT_TRUE(worker.start());
    EXPECT_TRUE(worker.waitForAtLeastOneCycle());
    EXPECT_FALSE(worker.hasError());
#if defined(__ANDROID__)
    EXPECT_GT(worker.getTid(), 0);
#endif
}

TEST_P(StreamWorkerTest, StartStop) {
+15 −4
Original line number Diff line number Diff line
@@ -479,14 +479,17 @@ std::unique_ptr<Configuration> getUsbConfiguration() {
// Mix ports:
//  * "test output", 1 max open, 1 max active stream
//    - profile PCM 24-bit; MONO, STEREO; 8000, 11025, 16000, 32000, 44100, 48000
//  * "compressed offload", DIRECT|COMPRESS_OFFLOAD|NON_BLOCKING, 1 max open, 1 max active stream
//  * "test fast output", 1 max open, 1 max active stream
//    - profile PCM 24-bit; STEREO; 44100, 48000
//  * "test compressed offload", DIRECT|COMPRESS_OFFLOAD|NON_BLOCKING, 1 max open, 1 max active
//  stream
//    - profile MP3; MONO, STEREO; 44100, 48000
//  * "test input", 2 max open, 2 max active streams
//    - profile PCM 24-bit; MONO, STEREO, FRONT_BACK;
//        8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000
//
// Routes:
//  "test output", "compressed offload" -> "Test Out"
//  "test output", "test fast output", "test compressed offload" -> "Test Out"
//  "Test In" -> "test input"
//
// Initial port configs:
@@ -525,8 +528,15 @@ std::unique_ptr<Configuration> getStubConfiguration() {
                              {8000, 11025, 16000, 32000, 44100, 48000}));
        c.ports.push_back(testOutMix);

        AudioPort testFastOutMix = createPort(c.nextPortId++, "test fast output",
                                              makeBitPositionFlagMask({AudioOutputFlags::FAST}),
                                              false, createPortMixExt(1, 1));
        testFastOutMix.profiles.push_back(createProfile(
                PcmType::INT_24_BIT, {AudioChannelLayout::LAYOUT_STEREO}, {44100, 48000}));
        c.ports.push_back(testFastOutMix);

        AudioPort compressedOffloadOutMix =
                createPort(c.nextPortId++, "compressed offload",
                createPort(c.nextPortId++, "test compressed offload",
                           makeBitPositionFlagMask({AudioOutputFlags::DIRECT,
                                                    AudioOutputFlags::COMPRESS_OFFLOAD,
                                                    AudioOutputFlags::NON_BLOCKING}),
@@ -551,7 +561,8 @@ std::unique_ptr<Configuration> getStubConfiguration() {
                              {8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000}));
        c.ports.push_back(testInMIx);

        c.routes.push_back(createRoute({testOutMix, compressedOffloadOutMix}, testOutDevice));
        c.routes.push_back(
                createRoute({testOutMix, testFastOutMix, compressedOffloadOutMix}, testOutDevice));
        c.routes.push_back(createRoute({testInDevice}, testInMIx));

        c.portConfigs.insert(c.portConfigs.end(), c.initialConfigs.begin(), c.initialConfigs.end());
+28 −2
Original line number Diff line number Diff line
@@ -27,12 +27,16 @@
using aidl::android::hardware::audio::common::AudioOffloadMetadata;
using aidl::android::hardware::audio::common::getChannelCount;
using aidl::android::hardware::audio::common::getFrameSizeInBytes;
using aidl::android::hardware::audio::common::isBitPositionFlagSet;
using aidl::android::hardware::audio::common::SinkMetadata;
using aidl::android::hardware::audio::common::SourceMetadata;
using aidl::android::media::audio::common::AudioDevice;
using aidl::android::media::audio::common::AudioDualMonoMode;
using aidl::android::media::audio::common::AudioInputFlags;
using aidl::android::media::audio::common::AudioIoFlags;
using aidl::android::media::audio::common::AudioLatencyMode;
using aidl::android::media::audio::common::AudioOffloadInfo;
using aidl::android::media::audio::common::AudioOutputFlags;
using aidl::android::media::audio::common::AudioPlaybackRate;
using aidl::android::media::audio::common::MicrophoneDynamicInfo;
using aidl::android::media::audio::common::MicrophoneInfo;
@@ -610,8 +614,30 @@ StreamCommonImpl::~StreamCommonImpl() {
ndk::ScopedAStatus StreamCommonImpl::initInstance(
        const std::shared_ptr<StreamCommonInterface>& delegate) {
    mCommon = ndk::SharedRefBase::make<StreamCommonDelegator>(delegate);
    return mWorker->start() ? ndk::ScopedAStatus::ok()
                            : ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE);
    if (!mWorker->start()) {
        return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE);
    }
    if (auto flags = getContext().getFlags();
        (flags.getTag() == AudioIoFlags::Tag::input &&
         isBitPositionFlagSet(flags.template get<AudioIoFlags::Tag::input>(),
                              AudioInputFlags::FAST)) ||
        (flags.getTag() == AudioIoFlags::Tag::output &&
         isBitPositionFlagSet(flags.template get<AudioIoFlags::Tag::output>(),
                              AudioOutputFlags::FAST))) {
        // FAST workers should be run with a SCHED_FIFO scheduler, however the host process
        // might be lacking the capability to request it, thus a failure to set is not an error.
        pid_t workerTid = mWorker->getTid();
        if (workerTid > 0) {
            struct sched_param param;
            param.sched_priority = 3;  // Must match SchedulingPolicyService.PRIORITY_MAX (Java).
            if (sched_setscheduler(workerTid, SCHED_FIFO | SCHED_RESET_ON_FORK, &param) != 0) {
                PLOG(WARNING) << __func__ << ": failed to set FIFO scheduler for a fast thread";
            }
        } else {
            LOG(WARNING) << __func__ << ": invalid worker tid: " << workerTid;
        }
    }
    return ndk::ScopedAStatus::ok();
}

ndk::ScopedAStatus StreamCommonImpl::getStreamCommonCommon(
Loading