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

Commit cd154933 authored by Andy Hung's avatar Andy Hung Committed by Android (Google) Code Review
Browse files

Merge "AudioRecord: Clean up state handling"

parents bd1be2da ce685406
Loading
Loading
Loading
Loading
+44 −29
Original line number Diff line number Diff line
@@ -6654,6 +6654,7 @@ reacquire_wakelock:

                case TrackBase::PAUSING:
                    mActiveTracks.remove(activeTrack);
                    activeTrack->mState = TrackBase::PAUSED;
                    doBroadcast = true;
                    size--;
                    continue;
@@ -6675,12 +6676,12 @@ reacquire_wakelock:
                    allStopped = false;
                    break;

                case TrackBase::IDLE:
                    i++;
                    continue;

                case TrackBase::IDLE:    // cannot be on ActiveTracks if idle
                case TrackBase::PAUSED:  // cannot be on ActiveTracks if paused
                case TrackBase::STOPPED: // cannot be on ActiveTracks if destroyed/terminated
                default:
                    LOG_ALWAYS_FATAL("Unexpected activeTrackState %d", activeTrackState);
                    LOG_ALWAYS_FATAL("%s: Unexpected active track state:%d, id:%d, tracks:%zu",
                            __func__, activeTrackState, activeTrack->id(), size);
                }

                activeTracks.add(activeTrack);
@@ -7323,8 +7324,14 @@ status_t AudioFlinger::RecordThread::start(RecordThread::RecordTrack* recordTrac
    {
        // This section is a rendezvous between binder thread executing start() and RecordThread
        AutoMutex lock(mLock);
        if (recordTrack->isInvalid()) {
            recordTrack->clearSyncStartEvent();
            return INVALID_OPERATION;
        }
        if (mActiveTracks.indexOf(recordTrack) >= 0) {
            if (recordTrack->mState == TrackBase::PAUSING) {
                // We haven't stopped yet (moved to PAUSED and not in mActiveTracks)
                // so no need to startInput().
                ALOGV("active record track PAUSING -> ACTIVE");
                recordTrack->mState = TrackBase::ACTIVE;
            } else {
@@ -7344,11 +7351,30 @@ status_t AudioFlinger::RecordThread::start(RecordThread::RecordTrack* recordTrac
            bool silenced;
            status = AudioSystem::startInput(recordTrack->portId(), &silenced);
            mLock.lock();
            // FIXME should verify that recordTrack is still in mActiveTracks
            if (recordTrack->isInvalid()) {
                recordTrack->clearSyncStartEvent();
                if (status == NO_ERROR && recordTrack->mState == TrackBase::STARTING_1) {
                    recordTrack->mState = TrackBase::STARTING_2;
                    // STARTING_2 forces destroy to call stopInput.
                }
                return INVALID_OPERATION;
            }
            if (recordTrack->mState != TrackBase::STARTING_1) {
                ALOGW("%s(%d): unsynchronized mState:%d change",
                    __func__, recordTrack->id(), recordTrack->mState);
                // Someone else has changed state, let them take over,
                // leave mState in the new state.
                recordTrack->clearSyncStartEvent();
                return INVALID_OPERATION;
            }
            // we're ok, but perhaps startInput has failed
            if (status != NO_ERROR) {
                ALOGW("%s(%d): startInput failed, status %d",
                    __func__, recordTrack->id(), status);
                // We are in ActiveTracks if STARTING_1 and valid, so remove from ActiveTracks,
                // leave in STARTING_1, so destroy() will not call stopInput.
                mActiveTracks.remove(recordTrack);
                recordTrack->clearSyncStartEvent();
                ALOGV("RecordThread::start error %d", status);
                return status;
            }
            recordTrack->setSilenced(silenced);
@@ -7366,21 +7392,8 @@ status_t AudioFlinger::RecordThread::start(RecordThread::RecordTrack* recordTrac
        recordTrack->mState = TrackBase::STARTING_2;
        // signal thread to start
        mWaitWorkCV.broadcast();
        if (mActiveTracks.indexOf(recordTrack) < 0) {
            ALOGV("Record failed to start");
            status = BAD_VALUE;
            goto startError;
        }
        return status;
    }

startError:
    if (recordTrack->isExternalTrack()) {
        AudioSystem::stopInput(recordTrack->portId());
    }
    recordTrack->clearSyncStartEvent();
    // FIXME I wonder why we do not reset the state here?
    return status;
}

void AudioFlinger::RecordThread::syncStartEventCallback(const wp<SyncEvent>& event)
@@ -7399,24 +7412,26 @@ void AudioFlinger::RecordThread::syncStartEventCallback(const wp<SyncEvent>& eve
bool AudioFlinger::RecordThread::stop(RecordThread::RecordTrack* recordTrack) {
    ALOGV("RecordThread::stop");
    AutoMutex _l(mLock);
    // if we're invalid, we can't be on the ActiveTracks.
    if (mActiveTracks.indexOf(recordTrack) < 0 || recordTrack->mState == TrackBase::PAUSING) {
        return false;
    }
    // note that threadLoop may still be processing the track at this point [without lock]
    recordTrack->mState = TrackBase::PAUSING;
    // signal thread to stop
    mWaitWorkCV.broadcast();
    // do not wait for mStartStopCond if exiting
    if (exitPending()) {
        return true;
    }
    // FIXME incorrect usage of wait: no explicit predicate or loop

    while (recordTrack->mState == TrackBase::PAUSING && !recordTrack->isInvalid()) {
        mWaitWorkCV.broadcast(); // signal thread to stop
        mStartStopCond.wait(mLock);
    // if we have been restarted, recordTrack is in mActiveTracks here
    if (exitPending() || mActiveTracks.indexOf(recordTrack) < 0) {
    }

    if (recordTrack->mState == TrackBase::PAUSED) { // successful stop
        ALOGV("Record stopped OK");
        return true;
    }

    // don't handle anything - we've been invalidated or restarted and in a different state
    ALOGW_IF("%s(%d): unsynchronized stop, state: %d",
            __func__, recordTrack->id(), recordTrack->mState);
    return false;
}

+2 −2
Original line number Diff line number Diff line
@@ -25,13 +25,13 @@ class TrackBase : public ExtendedAudioBufferProvider, public RefBase {
public:
    enum track_state {
        IDLE,
        FLUSHED,
        FLUSHED,        // for PlaybackTracks only
        STOPPED,
        // next 2 states are currently used for fast tracks
        // and offloaded tracks only
        STOPPING_1,     // waiting for first underrun
        STOPPING_2,     // waiting for presentation complete
        RESUMING,
        RESUMING,       // for PlaybackTracks only
        ACTIVE,
        PAUSING,
        PAUSED,
+24 −7
Original line number Diff line number Diff line
@@ -1823,17 +1823,34 @@ void AudioFlinger::RecordThread::RecordTrack::destroy()
    // see comments at AudioFlinger::PlaybackThread::Track::destroy()
    sp<RecordTrack> keep(this);
    {
        if (isExternalTrack()) {
            if (mState == ACTIVE || mState == RESUMING) {
                AudioSystem::stopInput(mPortId);
            }
            AudioSystem::releaseInput(mPortId);
        }
        track_state priorState = mState;
        sp<ThreadBase> thread = mThread.promote();
        if (thread != 0) {
            Mutex::Autolock _l(thread->mLock);
            RecordThread *recordThread = (RecordThread *) thread.get();
            recordThread->destroyTrack_l(this);
            priorState = mState;
            recordThread->destroyTrack_l(this); // move mState to STOPPED, terminate
        }
        // APM portid/client management done outside of lock.
        // NOTE: if thread doesn't exist, the input descriptor probably doesn't either.
        if (isExternalTrack()) {
            switch (priorState) {
            case ACTIVE:     // invalidated while still active
            case STARTING_2: // invalidated/start-aborted after startInput successfully called
            case PAUSING:    // invalidated while in the middle of stop() pausing (still active)
                AudioSystem::stopInput(mPortId);
                break;

            case STARTING_1: // invalidated/start-aborted and startInput not successful
            case PAUSED:     // OK, not active
            case IDLE:       // OK, not active
                break;

            case STOPPED:    // unexpected (destroyed)
            default:
                LOG_ALWAYS_FATAL("%s(%d): invalid prior state: %d", __func__, mId, priorState);
            }
            AudioSystem::releaseInput(mPortId);
        }
    }
}