a2dp: fix race condition during a2dp suspend and reconfig
Device lock is released during a2dp suspend and re-config scenario,
which results into race condition.
Introduce a latch lock for the following reasons.
- We don't have to hold the out->lock that is too large granularity,
if we only care about certain fields in stream structure.
latch lock is of small granularity.
- out->lock can only be held after adev->lock, which makes it impossible
to loop through the adev->usecase_list and operate on usecase streams.
latch lock can be held after out->lock and adev->lock.
CRs-Fixed: 2770070
Change-Id: I58584820f924ce4c7e723899cb2595aa3adfd5b3
diff --git a/hal/audio_hw.c b/hal/audio_hw.c
index 12e89dc..174d314 100644
--- a/hal/audio_hw.c
+++ b/hal/audio_hw.c
@@ -515,7 +515,6 @@
//cache last MBDRC cal step level
static int last_known_cal_step = -1 ;
-static int check_a2dp_restore_l(struct audio_device *adev, struct stream_out *out, bool restore);
static int out_set_compr_volume(struct audio_stream_out *stream, float left, float right);
static int out_set_mmap_volume(struct audio_stream_out *stream, float left, float right);
static int out_set_voip_volume(struct audio_stream_out *stream, float left, float right);
@@ -3364,7 +3363,7 @@
return 0;
}
-/* must be called iwth out->lock locked */
+/* must be called with out->lock and latch lock */
static void stop_compressed_output_l(struct stream_out *out)
{
out->offload_state = OFFLOAD_STATE_IDLE;
@@ -3625,9 +3624,11 @@
static int destroy_offload_callback_thread(struct stream_out *out)
{
lock_output_stream(out);
+ pthread_mutex_lock(&out->latch_lock);
stop_compressed_output_l(out);
send_offload_cmd_l(out, OFFLOAD_CMD_EXIT);
+ pthread_mutex_unlock(&out->latch_lock);
pthread_mutex_unlock(&out->lock);
pthread_join(out->offload_thread, (void **) NULL);
pthread_cond_destroy(&out->offload_cond);
@@ -3700,6 +3701,9 @@
list_remove(&uc_info->list);
out->started = 0;
+ pthread_mutex_lock(&out->latch_lock);
+ out->muted = false;
+ pthread_mutex_unlock(&out->latch_lock);
if (is_offload_usecase(out->usecase) &&
(audio_extn_passthru_is_passthrough_stream(out))) {
ALOGV("Disable passthrough , reset mixer to pcm");
@@ -4519,8 +4523,11 @@
if (adev->adm_deregister_stream)
adev->adm_deregister_stream(adev->adm_data, out->handle);
- if (is_offload_usecase(out->usecase))
+ if (is_offload_usecase(out->usecase)) {
+ pthread_mutex_lock(&out->latch_lock);
stop_compressed_output_l(out);
+ pthread_mutex_unlock(&out->latch_lock);
+ }
pthread_mutex_lock(&adev->lock);
out->standby = true;
@@ -4594,7 +4601,9 @@
// is needed e.g. when SSR happens within compress_open
// since the stream is active, offload_callback_thread is also active.
if (out->flags & AUDIO_OUTPUT_FLAG_COMPRESS_OFFLOAD) {
+ pthread_mutex_lock(&out->latch_lock);
stop_compressed_output_l(out);
+ pthread_mutex_unlock(&out->latch_lock);
}
pthread_mutex_unlock(&out->lock);
@@ -4616,8 +4625,8 @@
}
/*
- *standby implementation without locks, assumes that the callee already
- *has taken adev and out lock.
+ * standby implementation without locks, assumes that the callee already
+ * has taken adev and out lock.
*/
int out_standby_l(struct audio_stream *stream)
{
@@ -4632,8 +4641,11 @@
if (adev->adm_deregister_stream)
adev->adm_deregister_stream(adev->adm_data, out->handle);
- if (is_offload_usecase(out->usecase))
+ if (is_offload_usecase(out->usecase)) {
+ pthread_mutex_lock(&out->latch_lock);
stop_compressed_output_l(out);
+ pthread_mutex_unlock(&out->latch_lock);
+ }
out->standby = true;
if (out->usecase == USECASE_COMPRESS_VOIP_CALL) {
@@ -4980,12 +4992,13 @@
audio_extn_perf_lock_release(&adev->perf_lock_handle);
}
if ((out->flags & AUDIO_OUTPUT_FLAG_COMPRESS_OFFLOAD) &&
- out->a2dp_compress_mute &&
(!is_a2dp_out_device_type(&out->device_list) || audio_extn_a2dp_source_is_ready())) {
- pthread_mutex_lock(&out->compr_mute_lock);
- out->a2dp_compress_mute = false;
- out_set_compr_volume(&out->stream, out->volume_l, out->volume_r);
- pthread_mutex_unlock(&out->compr_mute_lock);
+ pthread_mutex_lock(&out->latch_lock);
+ if (out->a2dp_compress_mute) {
+ out->a2dp_compress_mute = false;
+ out_set_compr_volume(&out->stream, out->volume_l, out->volume_r);
+ }
+ pthread_mutex_unlock(&out->latch_lock);
} else if (out->usecase == USECASE_AUDIO_PLAYBACK_VOIP) {
out_set_voip_volume(&out->stream, out->volume_l, out->volume_r);
}
@@ -5540,7 +5553,9 @@
ALOGD("%s: called with left_vol=%f, right_vol=%f", __func__, left, right);
if (out->usecase == USECASE_AUDIO_PLAYBACK_MULTI_CH) {
/* only take left channel into account: the API is for stereo anyway */
+ pthread_mutex_lock(&out->latch_lock);
out->muted = (left == 0.0f);
+ pthread_mutex_unlock(&out->latch_lock);
return 0;
} else if (is_offload_usecase(out->usecase)) {
if (audio_extn_passthru_is_passthrough_stream(out)) {
@@ -5577,18 +5592,20 @@
out->volume_r = out_ctxt->output->volume_r;
}
}
+ pthread_mutex_lock(&out->latch_lock);
if (!out->a2dp_compress_mute) {
ret = out_set_compr_volume(&out->stream, out->volume_l, out->volume_r);
}
+ pthread_mutex_unlock(&out->latch_lock);
return ret;
} else {
- pthread_mutex_lock(&out->compr_mute_lock);
+ pthread_mutex_lock(&out->latch_lock);
ALOGV("%s: compress mute %d", __func__, out->a2dp_compress_mute);
if (!out->a2dp_compress_mute)
ret = out_set_compr_volume(stream, left, right);
out->volume_l = left;
out->volume_r = right;
- pthread_mutex_unlock(&out->compr_mute_lock);
+ pthread_mutex_unlock(&out->latch_lock);
return ret;
}
} else if (out->usecase == USECASE_AUDIO_PLAYBACK_VOIP) {
@@ -5999,7 +6016,9 @@
}
audio_extn_dts_eagle_fade(adev, true, out);
out->playback_started = 1;
+ pthread_mutex_lock(&out->latch_lock);
out->offload_state = OFFLOAD_STATE_PLAYING;
+ pthread_mutex_unlock(&out->latch_lock);
audio_extn_dts_notify_playback_state(out->usecase, 0, out->sample_rate,
popcount(out->channel_mask),
@@ -6011,8 +6030,10 @@
} else {
if (out->pcm) {
size_t bytes_to_write = bytes;
+ pthread_mutex_lock(&out->latch_lock);
if (out->muted)
memset((void *)buffer, 0, bytes);
+ pthread_mutex_unlock(&out->latch_lock);
ALOGV("%s: frames=%zu, frame_size=%zu, bytes_to_write=%zu",
__func__, frames, frame_size, bytes_to_write);
@@ -6386,6 +6407,7 @@
ALOGD("copl(%p):pause compress driver", out);
status = -ENODATA;
lock_output_stream(out);
+ pthread_mutex_lock(&out->latch_lock);
if (out->compr != NULL && out->offload_state == OFFLOAD_STATE_PLAYING) {
if (out->card_status != CARD_STATUS_OFFLINE)
status = compress_pause(out->compr);
@@ -6402,6 +6424,7 @@
out->sample_rate, popcount(out->channel_mask),
0);
}
+ pthread_mutex_unlock(&out->latch_lock);
pthread_mutex_unlock(&out->lock);
}
return status;
@@ -6416,6 +6439,7 @@
ALOGD("copl(%p):resume compress driver", out);
status = -ENODATA;
lock_output_stream(out);
+ pthread_mutex_lock(&out->latch_lock);
if (out->compr != NULL && out->offload_state == OFFLOAD_STATE_PAUSED) {
if (out->card_status != CARD_STATUS_OFFLINE) {
status = compress_resume(out->compr);
@@ -6427,6 +6451,7 @@
audio_extn_dts_notify_playback_state(out->usecase, 0, out->sample_rate,
popcount(out->channel_mask), 1);
}
+ pthread_mutex_unlock(&out->latch_lock);
pthread_mutex_unlock(&out->lock);
}
return status;
@@ -6455,12 +6480,14 @@
if (is_offload_usecase(out->usecase)) {
ALOGD("copl(%p):calling compress flush", out);
lock_output_stream(out);
+ pthread_mutex_lock(&out->latch_lock);
if (out->offload_state == OFFLOAD_STATE_PAUSED) {
stop_compressed_output_l(out);
} else {
ALOGW("%s called in invalid state %d", __func__, out->offload_state);
}
out->written = 0;
+ pthread_mutex_unlock(&out->latch_lock);
pthread_mutex_unlock(&out->lock);
ALOGD("copl(%p):out of compress flush", out);
return 0;
@@ -7735,7 +7762,7 @@
pthread_mutex_init(&out->lock, (const pthread_mutexattr_t *) NULL);
pthread_mutex_init(&out->pre_lock, (const pthread_mutexattr_t *) NULL);
- pthread_mutex_init(&out->compr_mute_lock, (const pthread_mutexattr_t *) NULL);
+ pthread_mutex_init(&out->latch_lock, (const pthread_mutexattr_t *) NULL);
pthread_mutex_init(&out->position_query_lock, (const pthread_mutexattr_t *) NULL);
pthread_cond_init(&out->cond, (const pthread_condattr_t *) NULL);
@@ -8650,6 +8677,9 @@
pthread_cond_destroy(&out->cond);
pthread_mutex_destroy(&out->lock);
+ pthread_mutex_destroy(&out->pre_lock);
+ pthread_mutex_destroy(&out->latch_lock);
+ pthread_mutex_destroy(&out->position_query_lock);
pthread_mutex_lock(&adev->lock);
streams_output_ctxt_t *out_ctxt = out_get_stream(adev, out->handle);
@@ -8712,7 +8742,7 @@
}
ret = str_parms_get_str(parms, "A2dpSuspended", value, sizeof(value));
- if (ret>=0) {
+ if (ret >= 0) {
if (!strncmp(value, "false", 5) &&
audio_extn_a2dp_source_is_suspended()) {
struct audio_usecase *usecase;
@@ -8874,30 +8904,30 @@
if (is_a2dp_out_device_type(&usecase->device_list)) {
ALOGD("reconfigure a2dp... forcing device switch");
- pthread_mutex_unlock(&adev->lock);
- lock_output_stream(usecase->stream.out);
- pthread_mutex_lock(&adev->lock);
audio_extn_a2dp_set_handoff_mode(true);
ALOGD("Switching to speaker and muting the stream before select_devices");
check_a2dp_restore_l(adev, usecase->stream.out, false);
//force device switch to re configure encoder
select_devices(adev, usecase->id);
ALOGD("Unmuting the stream after select_devices");
+ pthread_mutex_lock(&usecase->stream.out->latch_lock);
usecase->stream.out->a2dp_compress_mute = false;
- out_set_compr_volume(&usecase->stream.out->stream, usecase->stream.out->volume_l, usecase->stream.out->volume_r);
+ out_set_compr_volume(&usecase->stream.out->stream,
+ usecase->stream.out->volume_l,
+ usecase->stream.out->volume_r);
+ pthread_mutex_unlock(&usecase->stream.out->latch_lock);
audio_extn_a2dp_set_handoff_mode(false);
- pthread_mutex_unlock(&usecase->stream.out->lock);
break;
- } else if ((usecase->stream.out->flags & AUDIO_OUTPUT_FLAG_COMPRESS_OFFLOAD) &&
- usecase->stream.out->a2dp_compress_mute) {
- pthread_mutex_unlock(&adev->lock);
- lock_output_stream(usecase->stream.out);
- pthread_mutex_lock(&adev->lock);
- reassign_device_list(&usecase->stream.out->device_list,
- AUDIO_DEVICE_OUT_BLUETOOTH_A2DP, "");
- check_a2dp_restore_l(adev, usecase->stream.out, true);
- pthread_mutex_unlock(&usecase->stream.out->lock);
- break;
+ } else if (usecase->stream.out->flags & AUDIO_OUTPUT_FLAG_COMPRESS_OFFLOAD) {
+ pthread_mutex_lock(&usecase->stream.out->latch_lock);
+ if (usecase->stream.out->a2dp_compress_mute) {
+ pthread_mutex_unlock(&usecase->stream.out->latch_lock);
+ reassign_device_list(&usecase->stream.out->device_list,
+ AUDIO_DEVICE_OUT_BLUETOOTH_A2DP, "");
+ check_a2dp_restore_l(adev, usecase->stream.out, true);
+ break;
+ }
+ pthread_mutex_unlock(&usecase->stream.out->latch_lock);
}
}
}
@@ -9717,6 +9747,9 @@
} else
in_standby(&stream->common);
+ pthread_mutex_destroy(&in->lock);
+ pthread_mutex_destroy(&in->pre_lock);
+
pthread_mutex_lock(&adev->lock);
if (in->usecase == USECASE_AUDIO_RECORD) {
adev->pcm_record_uc_state = 0;
@@ -10321,8 +10354,8 @@
return;
}
-/* out and adev lock held */
-static int check_a2dp_restore_l(struct audio_device *adev, struct stream_out *out, bool restore)
+/* adev lock held */
+int check_a2dp_restore_l(struct audio_device *adev, struct stream_out *out, bool restore)
{
struct audio_usecase *uc_info;
float left_p;
@@ -10341,23 +10374,26 @@
out->usecase, use_case_table[out->usecase]);
if (restore) {
+ pthread_mutex_lock(&out->latch_lock);
// restore A2DP device for active usecases and unmute if required
if (is_a2dp_out_device_type(&out->device_list)) {
ALOGD("%s: restoring A2dp and unmuting stream", __func__);
if (uc_info->out_snd_device != SND_DEVICE_OUT_BT_A2DP)
select_devices(adev, uc_info->id);
- pthread_mutex_lock(&out->compr_mute_lock);
if ((out->flags & AUDIO_OUTPUT_FLAG_COMPRESS_OFFLOAD) &&
- (out->a2dp_compress_mute) && (uc_info->out_snd_device == SND_DEVICE_OUT_BT_A2DP)) {
- out->a2dp_compress_mute = false;
- out_set_compr_volume(&out->stream, out->volume_l, out->volume_r);
+ (uc_info->out_snd_device == SND_DEVICE_OUT_BT_A2DP)) {
+ if (out->a2dp_compress_mute) {
+ out->a2dp_compress_mute = false;
+ out_set_compr_volume(&out->stream, out->volume_l, out->volume_r);
+ }
}
- pthread_mutex_unlock(&out->compr_mute_lock);
}
+ out->muted = false;
+ pthread_mutex_unlock(&out->latch_lock);
} else {
+ pthread_mutex_lock(&out->latch_lock);
if (out->flags & AUDIO_OUTPUT_FLAG_COMPRESS_OFFLOAD) {
// mute compress stream if suspended
- pthread_mutex_lock(&out->compr_mute_lock);
if (!out->a2dp_compress_mute && !out->standby) {
ALOGD("%s: selecting speaker and muting stream", __func__);
assign_devices(&devices, &out->device_list);
@@ -10375,31 +10411,18 @@
out->volume_l = left_p;
out->volume_r = right_p;
}
- pthread_mutex_unlock(&out->compr_mute_lock);
} else {
- // tear down a2dp path for non offloaded streams
- if (audio_extn_a2dp_source_is_suspended())
- out_standby_l(&out->stream.common);
+ // mute for non offloaded streams
+ if (audio_extn_a2dp_source_is_suspended()) {
+ out->muted = true;
+ }
}
+ pthread_mutex_unlock(&out->latch_lock);
}
ALOGV("%s: exit", __func__);
return 0;
}
-int check_a2dp_restore(struct audio_device *adev, struct stream_out *out, bool restore)
-{
- int ret = 0;
-
- lock_output_stream(out);
- pthread_mutex_lock(&adev->lock);
-
- ret = check_a2dp_restore_l(adev, out, restore);
-
- pthread_mutex_unlock(&adev->lock);
- pthread_mutex_unlock(&out->lock);
- return ret;
-}
-
void adev_on_battery_status_changed(bool charging)
{
pthread_mutex_lock(&adev->lock);