From 0145858a05d704e194ef3f7ac96d7804f08a362f Mon Sep 17 00:00:00 2001 From: Glenn Kasten Date: Wed, 29 Jun 2011 08:22:59 -0700 Subject: [PATCH] Call AudioTrack start, stop, and pause when needed These AudioTrack operations are relatively expensive Binder calls (and will soon be even more expensive to fix a deadlock at AudioTrack::start), and they were being called excessively. Now AudioTrack start, stop, and pause are only called when there is an actual play state change. Details: - distinguish ATTR_TRANSPORT vs. ATTR_PLAY_STATE for audio players - android_audioPlayer_setPlayState is only called when the play state changes - handler_AudioPlayer_transport is only called for transport changes other than play state - android_audioPlayer_setPlayState is always called with mutex locked - since media player currently only handles play state, handler_MediaPlayer_transport is still called for both ATTR_TRANSPORT and ATTR_PLAY_STATE - since the handlers for ATTR_BQ_ENQUEUE and ATTR_ABQ_ENQUEUE are only called if in state PLAYING, changed an "if" to an "assert" Change-Id: Iee2968fd98d215885b7105053bb1604f962ea337 --- wilhelm/src/android/AudioPlayer_to_android.cpp | 8 +++++--- wilhelm/src/android/AudioPlayer_to_android.h | 2 +- wilhelm/src/attr.h | 6 +++--- wilhelm/src/handler_bodies.c | 27 +++++++++++++++----------- wilhelm/src/handlers.c | 6 ++++-- wilhelm/src/handlers.h | 4 ++++ wilhelm/src/itf/IPlay.c | 21 ++++++++++++-------- 7 files changed, 46 insertions(+), 28 deletions(-) diff --git a/wilhelm/src/android/AudioPlayer_to_android.cpp b/wilhelm/src/android/AudioPlayer_to_android.cpp index 4fceff7d..e6dbfc4c 100644 --- a/wilhelm/src/android/AudioPlayer_to_android.cpp +++ b/wilhelm/src/android/AudioPlayer_to_android.cpp @@ -1725,12 +1725,14 @@ SLresult android_audioPlayer_metadata_getValue(CAudioPlayer *ap, } //----------------------------------------------------------------------------- -void android_audioPlayer_setPlayState(CAudioPlayer *ap, bool lockAP) { +// preconditions +// ap != NULL +// mutex is locked +// play state has changed +void android_audioPlayer_setPlayState(CAudioPlayer *ap) { - if (lockAP) { object_lock_shared(&ap->mObject); } SLuint32 playState = ap->mPlay.mState; AndroidObjectState objState = ap->mAndroidObjState; - if (lockAP) { object_unlock_shared(&ap->mObject); } switch(ap->mAndroidObjType) { case AUDIOPLAYER_FROM_PCM_BUFFERQUEUE: diff --git a/wilhelm/src/android/AudioPlayer_to_android.h b/wilhelm/src/android/AudioPlayer_to_android.h index 008a79bc..f4137b51 100644 --- a/wilhelm/src/android/AudioPlayer_to_android.h +++ b/wilhelm/src/android/AudioPlayer_to_android.h @@ -123,7 +123,7 @@ extern SLresult android_audioPlayer_metadata_getValue(CAudioPlayer *pAudioPlayer /************************************************************************************************** * Playback control and events ****************************/ -extern void android_audioPlayer_setPlayState(CAudioPlayer *pAudioPlayer, bool lockAP); +extern void android_audioPlayer_setPlayState(CAudioPlayer *pAudioPlayer); extern void android_audioPlayer_useEventMask(CAudioPlayer *pAudioPlayer); diff --git a/wilhelm/src/attr.h b/wilhelm/src/attr.h index 305e715e..28668ae4 100644 --- a/wilhelm/src/attr.h +++ b/wilhelm/src/attr.h @@ -21,14 +21,14 @@ #define ATTR_INDEX_GAIN 0 // volume: volume, stereo position, mute // mute solo: channel mute, channel solo // effect send: set direct level -#define ATTR_INDEX_TRANSPORT 1 // play: play state, looping, callback events mask, +#define ATTR_INDEX_TRANSPORT 1 // play: looping, callback events mask, // marker position, position update period // recorder: duration limit, events mask, // marker position, position update period #define ATTR_INDEX_POSITION 2 // requested position (a.k.a. seek position) #define ATTR_INDEX_BQ_ENQUEUE 3 // (buffer queue non-empty and in playing state) became true #define ATTR_INDEX_ABQ_ENQUEUE 4 // Android buffer queue became non-empty and in playing state -#define ATTR_INDEX_UNUSED5 5 // reserved for future use +#define ATTR_INDEX_PLAY_STATE 5 // play: play state #define ATTR_INDEX_UNUSED6 6 // reserved for future use #define ATTR_INDEX_UNUSED7 7 // reserved for future use #define ATTR_INDEX_MAX 8 // total number of bits used so far @@ -41,6 +41,6 @@ #define ATTR_POSITION (1 << ATTR_INDEX_POSITION) #define ATTR_BQ_ENQUEUE (1 << ATTR_INDEX_BQ_ENQUEUE) #define ATTR_ABQ_ENQUEUE (1 << ATTR_INDEX_ABQ_ENQUEUE) -#define ATTR_UNUSED5 (1 << ATTR_INDEX_UNUSED5) +#define ATTR_PLAY_STATE (1 << ATTR_INDEX_PLAY_STATE) #define ATTR_UNUSED6 (1 << ATTR_INDEX_UNUSED6) #define ATTR_UNUSED7 (1 << ATTR_INDEX_UNUSED7) diff --git a/wilhelm/src/handler_bodies.c b/wilhelm/src/handler_bodies.c index 8bc9d627..9e435b8d 100644 --- a/wilhelm/src/handler_bodies.c +++ b/wilhelm/src/handler_bodies.c @@ -81,14 +81,21 @@ unsigned handler_MidiPlayer_position(IObject *thiz) unsigned handler_AudioPlayer_transport(IObject *thiz) { CAudioPlayer *ap = (CAudioPlayer *) thiz; - // FIXME should only call when state changes - android_audioPlayer_setPlayState(ap, false /*lockAP*/); - // FIXME ditto, but for either eventflags or marker position + // FIXME should only call when either eventflags or marker position changes android_audioPlayer_useEventMask(ap); return ATTR_TRANSPORT; } +// SL_OBJECTID_AUDIOPLAYER, ATTR_PLAY_STATE +unsigned handler_AudioPlayer_play_state(IObject *thiz) +{ + CAudioPlayer *ap = (CAudioPlayer *) thiz; + android_audioPlayer_setPlayState(ap); + return ATTR_PLAY_STATE; +} + + // SL_OBJECTID_AUDIORECORDER, ATTR_TRANSPORT unsigned handler_AudioRecorder_transport(IObject *thiz) { @@ -98,7 +105,7 @@ unsigned handler_AudioRecorder_transport(IObject *thiz) } -// XA_OBJECTID_MEDIAPLAYER, ATTR_TRANSPORT +// XA_OBJECTID_MEDIAPLAYER, ATTR_TRANSPORT or ATTR_PLAY_STATE unsigned handler_MediaPlayer_transport(IObject *thiz) { CMediaPlayer *mp = (CMediaPlayer *) thiz; @@ -106,7 +113,7 @@ unsigned handler_MediaPlayer_transport(IObject *thiz) if (avp != NULL) { android_Player_setPlayState(avp, mp->mPlay.mState, &(mp->mAndroidObjState)); } - return ATTR_TRANSPORT; + return ATTR_TRANSPORT | ATTR_PLAY_STATE; } @@ -115,9 +122,8 @@ unsigned handler_AudioPlayer_bq_enqueue(IObject *thiz) { // ( buffer queue count is non-empty and play state == PLAYING ) became true CAudioPlayer *ap = (CAudioPlayer *) thiz; - if (SL_PLAYSTATE_PLAYING == ap->mPlay.mState) { - android_audioPlayer_bufferQueue_onRefilled_l(ap); - } + assert(SL_PLAYSTATE_PLAYING == ap->mPlay.mState); + android_audioPlayer_bufferQueue_onRefilled_l(ap); return ATTR_BQ_ENQUEUE; } @@ -127,9 +133,8 @@ unsigned handler_AudioPlayer_abq_enqueue(IObject *thiz) { // (Android buffer queue count is non-empty and play state == PLAYING ) became true CAudioPlayer *ap = (CAudioPlayer *) thiz; - if (SL_PLAYSTATE_PLAYING == ap->mPlay.mState) { - android_audioPlayer_androidBufferQueue_onRefilled_l(ap); - } + assert(SL_PLAYSTATE_PLAYING == ap->mPlay.mState); + android_audioPlayer_androidBufferQueue_onRefilled_l(ap); return ATTR_ABQ_ENQUEUE; } diff --git a/wilhelm/src/handlers.c b/wilhelm/src/handlers.c index 739d0b2b..80d9a3f3 100644 --- a/wilhelm/src/handlers.c +++ b/wilhelm/src/handlers.c @@ -32,7 +32,8 @@ const AttributeHandler handlerTable[1 + XA_OBJECTID_CAMERADEVICE + [ATTR_INDEX_GAIN] = handler_MediaPlayer_gain, [ATTR_INDEX_TRANSPORT] = handler_MediaPlayer_transport, [ATTR_INDEX_POSITION] = handler_MediaPlayer_position, - [ATTR_INDEX_ABQ_ENQUEUE] = handler_MediaPlayer_abq_enqueue}, + [ATTR_INDEX_ABQ_ENQUEUE] = handler_MediaPlayer_abq_enqueue, + [ATTR_INDEX_PLAY_STATE] = handler_MediaPlayer_play_state}, // SL IDs need a little arithmetic to make them contiguous with XA IDs #define _(id) ((id) - SL_OBJECTID_ENGINE + XA_OBJECTID_CAMERADEVICE + 1) @@ -42,7 +43,8 @@ const AttributeHandler handlerTable[1 + XA_OBJECTID_CAMERADEVICE + [ATTR_INDEX_TRANSPORT] = handler_AudioPlayer_transport, [ATTR_INDEX_POSITION] = handler_AudioPlayer_position, [ATTR_INDEX_BQ_ENQUEUE] = handler_AudioPlayer_bq_enqueue, - [ATTR_INDEX_ABQ_ENQUEUE] = handler_AudioPlayer_abq_enqueue}, + [ATTR_INDEX_ABQ_ENQUEUE] = handler_AudioPlayer_abq_enqueue, + [ATTR_INDEX_PLAY_STATE] = handler_AudioPlayer_play_state}, [_(SL_OBJECTID_AUDIORECORDER)] = { [ATTR_INDEX_TRANSPORT] = handler_AudioRecorder_transport}, diff --git a/wilhelm/src/handlers.h b/wilhelm/src/handlers.h index 8cdb79b7..4cf49cef 100644 --- a/wilhelm/src/handlers.h +++ b/wilhelm/src/handlers.h @@ -29,11 +29,13 @@ extern unsigned handler_MediaPlayer_gain(IObject *thiz); extern unsigned handler_MediaPlayer_transport(IObject *thiz); extern unsigned handler_MediaPlayer_position(IObject *thiz); extern unsigned handler_MediaPlayer_abq_enqueue(IObject *thiz); +#define handler_MediaPlayer_play_state handler_MediaPlayer_transport extern unsigned handler_AudioPlayer_gain(IObject *thiz); extern unsigned handler_AudioPlayer_transport(IObject *thiz); extern unsigned handler_AudioPlayer_position(IObject *thiz); extern unsigned handler_AudioPlayer_bq_enqueue(IObject *thiz); extern unsigned handler_AudioPlayer_abq_enqueue(IObject *thiz); +extern unsigned handler_AudioPlayer_play_state(IObject *thiz); extern unsigned handler_AudioRecorder_transport(IObject *thiz); extern unsigned handler_MidiPlayer_gain(IObject *thiz); extern unsigned handler_MidiPlayer_position(IObject *thiz); @@ -43,10 +45,12 @@ extern unsigned handler_OutputMix_gain(IObject *thiz); #define handler_MediaPlayer_transport NULL #define handler_MediaPlayer_position NULL #define handler_MediaPlayer_abq_enqueue NULL +#define handler_MediaPlayer_play_state NULL #define handler_AudioPlayer_transport NULL #define handler_AudioPlayer_position NULL #define handler_AudioPlayer_bq_enqueue NULL #define handler_AudioPlayer_abq_enqueue NULL +#define handler_AudioPlayer_play_state NULL #define handler_AudioRecorder_transport NULL #define handler_MidiPlayer_gain NULL #define handler_MidiPlayer_position NULL diff --git a/wilhelm/src/itf/IPlay.c b/wilhelm/src/itf/IPlay.c index 85de8777..676113ac 100644 --- a/wilhelm/src/itf/IPlay.c +++ b/wilhelm/src/itf/IPlay.c @@ -36,26 +36,29 @@ static SLresult IPlay_SetPlayState(SLPlayItf self, SLuint32 state) (CAudioPlayer *) thiz->mThis : NULL; #endif interface_lock_exclusive(thiz); + SLuint32 oldState = thiz->mState; + if (state != oldState) { #ifdef USE_OUTPUTMIXEXT - for (;; interface_cond_wait(thiz)) { + for (;; interface_cond_wait(thiz)) { // We are comparing the old state (left) vs. new state (right). // Note that the old state is 3 bits wide, but new state is only 2 bits wide. // That is why the old state is on the left and new state is on the right. - switch ((thiz->mState << 2) | state) { + switch ((oldState << 2) | state) { case (SL_PLAYSTATE_STOPPED << 2) | SL_PLAYSTATE_STOPPED: case (SL_PLAYSTATE_PAUSED << 2) | SL_PLAYSTATE_PAUSED: case (SL_PLAYSTATE_PLAYING << 2) | SL_PLAYSTATE_PLAYING: - // no-op + // no-op, and unreachable due to earlier "if (state != oldState)" break; case (SL_PLAYSTATE_STOPPED << 2) | SL_PLAYSTATE_PLAYING: case (SL_PLAYSTATE_PAUSED << 2) | SL_PLAYSTATE_PLAYING: - attr = ATTR_TRANSPORT; + attr = ATTR_PLAY_STATE; // set enqueue attribute if queue is non-empty and state becomes PLAYING if ((NULL != audioPlayer) && (audioPlayer->mBufferQueue.mFront != audioPlayer->mBufferQueue.mRear)) { + // note that USE_OUTPUTMIXEXT does not support ATTR_ABQ_ENQUEUE attr |= ATTR_BQ_ENQUEUE; } // fall through @@ -90,12 +93,14 @@ static SLresult IPlay_SetPlayState(SLPlayItf self, SLuint32 state) } break; - } + } #else - // Here life looks easy for an Android, but there are other troubles in play land - thiz->mState = state; - attr = ATTR_TRANSPORT; + // Here life looks easy for an Android, but there are other troubles in play land + thiz->mState = state; + attr = ATTR_PLAY_STATE; + // no need to set ATTR_BQ_ENQUEUE or ATTR_ABQ_ENQUEUE #endif + } // SL_LOGD("set play state %d", state); interface_unlock_exclusive_attributes(thiz, attr); } -- 2.11.0