diff --git a/media/libaudioclient/include/media/AudioMixer.h b/media/libaudioclient/include/media/AudioMixer.h index 2e29316043..cf7d90fe71 100644 --- a/media/libaudioclient/include/media/AudioMixer.h +++ b/media/libaudioclient/include/media/AudioMixer.h @@ -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 mOutputTemp; std::unique_ptr 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 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> mGroups; diff --git a/media/libaudioprocessing/AudioMixer.cpp b/media/libaudioprocessing/AudioMixer.cpp index f1daeb4a4e..2042913d92 100644 --- a/media/libaudioprocessing/AudioMixer.cpp +++ b/media/libaudioprocessing/AudioMixer.cpp @@ -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; - } + LOG_ALWAYS_FATAL_IF(exists(name), "name %d already exists", name); - // 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(); + 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(); - 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 = 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 = 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 = 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 = mTracks[name]; int valueInt = static_cast(reinterpret_cast(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 = mTracks[name]; if (track->mInputBufferProvider == bufferProvider) { diff --git a/media/libaudioprocessing/tests/test-mixer.cpp b/media/libaudioprocessing/tests/test-mixer.cpp index b67810de3c..bc9d2a69c7 100644 --- a/media/libaudioprocessing/tests/test-mixer.cpp +++ b/media/libaudioprocessing/tests/test-mixer.cpp @@ -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); } diff --git a/services/audioflinger/FastMixer.cpp b/services/audioflinger/FastMixer.cpp index ace586c1a0..ef466a26dd 100644 --- a/services/audioflinger/FastMixer.cpp +++ b/services/audioflinger/FastMixer.cpp @@ -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 = ¤t->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)); diff --git a/services/audioflinger/FastMixer.h b/services/audioflinger/FastMixer.h index 930fa8d8cc..235d23faf9 100644 --- a/services/audioflinger/FastMixer.h +++ b/services/audioflinger/FastMixer.h @@ -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; diff --git a/services/audioflinger/PatchPanel.cpp b/services/audioflinger/PatchPanel.cpp index 27c6d35816..e5cb8a2138 100644 --- a/services/audioflinger/PatchPanel.cpp +++ b/services/audioflinger/PatchPanel.cpp @@ -497,9 +497,6 @@ status_t AudioFlinger::PatchPanel::createPatchConnections(Patch *patch, patch->mPatchRecord->buffer(), patch->mPatchRecord->bufferSize(), AUDIO_OUTPUT_FLAG_NONE); - if (patch->mPatchTrack == 0) { - return NO_MEMORY; - } status = patch->mPatchTrack->initCheck(); if (status != NO_ERROR) { return status; diff --git a/services/audioflinger/PlaybackTracks.h b/services/audioflinger/PlaybackTracks.h index e97bb061aa..6454be57ec 100644 --- a/services/audioflinger/PlaybackTracks.h +++ b/services/audioflinger/PlaybackTracks.h @@ -51,6 +51,11 @@ public: void flush(); void destroy(); int name() const { return mName; } + void setName(int name) { + LOG_ALWAYS_FATAL_IF(mName >= 0 && name >= 0, + "%s both old name %d and new name %d are valid", __func__, mName, name); + mName = name; + } virtual uint32_t sampleRate() const; @@ -146,10 +151,7 @@ protected: bool mResetDone; const audio_stream_type_t mStreamType; - int mName; // track name on the normal mixer, - // allocated statically at track creation time, - // and is even allocated (though unused) for fast tracks - // FIXME don't allocate track name for fast tracks + int mName; effect_buffer_t *mMainBuffer; int32_t *mAuxBuffer; diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp index d6021b369f..cae296ea15 100644 --- a/services/audioflinger/Threads.cpp +++ b/services/audioflinger/Threads.cpp @@ -1661,6 +1661,7 @@ AudioFlinger::PlaybackThread::PlaybackThread(const sp& audioFlinge mSuspendedFrames(0), mActiveTracks(&this->mLocalLog), // mStreamTypes[] initialized in constructor body + mTracks(type == MIXER), mOutput(output), mLastWriteTime(-1), mNumWrites(0), mNumDelayedWrites(0), mInWrite(false), mMixerStatus(MIXER_IDLE), @@ -2157,6 +2158,53 @@ Exit: return track; } +template +ssize_t AudioFlinger::PlaybackThread::Tracks::add(const sp &track) +{ + const ssize_t index = mTracks.add(track); + if (index >= 0) { + // set name for track when adding. + int name; + if (mUnusedTrackNames.empty()) { + name = mTracks.size() - 1; // new name {0 ... size-1}. + } else { + // reuse smallest name for deleted track. + auto it = mUnusedTrackNames.begin(); + name = *it; + (void)mUnusedTrackNames.erase(it); + } + track->setName(name); + } else { + LOG_ALWAYS_FATAL("cannot add track"); + } + return index; +} + +template +ssize_t AudioFlinger::PlaybackThread::Tracks::remove(const sp &track) +{ + const int name = track->name(); + const ssize_t index = mTracks.remove(track); + if (index >= 0) { + // invalidate name when removing from mTracks. + LOG_ALWAYS_FATAL_IF(name < 0, "invalid name %d for track on mTracks", name); + + if (mSaveDeletedTrackNames) { + // We can't directly access mAudioMixer since the caller may be outside of threadLoop. + // Instead, we add to mDeletedTrackNames which is solely used for mAudioMixer update, + // to be handled when MixerThread::prepareTracks_l() next changes mAudioMixer. + mDeletedTrackNames.emplace(name); + } + + mUnusedTrackNames.emplace(name); + track->setName(T::TRACK_NAME_PENDING); + } else { + LOG_ALWAYS_FATAL_IF(name >= 0, + "valid name %d for track not in mTracks (returned %zd)", name, index); + } + return index; +} + uint32_t AudioFlinger::PlaybackThread::correctLatency_l(uint32_t latency) const { return latency; @@ -2313,9 +2361,6 @@ void AudioFlinger::PlaybackThread::removeTrack_l(const sp& track) mLocalLog.log("removeTrack_l (%p) %s", track.get(), result.string()); mTracks.remove(track); - deleteTrackName_l(track->name()); - // redundant as track is about to be destroyed, for dumpsys only - track->mName = -1; if (track->isFastTrack()) { int index = track->mFastIndex; ALOG_ASSERT(0 < index && index < (int)FastMixerState::sMaxFastTracks); @@ -4111,6 +4156,14 @@ void AudioFlinger::MixerThread::threadLoop_sleepTime() AudioFlinger::PlaybackThread::mixer_state AudioFlinger::MixerThread::prepareTracks_l( Vector< sp > *tracksToRemove) { + // clean up deleted track names in AudioMixer before allocating new tracks + (void)mTracks.processDeletedTrackNames([this](int name) { + // for each name, destroy it in the AudioMixer + if (mAudioMixer->exists(name)) { + mAudioMixer->destroy(name); + } + }); + mTracks.clearDeletedTrackNames(); mixer_state mixerStatus = MIXER_IDLE; // find out which tracks need to be processed @@ -4332,6 +4385,24 @@ AudioFlinger::PlaybackThread::mixer_state AudioFlinger::MixerThread::prepareTrac // The first time a track is added we wait // for all its buffers to be filled before processing it int name = track->name(); + + // if an active track doesn't exist in the AudioMixer, create it. + if (!mAudioMixer->exists(name)) { + status_t status = mAudioMixer->create( + name, + track->mChannelMask, + track->mFormat, + track->mSessionId); + if (status != OK) { + ALOGW("%s: cannot create track name" + " %d, mask %#x, format %#x, sessionId %d in AudioMixer", + __func__, name, track->mChannelMask, track->mFormat, track->mSessionId); + tracksToRemove->add(track); + track->invalidate(); // consider it dead. + continue; + } + } + // make sure that we have enough frames to mix one full buffer. // enforce this condition only once to enable draining the buffer in case the client // app does not call stop() and relies on underrun to stop: @@ -4357,20 +4428,9 @@ AudioFlinger::PlaybackThread::mixer_state AudioFlinger::MixerThread::prepareTrac size_t framesReady = track->framesReady(); if (ATRACE_ENABLED()) { // I wish we had formatted trace names - char traceName[16]; - strcpy(traceName, "nRdy"); - int name = track->name(); - if (AudioMixer::TRACK0 <= name && - name < (int) (AudioMixer::TRACK0 + AudioMixer::MAX_NUM_TRACKS)) { - name -= AudioMixer::TRACK0; - traceName[4] = (name / 10) + '0'; - traceName[5] = (name % 10) + '0'; - } else { - traceName[4] = '?'; - traceName[5] = '?'; - } - traceName[6] = '\0'; - ATRACE_INT(traceName, framesReady); + std::string traceName("nRdy"); + traceName += std::to_string(track->name()); + ATRACE_INT(traceName.c_str(), framesReady); } if ((framesReady >= minFrames) && track->isReady() && !track->isPaused() && !track->isTerminated()) @@ -4736,7 +4796,7 @@ AudioFlinger::PlaybackThread::mixer_state AudioFlinger::MixerThread::prepareTrac } // trackCountForUid_l() must be called with ThreadBase::mLock held -uint32_t AudioFlinger::PlaybackThread::trackCountForUid_l(uid_t uid) +uint32_t AudioFlinger::PlaybackThread::trackCountForUid_l(uid_t uid) const { uint32_t trackCount = 0; for (size_t i = 0; i < mTracks.size() ; i++) { @@ -4747,21 +4807,24 @@ uint32_t AudioFlinger::PlaybackThread::trackCountForUid_l(uid_t uid) return trackCount; } -// getTrackName_l() must be called with ThreadBase::mLock held -int AudioFlinger::MixerThread::getTrackName_l(audio_channel_mask_t channelMask, - audio_format_t format, audio_session_t sessionId, uid_t uid) +// isTrackAllowed_l() must be called with ThreadBase::mLock held +bool AudioFlinger::MixerThread::isTrackAllowed_l( + audio_channel_mask_t channelMask, audio_format_t format, + audio_session_t sessionId, uid_t uid) const { - if (trackCountForUid_l(uid) > (PlaybackThread::kMaxTracksPerUid - 1)) { - return -1; + if (!PlaybackThread::isTrackAllowed_l(channelMask, format, sessionId, uid)) { + return false; } - return mAudioMixer->getTrackName(channelMask, format, sessionId); -} - -// deleteTrackName_l() must be called with ThreadBase::mLock held -void AudioFlinger::MixerThread::deleteTrackName_l(int name) -{ - ALOGV("remove track (%d) and delete from mixer", name); - mAudioMixer->deleteTrackName(name); + // Check validity as we don't call AudioMixer::create() here. + if (!AudioMixer::isValidFormat(format)) { + ALOGW("%s: invalid format: %#x", __func__, format); + return false; + } + if (!AudioMixer::isValidChannelMask(channelMask)) { + ALOGW("%s: invalid channelMask: %#x", __func__, channelMask); + return false; + } + return true; } // checkForNewParameter_l() must be called with ThreadBase::mLock held @@ -4854,13 +4917,18 @@ bool AudioFlinger::MixerThread::checkForNewParameter_l(const String8& keyValuePa readOutputParameters_l(); delete mAudioMixer; mAudioMixer = new AudioMixer(mNormalFrameCount, mSampleRate); - for (size_t i = 0; i < mTracks.size() ; i++) { - int name = getTrackName_l(mTracks[i]->mChannelMask, - mTracks[i]->mFormat, mTracks[i]->mSessionId, mTracks[i]->uid()); - if (name < 0) { - break; - } - mTracks[i]->mName = name; + for (const auto &track : mTracks) { + const int name = track->name(); + status_t status = mAudioMixer->create( + name, + track->mChannelMask, + track->mFormat, + track->mSessionId); + ALOGW_IF(status != NO_ERROR, + "%s: cannot create track name" + " %d, mask %#x, format %#x, sessionId %d in AudioMixer", + __func__, + name, track->mChannelMask, track->mFormat, track->mSessionId); } sendIoConfigEvent_l(AUDIO_OUTPUT_CONFIG_CHANGED); } @@ -5309,21 +5377,6 @@ bool AudioFlinger::DirectOutputThread::shouldStandby_l() return !mStandby && !(trackPaused || (mHwPaused && !trackStopped)); } -// getTrackName_l() must be called with ThreadBase::mLock held -int AudioFlinger::DirectOutputThread::getTrackName_l(audio_channel_mask_t channelMask __unused, - audio_format_t format __unused, audio_session_t sessionId __unused, uid_t uid) -{ - if (trackCountForUid_l(uid) > (PlaybackThread::kMaxTracksPerUid - 1)) { - return -1; - } - return 0; -} - -// deleteTrackName_l() must be called with ThreadBase::mLock held -void AudioFlinger::DirectOutputThread::deleteTrackName_l(int name __unused) -{ -} - // checkForNewParameter_l() must be called with ThreadBase::mLock held bool AudioFlinger::DirectOutputThread::checkForNewParameter_l(const String8& keyValuePair, status_t& status) @@ -5941,6 +5994,31 @@ void AudioFlinger::DuplicatingThread::threadLoop_standby() } } +void AudioFlinger::DuplicatingThread::dumpInternals(int fd, const Vector& args __unused) +{ + MixerThread::dumpInternals(fd, args); + + std::stringstream ss; + const size_t numTracks = mOutputTracks.size(); + ss << " " << numTracks << " OutputTracks"; + if (numTracks > 0) { + ss << ":"; + for (const auto &track : mOutputTracks) { + const sp thread = track->thread().promote(); + ss << " (" << track->name() << " : "; + if (thread.get() != nullptr) { + ss << thread.get() << ", " << thread->id(); + } else { + ss << "null"; + } + ss << ")"; + } + } + ss << "\n"; + std::string result = ss.str(); + write(fd, result.c_str(), result.size()); +} + void AudioFlinger::DuplicatingThread::saveOutputTracks() { outputTracks = mOutputTracks; diff --git a/services/audioflinger/Threads.h b/services/audioflinger/Threads.h index 53cb8adccb..ae14ac1a6b 100644 --- a/services/audioflinger/Threads.h +++ b/services/audioflinger/Threads.h @@ -624,6 +624,7 @@ public: static const int8_t kMaxTrackStartupRetriesOffload = 100; static const int8_t kMaxTrackStopRetriesOffload = 2; static constexpr uint32_t kMaxTracksPerUid = 40; + static constexpr size_t kMaxTracks = 256; // Maximum delay (in nanoseconds) for upcoming buffers in suspend mode, otherwise // if delay is greater, the estimated time for timeLoopNextNs is reset. @@ -778,6 +779,16 @@ public: virtual bool isOutput() const override { return true; } + // returns true if the track is allowed to be added to the thread. + virtual bool isTrackAllowed_l( + audio_channel_mask_t channelMask __unused, + audio_format_t format __unused, + audio_session_t sessionId __unused, + uid_t uid) const { + return trackCountForUid_l(uid) < PlaybackThread::kMaxTracksPerUid + && mTracks.size() < PlaybackThread::kMaxTracks; + } + protected: // updated by readOutputParameters_l() size_t mNormalFrameCount; // normal mixer and effects @@ -866,12 +877,6 @@ private: protected: ActiveTracks mActiveTracks; - // Allocate a track name for a given channel mask. - // Returns name >= 0 if successful, -1 on failure. - virtual int getTrackName_l(audio_channel_mask_t channelMask, audio_format_t format, - audio_session_t sessionId, uid_t uid) = 0; - virtual void deleteTrackName_l(int name) = 0; - // Time to sleep between cycles when: virtual uint32_t activeSleepTimeUs() const; // mixer state MIXER_TRACKS_ENABLED virtual uint32_t idleSleepTimeUs() const = 0; // mixer state MIXER_IDLE @@ -899,7 +904,7 @@ protected: && mHwSupportsPause && (mOutput->flags & AUDIO_OUTPUT_FLAG_HW_AV_SYNC); } - uint32_t trackCountForUid_l(uid_t uid); + uint32_t trackCountForUid_l(uid_t uid) const; private: @@ -916,7 +921,64 @@ private: virtual void dumpInternals(int fd, const Vector& args); void dumpTracks(int fd, const Vector& args); - SortedVector< sp > mTracks; + // The Tracks class manages names for all tracks + // added and removed from the Thread. + template + class Tracks { + public: + Tracks(bool saveDeletedTrackNames) : + mSaveDeletedTrackNames(saveDeletedTrackNames) { } + + // SortedVector methods + ssize_t add(const sp &track); + ssize_t remove(const sp &track); + size_t size() const { + return mTracks.size(); + } + bool isEmpty() const { + return mTracks.isEmpty(); + } + ssize_t indexOf(const sp &item) { + return mTracks.indexOf(item); + } + sp operator[](size_t index) const { + return mTracks[index]; + } + typename SortedVector>::iterator begin() { + return mTracks.begin(); + } + typename SortedVector>::iterator end() { + return mTracks.end(); + } + + size_t processDeletedTrackNames(std::function f) { + const size_t size = mDeletedTrackNames.size(); + if (size > 0) { + for (const int name : mDeletedTrackNames) { + f(name); + } + } + return size; + } + + void clearDeletedTrackNames() { mDeletedTrackNames.clear(); } + + private: + // Track names pending deletion for MIXER type threads + const bool mSaveDeletedTrackNames; // true to enable tracking + std::set mDeletedTrackNames; + + // Fast lookup of previously deleted track names for reuse. + // This is an arbitrary decision (actually any non-negative + // integer that isn't in mTracks[*]->names() could be used) - we attempt + // to use the smallest possible available name. + std::set mUnusedTrackNames; + + SortedVector> mTracks; // wrapped SortedVector. + }; + + Tracks mTracks; + stream_type_t mStreamTypes[AUDIO_STREAM_CNT]; AudioStreamOut *mOutput; @@ -1023,11 +1085,11 @@ public: status_t& status); virtual void dumpInternals(int fd, const Vector& args); + virtual bool isTrackAllowed_l( + audio_channel_mask_t channelMask, audio_format_t format, + audio_session_t sessionId, uid_t uid) const override; protected: virtual mixer_state prepareTracks_l(Vector< sp > *tracksToRemove); - virtual int getTrackName_l(audio_channel_mask_t channelMask, audio_format_t format, - audio_session_t sessionId, uid_t uid); - virtual void deleteTrackName_l(int name); virtual uint32_t idleSleepTimeUs() const; virtual uint32_t suspendSleepTimeUs() const; virtual void cacheParameters_l(); @@ -1105,9 +1167,6 @@ public: virtual void flushHw_l(); protected: - virtual int getTrackName_l(audio_channel_mask_t channelMask, audio_format_t format, - audio_session_t sessionId, uid_t uid); - virtual void deleteTrackName_l(int name); virtual uint32_t activeSleepTimeUs() const; virtual uint32_t idleSleepTimeUs() const; virtual uint32_t suspendSleepTimeUs() const; @@ -1211,6 +1270,8 @@ public: virtual ~DuplicatingThread(); // Thread virtuals + virtual void dumpInternals(int fd, const Vector& args) override; + void addOutputTrack(MixerThread* thread); void removeOutputTrack(MixerThread* thread); uint32_t waitTimeMs() const { return mWaitTimeMs; } diff --git a/services/audioflinger/TrackBase.h b/services/audioflinger/TrackBase.h index a3ea756d28..a7e966f1e9 100644 --- a/services/audioflinger/TrackBase.h +++ b/services/audioflinger/TrackBase.h @@ -54,6 +54,11 @@ public: TYPE_PATCH, }; + enum { + TRACK_NAME_PENDING = -1, + TRACK_NAME_FAILURE = -2, + }; + TrackBase(ThreadBase *thread, const sp& client, uint32_t sampleRate, diff --git a/services/audioflinger/Tracks.cpp b/services/audioflinger/Tracks.cpp index 67f27d0d33..9b93939bdb 100644 --- a/services/audioflinger/Tracks.cpp +++ b/services/audioflinger/Tracks.cpp @@ -394,7 +394,7 @@ AudioFlinger::PlaybackThread::Track::Track( // mRetryCount initialized later when needed mSharedBuffer(sharedBuffer), mStreamType(streamType), - mName(-1), // see note below + mName(TRACK_NAME_FAILURE), // set to TRACK_NAME_PENDING on constructor success. mMainBuffer(thread->sinkBuffer()), mAuxBuffer(NULL), mAuxEffectId(0), mHasVolumeController(false), @@ -427,9 +427,8 @@ AudioFlinger::PlaybackThread::Track::Track( } mServerProxy = mAudioTrackServerProxy; - mName = thread->getTrackName_l(channelMask, format, sessionId, uid); - if (mName < 0) { - ALOGE("no more track names available"); + if (!thread->isTrackAllowed_l(channelMask, format, sessionId, uid)) { + ALOGE("no more tracks available"); return; } // only allocate a fast track index if we were able to allocate a normal track name @@ -448,6 +447,7 @@ AudioFlinger::PlaybackThread::Track::Track( mFastIndex = i; thread->mFastTrackAvailMask &= ~(1 << i); } + mName = TRACK_NAME_PENDING; } AudioFlinger::PlaybackThread::Track::~Track() @@ -466,7 +466,7 @@ AudioFlinger::PlaybackThread::Track::~Track() status_t AudioFlinger::PlaybackThread::Track::initCheck() const { status_t status = TrackBase::initCheck(); - if (status == NO_ERROR && mName < 0) { + if (status == NO_ERROR && mName == TRACK_NAME_FAILURE) { status = NO_MEMORY; } return status; @@ -527,10 +527,12 @@ void AudioFlinger::PlaybackThread::Track::appendDump(String8& result, bool activ if (isFastTrack()) { result.appendFormat("F%c %3d", trackType, mFastIndex); - } else if (mName >= AudioMixer::TRACK0) { - result.appendFormat("%c %4d", trackType, mName - AudioMixer::TRACK0); + } else if (mName == TRACK_NAME_PENDING) { + result.appendFormat("%c pend", trackType); + } else if (mName == TRACK_NAME_FAILURE) { + result.appendFormat("%c fail", trackType); } else { - result.appendFormat("%c none", trackType); + result.appendFormat("%c %4d", trackType, mName); } char nowInUnderrun;