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

Commit 421ddc01 authored by Eric Laurent's avatar Eric Laurent
Browse files

Fix issue 3439872: video chat and bluetooth SCO 

This change fixes the stability problems experienced when using
a bluetooth headset supporting both A2DP and SCO. Problems occur
when starting the video chat at which time the A2DP output is being
stopped to start SCO. At that time, active AudioTracks are invalidated
by AudioFlinger so that a new AudioTrack binder interface can be
recreated by the client process on the new mixer thread with correct parameters.
The problem was that the process to restore the binder interface was not
protected against concurrent requests which caused 2 binder interfaces
to be created sometimes. This could lead to permanent client deadlock
if one of the client threads was waiting for a condition of the first
created binder interface while the second one was created (as the AudioFlinger
would only signal conditions on the last one created).
This concurrent request situation is more likely to happen when a client
uses the JAVA AudioTrack as the JNI implementation uses simultaneously the
native AudioTrack callback and write push mechanisms. By doing so, the code
that checks if the binder interface should be restored (in obtainBuffer()) is
much more likely to be called concurrently from two different threads.

The fix consists in protecting the critical binder interface restore phase
with a flag in the AudioTrack control block. The first thread acting upon the binder
interface restore request will raise the flag and the second thread will just wait for
a condition to be signaled when the restore process is complete.

Also protected all accesses to the AudioTrack control block by a mutex to prevent
access while the track is being destroyed and restored. If a mutex cannot be held
(e.g because we call a callback function), acquire a strong reference on the IAudioTrack
to prevent its destruction while the cblk is being accessed.

Modified AudioTrack JNI to use GetByteArrayElements() instead of
GetPrimitiveArrayCritical() when writing audio buffers. Entering a critical section would
cause the JNI to abort if a mediaserver crash occurs during a write due to the AudioSystem
callback being called during the critical section when media server process restarts.
Anyway with current JNI implementation, either versions do not copy data most of the times
and the criticial version does not guaranty no data copy.

The same modifications have been made to AudioRecord.

Change-Id: Idc5aa711a04c3eee180cdd03f44fe17f3c4dcb52
parent 949d0c8c

core/jni/android_media_AudioRecord.cpp

+6 −2
Original line number Diff line number Diff line
@@ -315,7 +315,11 @@ static jint android_media_AudioRecord_readInByteArray(JNIEnv *env, jobject thiz 
    }



    // get the pointer to where we'll record the audio

    recordBuff = (jbyte *)env->GetPrimitiveArrayCritical(javaAudioData, NULL);

    // NOTE: We may use GetPrimitiveArrayCritical() when the JNI implementation changes in such

    // a way that it becomes much more efficient. When doing so, we will have to prevent the

    // AudioSystem callback to be called while in critical section (in case of media server

    // process crash for instance)

    recordBuff = (jbyte *)env->GetByteArrayElements(javaAudioData, NULL);



    if (recordBuff == NULL) {

        LOGE("Error retrieving destination for recorded audio data, can't record");
@@ -327,7 +331,7 @@ static jint android_media_AudioRecord_readInByteArray(JNIEnv *env, jobject thiz 
    ssize_t readSize = lpRecorder->read(recordBuff + offsetInBytes, 

                                        sizeInBytes > (jint)recorderBuffSize ? 

                                            (jint)recorderBuffSize : sizeInBytes );

    env->ReleasePrimitiveArrayCritical(javaAudioData, recordBuff, 0);

    env->ReleaseByteArrayElements(javaAudioData, recordBuff, 0);



    return (jint) readSize;

}

core/jni/android_media_AudioTrack.cpp

+6 −2
Original line number Diff line number Diff line
@@ -530,8 +530,12 @@ static jint android_media_AudioTrack_native_write(JNIEnv *env, jobject thiz, 
    }



    // get the pointer for the audio data from the java array

    // NOTE: We may use GetPrimitiveArrayCritical() when the JNI implementation changes in such

    // a way that it becomes much more efficient. When doing so, we will have to prevent the

    // AudioSystem callback to be called while in critical section (in case of media server

    // process crash for instance)

    if (javaAudioData) {

        cAudioData = (jbyte *)env->GetPrimitiveArrayCritical(javaAudioData, NULL);

        cAudioData = (jbyte *)env->GetByteArrayElements(javaAudioData, NULL);

        if (cAudioData == NULL) {

            LOGE("Error retrieving source of audio data to play, can't play");

            return 0; // out of memory or no data to load
@@ -543,7 +547,7 @@ static jint android_media_AudioTrack_native_write(JNIEnv *env, jobject thiz, 


    jint written = writeToTrack(lpTrack, javaAudioFormat, cAudioData, offsetInBytes, sizeInBytes);



    env->ReleasePrimitiveArrayCritical(javaAudioData, cAudioData, 0);

    env->ReleaseByteArrayElements(javaAudioData, cAudioData, 0);



    //LOGV("write wrote %d (tried %d) bytes in the native AudioTrack with offset %d",

    //     (int)written, (int)(sizeInBytes), (int)offsetInBytes);

include/media/AudioRecord.h

+3 −1
Original line number Diff line number Diff line
@@ -346,12 +346,14 @@ private: 
    };



            bool processAudioBuffer(const sp<ClientRecordThread>& thread);

            status_t openRecord(uint32_t sampleRate,

            status_t openRecord_l(uint32_t sampleRate,

                                int format,

                                int channelCount,

                                int frameCount,

                                uint32_t flags,

                                audio_io_handle_t input);

            audio_io_handle_t getInput_l();

            status_t restoreRecord_l(audio_track_cblk_t*& cblk);



    sp<IAudioRecord>        mAudioRecord;

    sp<IMemory>             mCblkMemory;

include/media/AudioTrack.h

+5 −1
Original line number Diff line number Diff line
@@ -437,7 +437,7 @@ private: 
    };



            bool processAudioBuffer(const sp<AudioTrackThread>& thread);

            status_t createTrack(int streamType,

            status_t createTrack_l(int streamType,

                                 uint32_t sampleRate,

                                 int format,

                                 int channelCount,
@@ -446,6 +446,10 @@ private: 
                                 const sp<IMemory>& sharedBuffer,

                                 audio_io_handle_t output,

                                 bool enforceFrameCount);

            void flush_l();

            status_t setLoop_l(uint32_t loopStart, uint32_t loopEnd, int loopCount);

            audio_io_handle_t getOutput_l();

            status_t restoreTrack_l(audio_track_cblk_t*& cblk, bool fromStart);



    sp<IAudioTrack>         mAudioTrack;

    sp<IMemory>             mCblkMemory;

include/private/media/AudioTrackShared.h

+7 −0
Original line number Diff line number Diff line
@@ -31,6 +31,7 @@ namespace android { 
#define MAX_STARTUP_TIMEOUT_MS  3000    // Longer timeout period at startup to cope with A2DP init time

#define MAX_RUN_TIMEOUT_MS      1000

#define WAIT_PERIOD_MS          10

#define RESTORE_TIMEOUT_MS      5000    // Maximum waiting time for a track to be restored



#define CBLK_UNDERRUN_MSK       0x0001

#define CBLK_UNDERRUN_ON        0x0001  // underrun (out) or overrrun (in) indication
@@ -47,6 +48,12 @@ namespace android { 
#define CBLK_DISABLED_MSK       0x0010

#define CBLK_DISABLED_ON        0x0010  // track disabled by AudioFlinger due to underrun:

#define CBLK_DISABLED_OFF       0x0000  // must be re-started

#define CBLK_RESTORING_MSK      0x0020

#define CBLK_RESTORING_ON       0x0020  // track is being restored after invalidation

#define CBLK_RESTORING_OFF      0x0000  // by AudioFlinger

#define CBLK_RESTORED_MSK       0x0040

#define CBLK_RESTORED_ON        0x0040  // track has been restored after invalidation

#define CBLK_RESTORED_OFF       0x0040  // by AudioFlinger



struct audio_track_cblk_t

{