From 4d9c2bb1eef39d3774b7129a280cb4d0c6509d81 Mon Sep 17 00:00:00 2001 From: Edmunt Pienkowsky Date: Wed, 19 Jun 2024 10:58:03 +0200 Subject: [PATCH] Improve ALSA capture Improve locking/unlocking of private data. --- src/chan_quectel.c | 28 +++++++++++++++--------- src/chan_quectel.h | 3 ++- src/channel.c | 52 ++++++++++++++++++++++---------------------- src/cli.c | 2 +- src/cpvt.c | 34 ++++++++--------------------- src/cpvt.h | 7 ++---- src/monitor_thread.c | 2 +- src/pcm.c | 2 +- 8 files changed, 60 insertions(+), 70 deletions(-) diff --git a/src/chan_quectel.c b/src/chan_quectel.c index a49976b4..5bb31b00 100644 --- a/src/chan_quectel.c +++ b/src/chan_quectel.c @@ -302,7 +302,7 @@ static void pvt_finish(struct pvt* const pvt) static int pvt_finish_cb(void* obj, attribute_unused void* arg, attribute_unused int flags) { - SCOPED_AO2LOCK(pvt_lock, obj); + SCOPED_AO2LOCK(pvtl, obj); struct pvt* const pvt = obj; pvt_monitor_stop(pvt); at_queue_flush(pvt); @@ -311,7 +311,7 @@ static int pvt_finish_cb(void* obj, attribute_unused void* arg, attribute_unused static void pvt_destroy(void* obj) { - SCOPED_AO2LOCK(pvt_lock, obj); + SCOPED_AO2LOCK(pvtl, obj); struct pvt* const pvt = (struct pvt* const)obj; ast_string_field_free_memory(pvt); } @@ -483,7 +483,7 @@ void pvt_on_create_1st_channel(struct pvt* pvt) { const struct ast_format* const fmt = pvt_get_audio_format(pvt); const size_t silence_buf_size = 2u * pvt_get_audio_frame_size(PTIME_PLAYBACK, fmt); - pvt->silence_buf = ast_calloc(1, silence_buf_size + AST_FRIENDLY_OFFSET); + pvt->silence_buf = ast_calloc(1, silence_buf_size); if (CONF_SHARED(pvt, multiparty)) { if (CONF_UNIQ(pvt, uac) > TRIBOOL_FALSE) { @@ -635,13 +635,21 @@ static int can_send_message(struct pvt* pvt, attribute_unused unsigned int opts) return 1; } -void pvt_unlock(struct pvt* const pvt) +int pvt_lock(struct pvt* const pvt) { if (!pvt) { - return; + return -1; + } + return AO2_REF_AND_LOCK(pvt); +} + +int pvt_unlock(struct pvt* const pvt) +{ + if (!pvt) { + return -1; } - AO2_UNLOCK_AND_UNREF(pvt); + return AO2_UNLOCK_AND_UNREF(pvt); } int pvt_taskproc_trylock_and_execute(struct pvt* pvt, void (*task_exe)(struct pvt* pvt), const char* task_name) @@ -757,7 +765,7 @@ void* get_rr_next(struct ao2_iterator* i, const struct pvt_test_fn* const fn, vo int last_used_found = 0; while ((obj = ao2_iterator_next(i))) { if (last_used_found) { - SCOPED_AO2LOCK(pvt_lock, obj); + SCOPED_AO2LOCK(pvtl, obj); if (call_pvt_test_fn(fn, obj)) { break; } @@ -782,7 +790,7 @@ void* get_rr_next(struct ao2_iterator* i, const struct pvt_test_fn* const fn, vo } { - SCOPED_AO2LOCK(pvt_lock, obj); + SCOPED_AO2LOCK(pvtl, obj); if (call_pvt_test_fn(fn, obj)) { break; } @@ -1322,7 +1330,7 @@ int pvt_set_act(struct pvt* pvt, int act) static int pvt_mark_must_remove_cb(void* obj, attribute_unused void* arg, attribute_unused int flags) { - SCOPED_AO2LOCK(pvt_lock, obj); + SCOPED_AO2LOCK(pvtl, obj); struct pvt* pvt = obj; pvt->must_remove = 1; return 0; @@ -1494,7 +1502,7 @@ size_t pvt_get_audio_frame_size(unsigned int ptime, const struct ast_format* con #endif -void* pvt_get_silence_buffer(struct pvt* const pvt) { return pvt->silence_buf + AST_FRIENDLY_OFFSET; } +void* pvt_get_silence_buffer(struct pvt* const pvt) { return pvt->silence_buf; } int pvt_direct_write(struct pvt* pvt, const char* buf, size_t count) { diff --git a/src/chan_quectel.h b/src/chan_quectel.h index 9e501029..75122b8e 100644 --- a/src/chan_quectel.h +++ b/src/chan_quectel.h @@ -251,7 +251,8 @@ struct cpvt* pvt_channel_find_by_call_idx(struct pvt* pvt, int call_idx); struct cpvt* pvt_channel_find_active(struct pvt* pvt); struct cpvt* pvt_channel_find_last_initialized(struct pvt* pvt); -void pvt_unlock(struct pvt* const pvt); +int pvt_lock(struct pvt* const pvt); +int pvt_unlock(struct pvt* const pvt); int pvt_taskproc_trylock_and_execute(struct pvt* pvt, void (*task_exe)(struct pvt* pvt), const char* task_name); #define PVT_TASKPROC_TRYLOCK_AND_EXECUTE(p, t) pvt_taskproc_trylock_and_execute(p, t, #t) diff --git a/src/channel.c b/src/channel.c index a33d2d62..42357b50 100644 --- a/src/channel.c +++ b/src/channel.c @@ -225,8 +225,7 @@ static int channel_call(struct ast_channel* channel, const char* dest, attribute return -1; } - struct pvt* const pvt = cpvt->pvt; - char* const dest_dev = ast_strdupa(dest); + char* const dest_dev = ast_strdupa(dest); const char* dest_num; int opts; @@ -240,7 +239,8 @@ static int channel_call(struct ast_channel* channel, const char* dest, attribute return -1; } - SCOPED_AO2LOCK(pvt_lock, pvt); + SCOPED_CPVT(cpvtl, cpvt); + struct pvt* const pvt = cpvt->pvt; // FIXME: check if bridged on same device with CALL_FLAG_HOLD_OTHER if (!pvt_ready4voice_call(pvt, cpvt, opts)) { @@ -283,10 +283,9 @@ static int channel_hangup(struct ast_channel* channel) /* its possible call with channel w/o tech_pvt */ if (cpvt && cpvt->channel == channel && cpvt->pvt) { + SCOPED_CPVT(cpvtl, cpvt); struct pvt* const pvt = cpvt->pvt; - SCOPED_AO2LOCK(pvt_lock, pvt); - const int need_hangup = CPVT_TEST_FLAG(cpvt, CALL_FLAG_NEED_HANGUP) ? 1 : 0; const int hangup_cause = ast_channel_hangupcause(channel); @@ -322,9 +321,9 @@ static int channel_answer(struct ast_channel* channel) ast_log(LOG_WARNING, "call on unreferenced %s\n", ast_channel_name(channel)); return 0; } - struct pvt* const pvt = cpvt->pvt; - SCOPED_AO2LOCK(pvt_lock, pvt); + SCOPED_CPVT(cpvtl, cpvt); + struct pvt* const pvt = cpvt->pvt; if (CPVT_DIR_INCOMING(cpvt)) { if (at_enqueue_answer(cpvt)) { @@ -345,10 +344,9 @@ static int channel_digit_begin(struct ast_channel* channel, char digit) ast_log(LOG_WARNING, "Call on unreferenced %s\n", ast_channel_name(channel)); return -1; } + SCOPED_CPVT(cpvtl, cpvt); struct pvt* const pvt = cpvt->pvt; - SCOPED_AO2LOCK(pvt_lock, pvt); - const int rv = at_enqueue_dtmf(cpvt, digit); if (rv) { if (rv == -1974) { @@ -531,14 +529,23 @@ static struct ast_frame* channel_read_uac(struct cpvt* cpvt, struct pvt* pvt, si return NULL; } +#if 1 const snd_pcm_sframes_t avail_frames = snd_pcm_avail_update(pvt->icard); if (avail_frames < 0) { - ast_log(LOG_ERROR, "[%s][ALSA][CAPTURE] Cannot determine available samples: %s\n", PVT_ID(pvt), snd_strerror((int)avail_frames)); + ast_log(LOG_ERROR, "[%s][ALSA][CAPTURE] Cannot determine number of available audio frames: %s\n", PVT_ID(pvt), snd_strerror((int)avail_frames)); return NULL; } else if (frames > (size_t)avail_frames) { - ast_log(LOG_WARNING, "[%s][ALSA][CAPTURE] Not enough samples: %d/%d\n", PVT_ID(pvt), (int)avail_frames, (int)frames); + ast_log(LOG_WARNING, "[%s][ALSA][CAPTURE][F:%d] Not enough audio frames: %d\n", PVT_ID(pvt), (int)frames, (int)avail_frames); return NULL; + } else { + const snd_pcm_sframes_t limit = 4 * frames; + if (avail_frames >= limit) { + const snd_pcm_sframes_t skipped = snd_pcm_forward(pvt->icard, avail_frames - (2 * frames)); + ast_log(LOG_NOTICE, "[%s][ALSA][CAPTURE][F:%d] Too many audio frames available: %d, skipped %d\n", PVT_ID(pvt), (int)frames, (int)avail_frames, + (int)skipped); + } } +#endif void* const buf = cpvt_get_buffer(cpvt); const int res = snd_pcm_mmap_readi(pvt->icard, buf, frames); @@ -580,8 +587,6 @@ static struct ast_frame* channel_read_uac(struct cpvt* cpvt, struct pvt* pvt, si return NULL; } -#define subclass_integer subclass.integer - static struct ast_frame* channel_read(struct ast_channel* channel) { struct cpvt* const cpvt = ast_channel_tech_pvt(channel); @@ -591,8 +596,8 @@ static struct ast_frame* channel_read(struct ast_channel* channel) return &ast_null_frame; } + SCOPED_CPVT(cpvtl, cpvt); struct pvt* const pvt = cpvt->pvt; - SCOPED_CPVT_TL(cpvt_lock, cpvt); ast_debug(8, "[%s] Read - idx:%d state:%s audio_fd:%d\n", PVT_ID(pvt), cpvt->call_idx, call_state2str(cpvt->state), pvt->audio_fd); @@ -807,8 +812,8 @@ static int channel_write(struct ast_channel* channel, struct ast_frame* f) return 0; } + SCOPED_CPVT(cpvtl, cpvt); struct pvt* const pvt = cpvt->pvt; - SCOPED_CPVT_TL(cpvt_lock, cpvt); const struct ast_format* const fmt = pvt_get_audio_format(pvt); const size_t frame_size = pvt_get_audio_frame_size(PTIME_PLAYBACK, fmt); @@ -836,9 +841,6 @@ static int channel_write(struct ast_channel* channel, struct ast_frame* f) return res >= 0 ? 0 : -1; } -#undef subclass_integer -#undef subclass_codec - #/* */ static int channel_fixup(struct ast_channel* oldchannel, struct ast_channel* newchannel) @@ -850,9 +852,7 @@ static int channel_fixup(struct ast_channel* oldchannel, struct ast_channel* new return -1; } - struct pvt* const pvt = cpvt->pvt; - - SCOPED_AO2LOCK(pvt_lock, pvt); + SCOPED_CPVT(cpvtl, cpvt); if (cpvt->channel == oldchannel) { cpvt->channel = newchannel; @@ -928,7 +928,7 @@ static int channel_indicate(struct ast_channel* channel, int condition, const vo if (!pvt || CONF_SHARED(pvt, moh)) { ast_moh_start(channel, data, NULL); } else { - SCOPED_AO2LOCK(pvt_lock, pvt); + SCOPED_CPVT(cpvtl, cpvt); at_enqueue_mute(cpvt, 1); } break; @@ -937,14 +937,14 @@ static int channel_indicate(struct ast_channel* channel, int condition, const vo if (!pvt || CONF_SHARED(pvt, moh)) { ast_moh_stop(channel); } else { - SCOPED_AO2LOCK(pvt_lock, pvt); + SCOPED_CPVT(cpvtl, cpvt); at_enqueue_mute(cpvt, 0); } break; case AST_CONTROL_CONNECTED_LINE: { struct ast_party_connected_line* const cnncd = ast_channel_connected(channel); - SCOPED_AO2LOCK(pvt_lock, pvt); + SCOPED_CPVT(cpvtl, cpvt); ast_log(LOG_NOTICE, "[%s] Connected party is now %s <%s>\n", PVT_ID(pvt), S_COR(cnncd->id.name.valid, cnncd->id.name.str, ""), S_COR(cnncd->id.number.valid, cnncd->id.number.str, "")); break; @@ -1150,7 +1150,7 @@ static int channel_func_read(struct ast_channel* channel, attribute_unused const } if (!strcasecmp(data, "callstate")) { - SCOPED_CPVT_TL(cpvt_lock, cpvt); + SCOPED_CPVT(cpvtl, cpvt); call_state_t state = cpvt->state; ast_copy_string(buf, call_state2str(state), len); } else { @@ -1181,7 +1181,7 @@ static int channel_func_write(struct ast_channel* channel, const char* function, return -1; } - SCOPED_CPVT_TL(cpvt_lock, cpvt); + SCOPED_CPVT(cpvtl, cpvt); oldstate = cpvt->state; if (oldstate == newstate) diff --git a/src/cli.c b/src/cli.c index 9329c707..1672fcf7 100644 --- a/src/cli.c +++ b/src/cli.c @@ -94,7 +94,7 @@ static char* cli_show_devices(struct ast_cli_entry* e, int cmd, struct ast_cli_a struct ao2_iterator i = ao2_iterator_init(gpublic->pvts, 0); while ((pvt = ao2_iterator_next(&i))) { - SCOPED_AO2LOCK(pvt_lock, pvt); + SCOPED_AO2LOCK(pvtl, pvt); ast_cli(a->fd, FORMAT2, PVT_ID(pvt), CONF_SHARED(pvt, group), pvt_str_state(pvt), pvt->rssi, pvt->act, pvt->provider_name, pvt->model, pvt->firmware, pvt->imei, pvt->imsi, pvt->subscriber_number); } diff --git a/src/cpvt.c b/src/cpvt.c index 33ca4230..367ca7f3 100644 --- a/src/cpvt.c +++ b/src/cpvt.c @@ -71,14 +71,14 @@ struct cpvt* cpvt_alloc(struct pvt* pvt, int call_idx, unsigned dir, call_state_ } const struct ast_format* const fmt = pvt_get_audio_format(pvt); - const size_t buffer_size = pvt_get_audio_frame_size(PTIME_PLAYBACK, fmt); + const size_t buffer_size = pvt_get_audio_frame_size(PTIME_CAPTURE, fmt); cpvt->pvt = pvt; cpvt->call_idx = call_idx; cpvt->state = state; cpvt->rd_pipe[0] = fd[0]; cpvt->rd_pipe[1] = fd[1]; - cpvt->buffer = ast_calloc(1, buffer_size + AST_FRIENDLY_OFFSET); + cpvt->buffer = ast_calloc(1, buffer_size); CPVT_SET_DIRECTION(cpvt, dir); CPVT_SET_LOCAL(cpvt, local_channel); @@ -376,42 +376,27 @@ int cpvt_change_state(struct cpvt* const cpvt, call_state_t newstate, int cause) int cpvt_lock(struct cpvt* const cpvt) { - struct pvt* const pvt = cpvt->pvt; - if (!pvt) { + if (!cpvt) { return -1; } - return AO2_REF_AND_LOCK(pvt); -} - -void cpvt_try_lock(struct cpvt* const cpvt) -{ struct pvt* const pvt = cpvt->pvt; if (!pvt) { - return; - } - - struct ast_channel* const channel = cpvt->channel; - if (!channel) { - return; + return -1; } - ao2_ref(pvt, 1); - - while (ao2_trylock(pvt)) { - CHANNEL_DEADLOCK_AVOIDANCE(channel); - } + return pvt_lock(pvt); } -void cpvt_unlock(struct cpvt* const cpvt) +int cpvt_unlock(struct cpvt* const cpvt) { if (!cpvt) { - return; + return -1; } - pvt_unlock(cpvt->pvt); + return pvt_unlock(cpvt->pvt); } -void* cpvt_get_buffer(struct cpvt* const cpvt) { return cpvt->buffer + AST_FRIENDLY_OFFSET; } +void* cpvt_get_buffer(struct cpvt* const cpvt) { return cpvt->buffer; } struct ast_frame* cpvt_prepare_voice_frame(struct cpvt* const cpvt, void* const buf, int samples, const struct ast_format* const fmt) { @@ -424,7 +409,6 @@ struct ast_frame* cpvt_prepare_voice_frame(struct cpvt* const cpvt, void* const f->samples = samples; f->datalen = samples * sizeof(int16_t); f->data.ptr = buf; - f->offset = AST_FRIENDLY_OFFSET; f->src = AST_MODULE; ast_frame_byteswap_le(f); diff --git a/src/cpvt.h b/src/cpvt.h index 7cb789fb..c44809f8 100644 --- a/src/cpvt.h +++ b/src/cpvt.h @@ -101,8 +101,8 @@ struct cpvt* cpvt_alloc(struct pvt* pvt, int call_idx, unsigned dir, call_state_ void cpvt_free(struct cpvt* cpvt); int cpvt_lock(struct cpvt* const); -void cpvt_try_lock(struct cpvt* const); -void cpvt_unlock(struct cpvt* const); +int cpvt_unlock(struct cpvt* const); +#define SCOPED_CPVT(varname, lock) SCOPED_LOCK(varname, lock, cpvt_lock, cpvt_unlock) void cpvt_call_activate(struct cpvt* const cpvt); void cpvt_call_disactivate(struct cpvt* const cpvt); @@ -110,9 +110,6 @@ void cpvt_call_disactivate(struct cpvt* const cpvt); int cpvt_control(const struct cpvt* const cpvt, enum ast_control_frame_type control); int cpvt_change_state(struct cpvt* const cpvt, call_state_t newstate, int cause); -#define SCOPED_CPVT(varname, lock) SCOPED_LOCK(varname, lock, cpvt_lock, cpvt_unlock) -#define SCOPED_CPVT_TL(varname, lock) SCOPED_LOCK(varname, lock, cpvt_try_lock, cpvt_unlock) - void* cpvt_get_buffer(struct cpvt* const cpvt); struct ast_frame* cpvt_prepare_voice_frame(struct cpvt* const cpvt, void* const buf, int samples, const struct ast_format* const fmt); struct ast_frame* cpvt_prepare_silence_voice_frame(struct cpvt* const cpvt, int samples, const struct ast_format* const fmt); diff --git a/src/monitor_thread.c b/src/monitor_thread.c index 2aeff660..8d4be888 100644 --- a/src/monitor_thread.c +++ b/src/monitor_thread.c @@ -349,7 +349,7 @@ void pvt_monitor_stop(struct pvt* pvt) { const pthread_t id = pvt->monitor_thread; - SCOPED_LOCK(pvt_lock, pvt, ao2_unlock, ao2_lock); // scoped UNlock + SCOPED_LOCK(pvtl, pvt, ao2_unlock, ao2_lock); // scoped UNlock pthread_join(id, NULL); } diff --git a/src/pcm.c b/src/pcm.c index 3cfe6d27..37114f93 100644 --- a/src/pcm.c +++ b/src/pcm.c @@ -99,7 +99,7 @@ int pcm_init(const char* dev, snd_pcm_stream_t stream, const struct ast_format* #else const size_t ptime = (stream == SND_PCM_STREAM_CAPTURE) ? PTIME_CAPTURE : PTIME_PLAYBACK; #endif - snd_pcm_uframes_t period_size = adjust_uframes(ptime, rate); + snd_pcm_uframes_t period_size = adjust_uframes(ptime + 10, rate); snd_pcm_uframes_t buffer_size = adjust_uframes(PTIME_BUFFER, rate); snd_pcm_uframes_t start_threshold = adjust_uframes(adjust_start_threshold(ptime * 2), rate); snd_pcm_uframes_t stop_threshold = buffer_size - period_size;