[Asterisk-code-review] audiohook.c: Difference in read/write rates caused continuou... (asterisk[master])
Matt Jordan
asteriskteam at digium.com
Thu May 21 07:22:21 CDT 2015
Matt Jordan has submitted this change and it was merged.
Change subject: audiohook.c: Difference in read/write rates caused continuous buffer resets
......................................................................
audiohook.c: Difference in read/write rates caused continuous buffer resets
Currently, everytime a sample rate change occurs (on read or write) the
associated factory buffers are reset. If the requested sample rate on a
read differed from that of a write then the buffers are continually reset
on every read and write. This has the side effect of emptying the buffer,
thus there being no data to read and then write to a file in the case of
call recording.
This patch fixes it so that an audiohook_list's rate always maintains the
maximum sample rate among hooks and formats. Audiohook sample rates are
only overwritten by this value when slin native compatibility is turned on.
Also, the audiohook sample rate can only overwrite the list's sample rate
when its rate is greater than that of the list or if compatibility is
turned off. This keeps the rate from constantly switching/resetting.
ASTERISK-24944 #close
Reported by: Ronald Raikes
Change-Id: Idab4dfef068a7922c09cc631dda27bc920a6c76f
---
M include/asterisk/audiohook.h
M main/audiohook.c
2 files changed, 109 insertions(+), 23 deletions(-)
Approvals:
Matt Jordan: Looks good to me, approved; Verified
Joshua Colp: Looks good to me, but someone else must approve
diff --git a/include/asterisk/audiohook.h b/include/asterisk/audiohook.h
index 375b2dd..cae8cc0 100644
--- a/include/asterisk/audiohook.h
+++ b/include/asterisk/audiohook.h
@@ -63,6 +63,7 @@
AST_AUDIOHOOK_SMALL_QUEUE = (1 << 4),
AST_AUDIOHOOK_MUTE_READ = (1 << 5), /*!< audiohook should be mute frames read */
AST_AUDIOHOOK_MUTE_WRITE = (1 << 6), /*!< audiohook should be mute frames written */
+ AST_AUDIOHOOK_COMPATIBLE = (1 << 7), /*!< is the audiohook native slin compatible */
};
enum ast_audiohook_init_flags {
diff --git a/main/audiohook.c b/main/audiohook.c
index b754f23..3e233fa 100644
--- a/main/audiohook.c
+++ b/main/audiohook.c
@@ -46,6 +46,8 @@
#define AST_AUDIOHOOK_SYNC_TOLERANCE 100 /*!< Tolerance in milliseconds for audiohooks synchronization */
#define AST_AUDIOHOOK_SMALL_QUEUE_TOLERANCE 100 /*!< When small queue is enabled, this is the maximum amount of audio that can remain queued at a time. */
+#define DEFAULT_INTERNAL_SAMPLE_RATE 8000
+
struct ast_audiohook_translate {
struct ast_trans_pvt *trans_pvt;
struct ast_format *format;
@@ -117,7 +119,7 @@
audiohook->init_flags = init_flags;
/* initialize internal rate at 8khz, this will adjust if necessary */
- audiohook_set_internal_rate(audiohook, 8000, 0);
+ audiohook_set_internal_rate(audiohook, DEFAULT_INTERNAL_SAMPLE_RATE, 0);
/* Since we are just starting out... this audiohook is new */
ast_audiohook_update_status(audiohook, AST_AUDIOHOOK_STATUS_NEW);
@@ -361,7 +363,19 @@
struct ast_frame *read_frame = NULL, *final_frame = NULL;
struct ast_format *slin;
- audiohook_set_internal_rate(audiohook, ast_format_get_sample_rate(format), 1);
+ /*
+ * Update the rate if compatibility mode is turned off or if it is
+ * turned on and the format rate is higher than the current rate.
+ *
+ * This makes it so any unnecessary rate switching/resetting does
+ * not take place and also any associated audiohook_list's internal
+ * sample rate maintains the highest sample rate between hooks.
+ */
+ if (!ast_test_flag(audiohook, AST_AUDIOHOOK_COMPATIBLE) ||
+ (ast_test_flag(audiohook, AST_AUDIOHOOK_COMPATIBLE) &&
+ ast_format_get_sample_rate(format) > audiohook->hook_internal_samp_rate)) {
+ audiohook_set_internal_rate(audiohook, ast_format_get_sample_rate(format), 1);
+ }
if (!(read_frame = (direction == AST_AUDIOHOOK_DIRECTION_BOTH ?
audiohook_read_frame_both(audiohook, samples, read_reference, write_reference) :
@@ -425,6 +439,22 @@
static void audiohook_list_set_samplerate_compatibility(struct ast_audiohook_list *audiohook_list)
{
struct ast_audiohook *ah = NULL;
+
+ /*
+ * Anytime the samplerate compatibility is set (attach/remove an audiohook) the
+ * list's internal sample rate needs to be reset so that the next time processing
+ * through write_list, if needed, it will get updated to the correct rate.
+ *
+ * A list's internal rate always chooses the higher between its own rate and a
+ * given rate. If the current rate is being driven by an audiohook that wanted a
+ * higher rate then when this audiohook is removed the list's rate would remain
+ * at that level when it should be lower, and with no way to lower it since any
+ * rate compared against it would be lower.
+ *
+ * By setting it back to the lowest rate it can recalulate the new highest rate.
+ */
+ audiohook_list->list_internal_samp_rate = DEFAULT_INTERNAL_SAMPLE_RATE;
+
audiohook_list->native_slin_compatible = 1;
AST_LIST_TRAVERSE(&audiohook_list->manipulate_list, ah, list) {
if (!(ah->init_flags & AST_AUDIOHOOK_MANIPULATE_ALL_RATES)) {
@@ -455,7 +485,7 @@
AST_LIST_HEAD_INIT_NOLOCK(&ast_channel_audiohooks(chan)->whisper_list);
AST_LIST_HEAD_INIT_NOLOCK(&ast_channel_audiohooks(chan)->manipulate_list);
/* This sample rate will adjust as necessary when writing to the list. */
- ast_channel_audiohooks(chan)->list_internal_samp_rate = 8000;
+ ast_channel_audiohooks(chan)->list_internal_samp_rate = DEFAULT_INTERNAL_SAMPLE_RATE;
}
/* Drop into respective list */
@@ -467,8 +497,11 @@
AST_LIST_INSERT_TAIL(&ast_channel_audiohooks(chan)->manipulate_list, audiohook, list);
}
-
- audiohook_set_internal_rate(audiohook, ast_channel_audiohooks(chan)->list_internal_samp_rate, 1);
+ /*
+ * Initialize the audiohook's rate to the default. If it needs to be,
+ * it will get updated later.
+ */
+ audiohook_set_internal_rate(audiohook, DEFAULT_INTERNAL_SAMPLE_RATE, 1);
audiohook_list_set_samplerate_compatibility(ast_channel_audiohooks(chan));
/* Change status over to running since it is now attached */
@@ -766,14 +799,14 @@
struct ast_frame *new_frame = frame;
struct ast_format *slin;
- /* If we are capable of maintaining doing samplerates other that 8khz, update
- * the internal audiohook_list's rate and higher samplerate audio arrives. By
- * updating the list's rate, all the audiohooks in the list will be updated as well
- * as the are written and read from. */
- if (audiohook_list->native_slin_compatible) {
- audiohook_list->list_internal_samp_rate =
- MAX(ast_format_get_sample_rate(frame->subclass.format), audiohook_list->list_internal_samp_rate);
- }
+ /*
+ * If we are capable of sample rates other that 8khz, update the internal
+ * audiohook_list's rate and higher sample rate audio arrives. If native
+ * slin compatibility is turned on all audiohooks in the list will be
+ * updated as well during read/write processing.
+ */
+ audiohook_list->list_internal_samp_rate =
+ MAX(ast_format_get_sample_rate(frame->subclass.format), audiohook_list->list_internal_samp_rate);
slin = ast_format_cache_get_slin_by_rate(audiohook_list->list_internal_samp_rate);
if (ast_format_cmp(frame->subclass.format, slin) == AST_FORMAT_CMP_EQUAL) {
@@ -822,6 +855,36 @@
}
/*!
+ *\brief Set the audiohook's internal sample rate to the audiohook_list's rate,
+ * but only when native slin compatibility is turned on.
+ *
+ * \param audiohook_list audiohook_list data object
+ * \param audiohook the audiohook to update
+ * \param rate the current max internal sample rate
+ */
+static void audiohook_list_set_hook_rate(struct ast_audiohook_list *audiohook_list,
+ struct ast_audiohook *audiohook, int *rate)
+{
+ /* The rate should always be the max between itself and the hook */
+ if (audiohook->hook_internal_samp_rate > *rate) {
+ *rate = audiohook->hook_internal_samp_rate;
+ }
+
+ /*
+ * If native slin compatibility is turned on then update the audiohook
+ * with the audiohook_list's current rate. Note, the audiohook's rate is
+ * set to the audiohook_list's rate and not the given rate. If there is
+ * a change in rate the hook's rate is changed on its next check.
+ */
+ if (audiohook_list->native_slin_compatible) {
+ ast_set_flag(audiohook, AST_AUDIOHOOK_COMPATIBLE);
+ audiohook_set_internal_rate(audiohook, audiohook_list->list_internal_samp_rate, 1);
+ } else {
+ ast_clear_flag(audiohook, AST_AUDIOHOOK_COMPATIBLE);
+ }
+}
+
+/*!
* \brief Pass an AUDIO frame off to be handled by the audiohook core
*
* \details
@@ -851,12 +914,26 @@
int samples;
int middle_frame_manipulated = 0;
int removed = 0;
+ int internal_sample_rate;
/* ---Part_1. translate start_frame to SLINEAR if necessary. */
if (!(middle_frame = audiohook_list_translate_to_slin(audiohook_list, direction, start_frame))) {
return frame;
}
samples = middle_frame->samples;
+
+ /*
+ * While processing each audiohook check to see if the internal sample rate needs
+ * to be adjusted (it should be the highest rate specified between formats and
+ * hooks). The given audiohook_list's internal sample rate is then set to the
+ * updated value before returning.
+ *
+ * If slin compatibility mode is turned on then an audiohook's internal sample
+ * rate is set to its audiohook_list's rate. If an audiohook_list's rate is
+ * adjusted during this pass then the change is picked up by the audiohooks
+ * on the next pass.
+ */
+ internal_sample_rate = audiohook_list->list_internal_samp_rate;
/* ---Part_2: Send middle_frame to spy and manipulator lists. middle_frame is guaranteed to be SLINEAR here.*/
/* Queue up signed linear frame to each spy */
@@ -872,7 +949,7 @@
}
continue;
}
- audiohook_set_internal_rate(audiohook, audiohook_list->list_internal_samp_rate, 1);
+ audiohook_list_set_hook_rate(audiohook_list, audiohook, &internal_sample_rate);
ast_audiohook_write_frame(audiohook, direction, middle_frame);
ast_audiohook_unlock(audiohook);
}
@@ -896,7 +973,7 @@
}
continue;
}
- audiohook_set_internal_rate(audiohook, audiohook_list->list_internal_samp_rate, 1);
+ audiohook_list_set_hook_rate(audiohook_list, audiohook, &internal_sample_rate);
if (ast_slinfactory_available(factory) >= samples && ast_slinfactory_read(factory, read_buf, samples)) {
/* Take audio from this whisper source and combine it into our main buffer */
for (i = 0, data1 = combine_buf, data2 = read_buf; i < samples; i++, data1++, data2++) {
@@ -929,14 +1006,16 @@
}
continue;
}
- audiohook_set_internal_rate(audiohook, audiohook_list->list_internal_samp_rate, 1);
- /* Feed in frame to manipulation. */
- if (!audiohook->manipulate_callback(audiohook, chan, middle_frame, direction)) {
- /* If the manipulation fails then the frame will be returned in its original state.
- * Since there are potentially more manipulator callbacks in the list, no action should
- * be taken here to exit early. */
- middle_frame_manipulated = 1;
- }
+ audiohook_list_set_hook_rate(audiohook_list, audiohook, &internal_sample_rate);
+ /*
+ * Feed in frame to manipulation.
+ *
+ * XXX FAILURES ARE IGNORED XXX
+ * If the manipulation fails then the frame will be returned in its original state.
+ * Since there are potentially more manipulator callbacks in the list, no action should
+ * be taken here to exit early.
+ */
+ audiohook->manipulate_callback(audiohook, chan, middle_frame, direction);
ast_audiohook_unlock(audiohook);
}
AST_LIST_TRAVERSE_SAFE_END;
@@ -960,6 +1039,12 @@
/* Before returning, if an audiohook got removed, reset samplerate compatibility */
if (removed) {
audiohook_list_set_samplerate_compatibility(audiohook_list);
+ } else {
+ /*
+ * Set the audiohook_list's rate to the updated rate. Note that if a hook
+ * was removed then the list's internal rate is reset to the default.
+ */
+ audiohook_list->list_internal_samp_rate = internal_sample_rate;
}
return end_frame;
--
To view, visit https://gerrit.asterisk.org/464
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Idab4dfef068a7922c09cc631dda27bc920a6c76f
Gerrit-PatchSet: 6
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
More information about the asterisk-code-review
mailing list