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

Commit 7410591d authored by Glenn Kasten's avatar Glenn Kasten
Browse files

Move AudioRecord frame count calculations to server 

Buffer frame count and notification frame count
are now calculated by server instead of by client.
The server has more information and can do a better job.

Also fix a few bugs:

 - If a fast track was re-created, even with same pipe depth, it would fail.
   Now it can correctly re-create a fast track provided the pipe depth is same.

 - Notification frame count for fast tracks was calculated by client
   as 1/2 of the total frame count, which is a large value due to the pipe.
   Now the notification frame count is set by server to the HAL frame count.
   This should reduce latency for fast tracks.

 - EVENT_OVERRUN were happening frequently when there was sample rate conversion,
   because the client didn't know about the sample rate conversion,
   and under-estimated the necessary buffer size.  Now since server
   calculates the buffer sizes, EVENT_OVERRUN is unlikely.

 - RecordThread::createRecordTrack_l was checking for mono and stereo
   for fast tracks.  This is not necessary, and now we can handle a
   multi-channel fast track.

Bug: 7498763
Change-Id: I0c581618e8db33084d5ff9ed50a592990c9749e8
parent b3b1660e

include/media/AudioRecord.h

+1 −0
Original line number Diff line number Diff line
@@ -461,6 +461,7 @@ private: 
    // for notification APIs

    uint32_t                mNotificationFramesReq; // requested number of frames between each

                                                    // notification callback

                                                    // as specified in constructor or set()

    uint32_t                mNotificationFramesAct; // actual number of frames between each

                                                    // notification callback

    bool                    mRefreshRemaining;      // processAudioBuffer() should refresh

media/libmedia/AudioRecord.cpp

+9 −44
Original line number Diff line number Diff line
@@ -211,7 +211,7 @@ status_t AudioRecord::set( 
    mReqFrameCount = frameCount;



    mNotificationFramesReq = notificationFrames;

    mNotificationFramesAct = 0;

    // mNotificationFramesAct is initialized in openRecord_l



    if (sessionId == AUDIO_SESSION_ALLOCATE) {

        mSessionId = AudioSystem::newAudioSessionId();
@@ -444,42 +444,6 @@ status_t AudioRecord::openRecord_l(size_t epoch) 
        }

    }



    // FIXME Assume double buffering, because we don't know the true HAL sample rate

    const uint32_t nBuffering = 2;



    mNotificationFramesAct = mNotificationFramesReq;

    size_t frameCount = mReqFrameCount;



    if (!(mFlags & AUDIO_INPUT_FLAG_FAST)) {

        // validate framecount

        // If fast track was not requested, this preserves

        // the old behavior of validating on client side.

        // FIXME Eventually the validation should be done on server side

        // regardless of whether it's a fast or normal track.  It's debatable

        // whether to account for the input latency to provision buffers appropriately.

        size_t minFrameCount;

        status = AudioRecord::getMinFrameCount(&minFrameCount,

                mSampleRate, mFormat, mChannelMask);

        if (status != NO_ERROR) {

            ALOGE("getMinFrameCount() failed for sampleRate %u, format %#x, channelMask %#x; "

                    "status %d",

                    mSampleRate, mFormat, mChannelMask, status);

            return status;

        }



        if (frameCount == 0) {

            frameCount = minFrameCount;

        } else if (frameCount < minFrameCount) {

            ALOGE("frameCount %zu < minFrameCount %zu", frameCount, minFrameCount);

            return BAD_VALUE;

        }



        // Make sure that application is notified with sufficient margin before overrun

        if (mNotificationFramesAct == 0 || mNotificationFramesAct > frameCount/2) {

            mNotificationFramesAct = frameCount/2;

        }

    }



    audio_io_handle_t input = AudioSystem::getInput(mInputSource, mSampleRate, mFormat,

            mChannelMask, mSessionId, mFlags);

    if (input == AUDIO_IO_HANDLE_NONE) {
@@ -492,12 +456,13 @@ status_t AudioRecord::openRecord_l(size_t epoch) 
    // Now that we have a reference to an I/O handle and have not yet handed it off to AudioFlinger,

    // we must release it ourselves if anything goes wrong.



    size_t frameCount = mReqFrameCount;

    size_t temp = frameCount;   // temp may be replaced by a revised value of frameCount,

                                // but we will still need the original value also

    int originalSessionId = mSessionId;



    // The notification frame count is the period between callbacks, as suggested by the server.

    size_t notificationFrames;

    size_t notificationFrames = mNotificationFramesReq;



    sp<IMemory> iMem;           // for cblk

    sp<IMemory> bufferMem;
@@ -576,13 +541,13 @@ status_t AudioRecord::openRecord_l(size_t epoch) 
            // once denied, do not request again if IAudioRecord is re-created

            mFlags = (audio_input_flags_t) (mFlags & ~AUDIO_INPUT_FLAG_FAST);

        }

        // Theoretically double-buffering is not required for fast tracks,

        // due to tighter scheduling.  But in practice, to accomodate kernels with

        // scheduling jitter, and apps with computation jitter, we use double-buffering.

        if (mNotificationFramesAct == 0 || mNotificationFramesAct > frameCount/nBuffering) {

            mNotificationFramesAct = frameCount/nBuffering;

    }



    // Make sure that application is notified with sufficient margin before overrun

    if (notificationFrames == 0 || notificationFrames > frameCount) {

        ALOGW("Received notificationFrames %zu for frameCount %zu", notificationFrames, frameCount);

    }

    mNotificationFramesAct = notificationFrames;



    // We retain a copy of the I/O handle, but don't own the reference

    mInput = input;

media/libmedia/IAudioFlinger.cpp

+2 −1
Original line number Diff line number Diff line
@@ -197,6 +197,7 @@ public: 
            lSessionId = *sessionId;

        }

        data.writeInt32(lSessionId);

        data.writeInt64(notificationFrames != NULL ? *notificationFrames : 0);

        cblk.clear();

        buffers.clear();

        status_t lStatus = remote()->transact(OPEN_RECORD, data, &reply);
@@ -966,7 +967,7 @@ status_t BnAudioFlinger::onTransact( 
            track_flags_t flags = (track_flags_t) data.readInt32();

            pid_t tid = (pid_t) data.readInt32();

            int sessionId = data.readInt32();

            size_t notificationFrames = 0;

            size_t notificationFrames = data.readInt64();

            sp<IMemory> cblk;

            sp<IMemory> buffers;

            status_t status;

services/audioflinger/Threads.cpp

+31 −35
Original line number Diff line number Diff line
@@ -5458,21 +5458,14 @@ sp<AudioFlinger::RecordThread::RecordTrack> AudioFlinger::RecordThread::createRe 
    // client expresses a preference for FAST, but we get the final say

    if (*flags & IAudioFlinger::TRACK_FAST) {

      if (

            // use case: callback handler and frame count is default or at least as large as HAL

            (

            // use case: callback handler

            (tid != -1) &&

                ((frameCount == 0) /*||

                // FIXME must be equal to pipe depth, so don't allow it to be specified by client

                // FIXME not necessarily true, should be native frame count for native SR!

                (frameCount >= mFrameCount)*/)

            ) &&

            // frame count is not specified, or is exactly the pipe depth

            ((frameCount == 0) || (frameCount == mPipeFramesP2)) &&

            // PCM data

            audio_is_linear_pcm(format) &&

            // native format

            (format == mFormat) &&

            // mono or stereo

            ( (channelMask == AUDIO_CHANNEL_IN_MONO) ||

              (channelMask == AUDIO_CHANNEL_IN_STEREO) ) &&

            // native channel mask

            (channelMask == mChannelMask) &&

            // native hardware sample rate
@@ -5482,40 +5475,43 @@ sp<AudioFlinger::RecordThread::RecordTrack> AudioFlinger::RecordThread::createRe 
            // there are sufficient fast track slots available

            mFastTrackAvail

        ) {

        // if frameCount not specified, then it defaults to pipe frame count

        if (frameCount == 0) {

            frameCount = mPipeFramesP2;

        }

        ALOGV("AUDIO_INPUT_FLAG_FAST accepted: frameCount=%d mFrameCount=%d",

        ALOGV("AUDIO_INPUT_FLAG_FAST accepted: frameCount=%u mFrameCount=%u",

                frameCount, mFrameCount);

      } else {

        ALOGV("AUDIO_INPUT_FLAG_FAST denied: frameCount=%d "

                "mFrameCount=%d format=%d isLinear=%d channelMask=%#x sampleRate=%u mSampleRate=%u "

        ALOGV("AUDIO_INPUT_FLAG_FAST denied: frameCount=%u mFrameCount=%u mPipeFramesP2=%u "

                "format=%#x isLinear=%d channelMask=%#x sampleRate=%u mSampleRate=%u "

                "hasFastCapture=%d tid=%d mFastTrackAvail=%d",

                frameCount, mFrameCount, format,

                audio_is_linear_pcm(format),

                channelMask, sampleRate, mSampleRate, hasFastCapture(), tid, mFastTrackAvail);

                frameCount, mFrameCount, mPipeFramesP2,

                format, audio_is_linear_pcm(format), channelMask, sampleRate, mSampleRate,

                hasFastCapture(), tid, mFastTrackAvail);

        *flags &= ~IAudioFlinger::TRACK_FAST;

        // FIXME It's not clear that we need to enforce this any more, since we have a pipe.

        // For compatibility with AudioRecord calculation, buffer depth is forced

        // to be at least 2 x the record thread frame count and cover audio hardware latency.

        // This is probably too conservative, but legacy application code may depend on it.

        // If you change this calculation, also review the start threshold which is related.

        // FIXME It's not clear how input latency actually matters.  Perhaps this should be 0.

        uint32_t latencyMs = 50; // FIXME mInput->stream->get_latency(mInput->stream);

        size_t mNormalFrameCount = 2048; // FIXME

        uint32_t minBufCount = latencyMs / ((1000 * mNormalFrameCount) / mSampleRate);

        if (minBufCount < 2) {

            minBufCount = 2;

      }

        size_t minFrameCount = mNormalFrameCount * minBufCount;

    }



    // compute track buffer size in frames, and suggest the notification frame count

    if (*flags & IAudioFlinger::TRACK_FAST) {

        // fast track: frame count is exactly the pipe depth

        frameCount = mPipeFramesP2;

        // ignore requested notificationFrames, and always notify exactly once every HAL buffer

        *notificationFrames = mFrameCount;

    } else {

        // not fast track: frame count is at least 2 HAL buffers and at least 20 ms

        size_t minFrameCount = ((int64_t) mFrameCount * 2 * sampleRate + mSampleRate - 1) /

                mSampleRate;

        if (frameCount < minFrameCount) {

            frameCount = minFrameCount;

        }

        minFrameCount = (sampleRate * 20 / 1000 + 1) & ~1;

        if (frameCount < minFrameCount) {

            frameCount = minFrameCount;

        }

        // notification is forced to be at least double-buffering

        size_t maxNotification = frameCount / 2;

        if (*notificationFrames == 0 || *notificationFrames > maxNotification) {

            *notificationFrames = maxNotification;

        }

    }

    *pFrameCount = frameCount;

    *notificationFrames = 0;    // FIXME implement



    lStatus = initCheck();

    if (lStatus != NO_ERROR) {