audio: fix audio glitch when closing pcm stream
Holding the audio device mutex while calling pcm_close() is not necessary
and will cause writes on other output streams to be blocked until close
completes which can take several hundred milliseconds.
Not holding the audio device mutex during the whole standby sequence
forces to change the lock order between audio device and output stream mutex.
The result is that we do not acquire the audio device mutex systematically before
the stream mutex in out_write(). This is not a problem with this audio HAL
as set_mode() does not acquire the stream mutex and out_set_parameters() is always
called in the same thread (same priority) as out_write().
Same change done for input threads.
Bug 8267567.
Change-Id: I17bb187c0564200f6362586885e61500d52d5bc2
diff --git a/hal/audio_hw.c b/hal/audio_hw.c
index 8b0bf97..42bbc59 100644
--- a/hal/audio_hw.c
+++ b/hal/audio_hw.c
@@ -832,20 +832,14 @@
return -EINVAL;
}
- /* 1. Close the PCM device first */
- if (in->pcm) {
- pcm_close(in->pcm);
- in->pcm = NULL;
- }
-
- /* 2. Disable stream specific mixer controls */
+ /* 1. Disable stream specific mixer controls */
in_snd_device = adev->cur_in_snd_device;
disable_audio_route(adev->audio_route, in->usecase, in_snd_device);
audio_route_update_mixer(adev->audio_route);
remove_usecase_from_list(adev, in->usecase);
- /* 3. Disable the tx device */
+ /* 2. Disable the tx device */
select_devices(adev);
ALOGD("%s: exit: status(%d)", __func__, ret);
@@ -940,20 +934,14 @@
return -EINVAL;
}
- /* 1. Close the PCM device first */
- if (out->pcm) {
- pcm_close(out->pcm);
- out->pcm = NULL;
- }
-
- /* 2. Get and set stream specific mixer controls */
+ /* 1. Get and set stream specific mixer controls */
out_snd_device = adev->cur_out_snd_device;
disable_audio_route(adev->audio_route, out->usecase, out_snd_device);
audio_route_update_mixer(adev->audio_route);
remove_usecase_from_list(adev, uc_info->id);
- /* 3. Disable the rx device */
+ /* 2. Disable the rx device */
adev->out_device = get_active_out_devices(adev, out->usecase);
adev->out_device |= get_voice_call_out_device(adev);
ret = select_devices(adev);
@@ -1254,15 +1242,19 @@
struct stream_out *out = (struct stream_out *)stream;
struct audio_device *adev = out->dev;
ALOGD("%s: enter: usecase(%d)", __func__, out->usecase);
- pthread_mutex_lock(&out->dev->lock);
pthread_mutex_lock(&out->lock);
if (!out->standby) {
out->standby = true;
+ if (out->pcm) {
+ pcm_close(out->pcm);
+ out->pcm = NULL;
+ }
+ pthread_mutex_lock(&adev->lock);
stop_output_stream(out);
+ pthread_mutex_unlock(&adev->lock);
}
pthread_mutex_unlock(&out->lock);
- pthread_mutex_unlock(&out->dev->lock);
ALOGD("%s: exit", __func__);
return 0;
}
@@ -1286,8 +1278,8 @@
ret = str_parms_get_str(parms, AUDIO_PARAMETER_STREAM_ROUTING, value, sizeof(value));
if (ret >= 0) {
val = atoi(value);
- pthread_mutex_lock(&adev->lock);
pthread_mutex_lock(&out->lock);
+ pthread_mutex_lock(&adev->lock);
if (adev->mode == AUDIO_MODE_IN_CALL && !adev->in_call && (val != 0)) {
adev->out_device = get_active_out_devices(adev, out->usecase) | val;
@@ -1307,8 +1299,8 @@
out->devices = val;
}
- pthread_mutex_unlock(&out->lock);
pthread_mutex_unlock(&adev->lock);
+ pthread_mutex_unlock(&out->lock);
}
str_parms_destroy(parms);
ALOGD("%s: exit: code(%d)", __func__, ret);
@@ -1374,21 +1366,16 @@
struct audio_device *adev = out->dev;
int i, ret = -1;
- /* acquiring hw device mutex systematically is useful if a low priority thread is waiting
- * on the output stream mutex - e.g. executing select_mode() while holding the hw device
- * mutex
- */
- pthread_mutex_lock(&adev->lock);
pthread_mutex_lock(&out->lock);
if (out->standby) {
+ pthread_mutex_lock(&adev->lock);
ret = start_output_stream(out);
+ pthread_mutex_unlock(&adev->lock);
if (ret != 0) {
- pthread_mutex_unlock(&adev->lock);
goto exit;
}
out->standby = false;
}
- pthread_mutex_unlock(&adev->lock);
if (out->pcm) {
//ALOGV("%s: writing buffer (%d bytes) to pcm device", __func__, bytes);
@@ -1471,14 +1458,18 @@
struct audio_device *adev = in->dev;
int status = 0;
ALOGD("%s: enter", __func__);
- pthread_mutex_lock(&adev->lock);
pthread_mutex_lock(&in->lock);
if (!in->standby) {
in->standby = true;
+ if (in->pcm) {
+ pcm_close(in->pcm);
+ in->pcm = NULL;
+ }
+ pthread_mutex_lock(&adev->lock);
status = stop_input_stream(in);
+ pthread_mutex_unlock(&adev->lock);
}
pthread_mutex_unlock(&in->lock);
- pthread_mutex_unlock(&adev->lock);
ALOGD("%s: exit: status(%d)", __func__, status);
return status;
}
@@ -1502,8 +1493,8 @@
ret = str_parms_get_str(parms, AUDIO_PARAMETER_STREAM_INPUT_SOURCE, value, sizeof(value));
- pthread_mutex_lock(&adev->lock);
pthread_mutex_lock(&in->lock);
+ pthread_mutex_lock(&adev->lock);
if (ret >= 0) {
val = atoi(value);
/* no audio source uses val == 0 */
@@ -1524,8 +1515,8 @@
}
}
- pthread_mutex_unlock(&in->lock);
pthread_mutex_unlock(&adev->lock);
+ pthread_mutex_unlock(&in->lock);
str_parms_destroy(parms);
ALOGD("%s: exit: status(%d)", __func__, ret);
@@ -1550,22 +1541,17 @@
struct audio_device *adev = in->dev;
int i, ret = -1;
- /* acquiring hw device mutex systematically is useful if a low priority thread is waiting
- * on the output stream mutex - e.g. executing select_mode() while holding the hw device
- * mutex
- */
//ALOGV("%s: buffer(%p) bytes(%d)", __func__, buffer, bytes);
- pthread_mutex_lock(&adev->lock);
pthread_mutex_lock(&in->lock);
if (in->standby) {
+ pthread_mutex_lock(&adev->lock);
ret = start_input_stream(in);
+ pthread_mutex_unlock(&adev->lock);
if (ret != 0) {
- pthread_mutex_unlock(&adev->lock);
goto exit;
}
in->standby = 0;
}
- pthread_mutex_unlock(&adev->lock);
if (in->pcm) {
ret = pcm_read(in->pcm, buffer, bytes);