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

Commit 1bc088a9 authored by Andy Hung's avatar Andy Hung
Browse files

Call AudioMixer only from MixerThread threadLoop. 

As part of change:
Remove track name offset by TRACK0.
Move track name management to the Tracks class.
Sync mixer track name to FastMixer track index.

Fixes regression introduced by commit 8ed196ac.

Test: SoundPool, AudioTrack CTS, Usability
Bug: 72937362
Bug: 73004420
Change-Id: I2f1a33f6f0da66bcd7aa91e2a4b663ff822df645
parent e54461ae

media/libaudioclient/include/media/AudioMixer.h

+39 −42
Original line number Diff line number Diff line
@@ -46,10 +46,6 @@ namespace android { 
class AudioMixer

{

public:

    // This mixer has a hard-coded upper limit of active track inputs;

    // the value is arbitrary but should be less than TRACK0 to avoid confusion.

    static constexpr int32_t MAX_NUM_TRACKS = 256;



    // Do not change these unless underlying code changes.

    // This mixer has a hard-coded upper limit of 8 channels for output.

    static constexpr uint32_t MAX_NUM_CHANNELS = FCC_8;
@@ -61,12 +57,6 @@ public: 
    static const CONSTEXPR float UNITY_GAIN_FLOAT = 1.0f;



    enum { // names



        // track names (MAX_NUM_TRACKS units)

        TRACK0          = 0x1000,



        // 0x2000 is unused



        // setParameter targets

        TRACK           = 0x3000,

        RESAMPLE        = 0x3001,
@@ -105,23 +95,33 @@ public: 
                                  // parameter 'value' is a pointer to the new playback rate.

    };



    AudioMixer(size_t frameCount, uint32_t sampleRate, int32_t maxNumTracks = MAX_NUM_TRACKS)

        : mMaxNumTracks(maxNumTracks)

        , mSampleRate(sampleRate)

    AudioMixer(size_t frameCount, uint32_t sampleRate)

        : mSampleRate(sampleRate)

        , mFrameCount(frameCount) {

        pthread_once(&sOnceControl, &sInitRoutine);

    }



    // For all APIs with "name": TRACK0 <= name < TRACK0 + MAX_NUM_TRACKS



    // Allocate a track name.  Returns new track name if successful, -1 on failure.

    // The failure could be because of an invalid channelMask or format, or that

    // the track capacity of the mixer is exceeded.

    int         getTrackName(audio_channel_mask_t channelMask,

                             audio_format_t format, int sessionId);

    // Create a new track in the mixer.

    //

    // \param name        a unique user-provided integer associated with the track.

    //                    If name already exists, the function will abort.

    // \param channelMask output channel mask.

    // \param format      PCM format

    // \param sessionId   Session id for the track. Tracks with the same

    //                    session id will be submixed together.

    //

    // \return OK        on success.

    //         BAD_VALUE if the format does not satisfy isValidFormat()

    //                   or the channelMask does not satisfy isValidChannelMask().

    status_t    create(

            int name, audio_channel_mask_t channelMask, audio_format_t format, int sessionId);



    bool        exists(int name) const {

        return mTracks.count(name) > 0;

    }



    // Free an allocated track by name

    void        deleteTrackName(int name);

    // Free an allocated track by name.

    void        destroy(int name);



    // Enable or disable an allocated track by name

    void        enable(int name);
@@ -149,6 +149,23 @@ public: 
        mNBLogWriter = logWriter;

    }



    static inline bool isValidFormat(audio_format_t format) {

        switch (format) {

        case AUDIO_FORMAT_PCM_8_BIT:

        case AUDIO_FORMAT_PCM_16_BIT:

        case AUDIO_FORMAT_PCM_24_BIT_PACKED:

        case AUDIO_FORMAT_PCM_32_BIT:

        case AUDIO_FORMAT_PCM_FLOAT:

            return true;

        default:

            return false;

        }

    }



    static inline bool isValidChannelMask(audio_channel_mask_t channelMask) {

        return audio_channel_mask_is_valid(channelMask); // the RemixBufferProvider is flexible.

    }



private:



    /* For multi-format functions (calls template functions
@@ -361,23 +378,9 @@ private: 
    static void convertMixerFormat(void *out, audio_format_t mixerOutFormat,

            void *in, audio_format_t mixerInFormat, size_t sampleCount);



    static inline bool isValidPcmTrackFormat(audio_format_t format) {

        switch (format) {

        case AUDIO_FORMAT_PCM_8_BIT:

        case AUDIO_FORMAT_PCM_16_BIT:

        case AUDIO_FORMAT_PCM_24_BIT_PACKED:

        case AUDIO_FORMAT_PCM_32_BIT:

        case AUDIO_FORMAT_PCM_FLOAT:

            return true;

        default:

            return false;

        }

    }



    static void sInitRoutine();



    // initialization constants

    const int mMaxNumTracks;

    const uint32_t mSampleRate;

    const size_t mFrameCount;
@@ -390,12 +393,6 @@ private: 
    std::unique_ptr<int32_t[]> mOutputTemp;

    std::unique_ptr<int32_t[]> mResampleTemp;



    // fast lookup of previously deleted track names for reuse.

    // the AudioMixer tries to return the smallest unused name -

    // this is an arbitrary decision (actually any non-negative

    // integer that isn't in mTracks could be used).

    std::set<int /* name */> mUnusedNames;    // set of unused track names (may be empty)



    // track names grouped by main buffer, in no particular order of main buffer.

    // however names for a particular main buffer are in order (by construction).

    std::unordered_map<void * /* mainBuffer */, std::vector<int /* name */>> mGroups;

media/libaudioprocessing/AudioMixer.cpp

+21 −38
Original line number Diff line number Diff line
@@ -90,34 +90,21 @@ static inline audio_format_t selectMixerInFormat(audio_format_t inputFormat __un 
    return kUseFloat && kUseNewMixer ? AUDIO_FORMAT_PCM_FLOAT : AUDIO_FORMAT_PCM_16_BIT;

}



int AudioMixer::getTrackName(

        audio_channel_mask_t channelMask, audio_format_t format, int sessionId)

status_t AudioMixer::create(

        int name, audio_channel_mask_t channelMask, audio_format_t format, int sessionId)

{

    if (!isValidPcmTrackFormat(format)) {

        ALOGE("AudioMixer::getTrackName invalid format (%#x)", format);

        return -1;

    }

    if (mTracks.size() >= (size_t)mMaxNumTracks) {

        ALOGE("%s: out of track names (max = %d)", __func__, mMaxNumTracks);

        return -1;

    }



    // get a new name for the track.

    int name;

    if (mUnusedNames.size() != 0) {

        // reuse first name for deleted track.

        auto it = mUnusedNames.begin();

        name = *it;

        (void)mUnusedNames.erase(it);

    } else {

        // we're fully populated, so create a new name.

        name = mTracks.size();

    LOG_ALWAYS_FATAL_IF(exists(name), "name %d already exists", name);



    if (!isValidChannelMask(channelMask)) {

        ALOGE("%s invalid channelMask: %#x", __func__, channelMask);

        return BAD_VALUE;

    }

    if (!isValidFormat(format)) {

        ALOGE("%s invalid format: %#x", __func__, format);

        return BAD_VALUE;

    }

    ALOGV("add track (%d)", name);



    auto t = std::make_shared<Track>();

    mTracks[name] = t;



    {

        // TODO: move initialization to the Track constructor.

        // assume default parameters for the track, except where noted below
@@ -179,12 +166,14 @@ int AudioMixer::getTrackName( 
        status_t status = t->prepareForDownmix();

        if (status != OK) {

            ALOGE("AudioMixer::getTrackName invalid channelMask (%#x)", channelMask);

            return -1;

            return BAD_VALUE;

        }

        // prepareForDownmix() may change mDownmixRequiresFormat

        ALOGVV("mMixerFormat:%#x  mMixerInFormat:%#x\n", t->mMixerFormat, t->mMixerInFormat);

        t->prepareForReformat();

        return TRACK0 + name;



        mTracks[name] = t;

        return OK;

    }

}
@@ -193,7 +182,7 @@ int AudioMixer::getTrackName( 
// which will simplify this logic.

bool AudioMixer::setChannelMasks(int name,

        audio_channel_mask_t trackChannelMask, audio_channel_mask_t mixerChannelMask) {

    LOG_ALWAYS_FATAL_IF(mTracks.find(name) == mTracks.end(), "invalid name: %d", name);

    LOG_ALWAYS_FATAL_IF(!exists(name), "invalid name: %d", name);

    const std::shared_ptr<Track> &track = mTracks[name];



    if (trackChannelMask == track->channelMask
@@ -361,23 +350,20 @@ void AudioMixer::Track::reconfigureBufferProviders() 
    }

}



void AudioMixer::deleteTrackName(int name)

void AudioMixer::destroy(int name)

{

    name -= TRACK0;

    LOG_ALWAYS_FATAL_IF(mTracks.find(name) == mTracks.end(), "invalid name: %d", name);

    LOG_ALWAYS_FATAL_IF(!exists(name), "invalid name: %d", name);

    ALOGV("deleteTrackName(%d)", name);



    if (mTracks[name]->enabled) {

        invalidate();

    }

    mTracks.erase(name); // deallocate track

    mUnusedNames.emplace(name); // recycle name

}



void AudioMixer::enable(int name)

{

    name -= TRACK0;

    LOG_ALWAYS_FATAL_IF(mTracks.find(name) == mTracks.end(), "invalid name: %d", name);

    LOG_ALWAYS_FATAL_IF(!exists(name), "invalid name: %d", name);

    const std::shared_ptr<Track> &track = mTracks[name];



    if (!track->enabled) {
@@ -389,8 +375,7 @@ void AudioMixer::enable(int name) 


void AudioMixer::disable(int name)

{

    name -= TRACK0;

    LOG_ALWAYS_FATAL_IF(mTracks.find(name) == mTracks.end(), "invalid name: %d", name);

    LOG_ALWAYS_FATAL_IF(!exists(name), "invalid name: %d", name);

    const std::shared_ptr<Track> &track = mTracks[name];



    if (track->enabled) {
@@ -528,8 +513,7 @@ static inline bool setVolumeRampVariables(float newVolume, int32_t ramp, 


void AudioMixer::setParameter(int name, int target, int param, void *value)

{

    name -= TRACK0;

    LOG_ALWAYS_FATAL_IF(mTracks.find(name) == mTracks.end(), "invalid name: %d", name);

    LOG_ALWAYS_FATAL_IF(!exists(name), "invalid name: %d", name);

    const std::shared_ptr<Track> &track = mTracks[name];



    int valueInt = static_cast<int>(reinterpret_cast<uintptr_t>(value));
@@ -808,7 +792,6 @@ inline void AudioMixer::Track::adjustVolumeRamp(bool aux, bool useFloat) 


size_t AudioMixer::getUnreleasedFrames(int name) const

{

    name -= TRACK0;

    const auto it = mTracks.find(name);

    if (it != mTracks.end()) {

        return it->second->getUnreleasedFrames();
@@ -818,7 +801,7 @@ size_t AudioMixer::getUnreleasedFrames(int name) const 


void AudioMixer::setBufferProvider(int name, AudioBufferProvider* bufferProvider)

{

    name -= TRACK0;

    LOG_ALWAYS_FATAL_IF(!exists(name), "invalid name: %d", name);

    const std::shared_ptr<Track> &track = mTracks[name];



    if (track->mInputBufferProvider == bufferProvider) {

media/libaudioprocessing/tests/test-mixer.cpp

+8 −9
Original line number Diff line number Diff line
@@ -143,10 +143,6 @@ int main(int argc, char* argv[]) { 
        usage(progname);

        return EXIT_FAILURE;

    }

    if ((unsigned)argc > AudioMixer::MAX_NUM_TRACKS) {

        fprintf(stderr, "too many tracks: %d > %u", argc, AudioMixer::MAX_NUM_TRACKS);

        return EXIT_FAILURE;

    }



    size_t outputFrames = 0;
@@ -246,9 +242,10 @@ int main(int argc, char* argv[]) { 
    for (size_t i = 0; i < providers.size(); ++i) {

        //printf("track %d out of %d\n", i, providers.size());

        uint32_t channelMask = audio_channel_out_mask_from_count(providers[i].getNumChannels());

        int32_t name = mixer->getTrackName(channelMask,

                formats[i], AUDIO_SESSION_OUTPUT_MIX);

        ALOG_ASSERT(name >= 0);

        const int name = i;

        const status_t status = mixer->create(

                name, channelMask, formats[i], AUDIO_SESSION_OUTPUT_MIX);

        LOG_ALWAYS_FATAL_IF(status != OK);

        names[i] = name;

        mixer->setBufferProvider(name, &providers[i]);

        mixer->setParameter(name, AudioMixer::TRACK, AudioMixer::MAIN_BUFFER,
@@ -315,8 +312,10 @@ int main(int argc, char* argv[]) { 
    writeFile(outputFilename, outputAddr,

            outputSampleRate, outputChannels, outputFrames, useMixerFloat);

    if (auxFilename) {

        // Aux buffer is always in q4_27 format for now.

        memcpy_to_i16_from_q4_27((int16_t*)auxAddr, (const int32_t*)auxAddr, outputFrames);

        // Aux buffer is always in q4_27 format for O and earlier.

        // memcpy_to_i16_from_q4_27((int16_t*)auxAddr, (const int32_t*)auxAddr, outputFrames);

        // Aux buffer is always in float format for P.

        memcpy_to_i16_from_float((int16_t*)auxAddr, (const float*)auxAddr, outputFrames);

        writeFile(auxFilename, auxAddr, outputSampleRate, 1, outputFrames, false);

    }

services/audioflinger/FastMixer.cpp

+13 −13
Original line number Diff line number Diff line
@@ -79,7 +79,6 @@ FastMixer::FastMixer() : FastThread("cycle_ms", "load_us"), 


    unsigned i;

    for (i = 0; i < FastMixerState::sMaxFastTracks; ++i) {

        mFastTrackNames[i] = -1;

        mGenerations[i] = 0;

    }

#ifdef FAST_THREAD_STATISTICS
@@ -190,7 +189,7 @@ void FastMixer::onStateChange() 
            // FIXME new may block for unbounded time at internal mutex of the heap

            //       implementation; it would be better to have normal mixer allocate for us

            //       to avoid blocking here and to prevent possible priority inversion

            mMixer = new AudioMixer(frameCount, mSampleRate, FastMixerState::sMaxFastTracks);

            mMixer = new AudioMixer(frameCount, mSampleRate);

            // FIXME See the other FIXME at FastMixer::setNBLogWriter()

            const size_t mixerFrameSize = mSinkChannelCount

                    * audio_bytes_per_sample(mMixerBufferFormat);
@@ -235,7 +234,6 @@ void FastMixer::onStateChange() 
    dumpState->mTrackMask = currentTrackMask;

    if (current->mFastTracksGen != mFastTracksGen) {

        ALOG_ASSERT(mMixerBuffer != NULL);

        int name;



        // process removed tracks first to avoid running out of track names

        unsigned removedTracks = previousTrackMask & ~currentTrackMask;
@@ -245,9 +243,7 @@ void FastMixer::onStateChange() 
            const FastTrack* fastTrack = &current->mFastTracks[i];

            ALOG_ASSERT(fastTrack->mBufferProvider == NULL);

            if (mMixer != NULL) {

                name = mFastTrackNames[i];

                ALOG_ASSERT(name >= 0);

                mMixer->deleteTrackName(name);

                mMixer->destroy(i);

            }

#if !LOG_NDEBUG

            mFastTrackNames[i] = -1;
@@ -265,10 +261,16 @@ void FastMixer::onStateChange() 
            AudioBufferProvider *bufferProvider = fastTrack->mBufferProvider;

            ALOG_ASSERT(bufferProvider != NULL && mFastTrackNames[i] == -1);

            if (mMixer != NULL) {

                name = mMixer->getTrackName(fastTrack->mChannelMask,

                const int name = i; // for clarity, choose name as fast track index.

                status_t status = mMixer->create(

                        name,

                        fastTrack->mChannelMask,

                        fastTrack->mFormat, AUDIO_SESSION_OUTPUT_MIX);

                ALOG_ASSERT(name >= 0);

                mFastTrackNames[i] = name;

                LOG_ALWAYS_FATAL_IF(status != NO_ERROR,

                        "%s: cannot create track name"

                        " %d, mask %#x, format %#x, sessionId %d in AudioMixer",

                        __func__, name,

                        fastTrack->mChannelMask, fastTrack->mFormat, AUDIO_SESSION_OUTPUT_MIX);

                mMixer->setBufferProvider(name, bufferProvider);

                mMixer->setParameter(name, AudioMixer::TRACK, AudioMixer::MAIN_BUFFER,

                        (void *)mMixerBuffer);
@@ -300,8 +302,7 @@ void FastMixer::onStateChange() 
                AudioBufferProvider *bufferProvider = fastTrack->mBufferProvider;

                ALOG_ASSERT(bufferProvider != NULL);

                if (mMixer != NULL) {

                    name = mFastTrackNames[i];

                    ALOG_ASSERT(name >= 0);

                    const int name = i;

                    mMixer->setBufferProvider(name, bufferProvider);

                    if (fastTrack->mVolumeProvider == NULL) {

                        float f = AudioMixer::UNITY_GAIN_FLOAT;
@@ -378,8 +379,7 @@ void FastMixer::onWork() 
            perTrackTimestamp.mPosition[ExtendedTimestamp::LOCATION_SERVER] = trackFramesWritten;

            fastTrack->mBufferProvider->onTimestamp(perTrackTimestamp);



            int name = mFastTrackNames[i];

            ALOG_ASSERT(name >= 0);

            const int name = i;

            if (fastTrack->mVolumeProvider != NULL) {

                gain_minifloat_packed_t vlr = fastTrack->mVolumeProvider->getVolumeLR();

                float vlf = float_from_gain(gain_minifloat_unpack_left(vlr));

services/audioflinger/FastMixer.h

+0 −2
Original line number Diff line number Diff line
@@ -57,8 +57,6 @@ private: 
    static const FastMixerState sInitial;



    FastMixerState  mPreIdle;   // copy of state before we went into idle

    int             mFastTrackNames[FastMixerState::kMaxFastTracks];

                                // handles used by mixer to identify tracks

    int             mGenerations[FastMixerState::kMaxFastTracks];

                                // last observed mFastTracks[i].mGeneration

    NBAIO_Sink*     mOutputSink;