[Asterisk-code-review] audiohook.c: Difference in read/write rates caused continuou... (asterisk[11])

Kevin Harwell asteriskteam at digium.com
Wed May 13 15:13:00 CDT 2015


Kevin Harwell has uploaded a new change for review.

  https://gerrit.asterisk.org/462

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 on a write, and before translation, the internal
sample rates are checked and updated accordingly if the rate happened to have
changed on a read. This also enforces that the internal rate stays in sync,
favoring a higher rate, despite any difference in read/write rates.

Change-Id: Idab4dfef068a7922c09cc631dda27bc920a6c76f
---
M main/audiohook.c
1 file changed, 75 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/62/462/1

diff --git a/main/audiohook.c b/main/audiohook.c
index d81df85..d25b194 100644
--- a/main/audiohook.c
+++ b/main/audiohook.c
@@ -69,7 +69,11 @@
 {
 	struct ast_format slin;
 
-	if (audiohook->hook_internal_samp_rate == rate) {
+	/*
+	 * Only set the rate if it is higher than the current one. This prevents possible switching
+	 * and resetting of the rate if there is a difference between read and write rates.
+	 */
+	if (audiohook->hook_internal_samp_rate && audiohook->hook_internal_samp_rate >= rate) {
 		return 0;
 	}
 
@@ -696,15 +700,6 @@
 	struct ast_format tmp_fmt;
 	enum ast_format_id slin_id;
 
-	/* 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_rate(&frame->subclass.format), audiohook_list->list_internal_samp_rate);
-	}
-
 	slin_id = ast_format_slin_by_rate(audiohook_list->list_internal_samp_rate);
 
 	if (frame->subclass.format.id == slin_id) {
@@ -751,6 +746,71 @@
 	return outframe;
 }
 
+static void audiohook_list_set_internal_rate(struct ast_audiohook_list *audiohook_list, int rate)
+{
+	/*
+	 * If we are capable of handling sample rates 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.
+	 */
+	if (audiohook_list->native_slin_compatible &&
+	    audiohook_list->list_internal_samp_rate < rate) {
+		audiohook_list->list_internal_samp_rate = rate;
+	}
+}
+
+static void audiohook_list_check_internal_rates(struct ast_audiohook_list *audiohook_list,
+						struct ast_audiohook *audiohook)
+{
+	/*
+	 * The audiohook's rate may have changed during a read. If so the list's rate may
+	 * also need to be updated.
+	 */
+	audiohook_list_set_internal_rate(audiohook_list, audiohook->hook_internal_samp_rate);
+
+	/*
+	 * The list's rate may have changed (an audiohook's rate changed or an incoming
+	 * frame had a higher sample rate, and compatibility mode was enabled). If so the
+	 * audiohook's rate may need to change as well.
+	 */
+	audiohook_set_internal_rate(audiohook, audiohook_list->list_internal_samp_rate, 1);
+}
+
+/*!
+ * \brief Audiohook lists pre-checks.
+ *
+ * If they need to be, update the audiohook_list's and hook's internal sample rates.
+ *
+ * \param frame the frame itself
+ * \param audiohook_list audiohook_list data object
+ */
+static void audiohook_lists_check(struct ast_frame *frame, struct ast_audiohook_list *audiohook_list)
+{
+	struct ast_audiohook *audiohook;
+
+	/*
+	 * The incoming frame's sample rate may be higher than the current audiohook_list's internal
+	 * sample rate. If it is adjust the internal rate accordingly (if native slin compatibility
+	 * is enabled).
+	 */
+	audiohook_list_set_internal_rate(audiohook_list, ast_format_rate(&frame->subclass.format));
+
+	/*
+	 * For the spy list check if the internal rates need to be updated. A change in rate may
+	 * occur if the incoming frame's sample rate changes it (see above), or if the internal
+	 * sample rate on a hook itself gets modified during a read. If a hook's rate gets modified
+	 * on a read, then by checking here the audiohook_list's rate can also be adjusted if needed.
+	 */
+	AST_LIST_TRAVERSE_SAFE_BEGIN(&audiohook_list->spy_list, audiohook, list) {
+		ast_audiohook_lock(audiohook);
+		if (audiohook->status == AST_AUDIOHOOK_STATUS_RUNNING) {
+			audiohook_list_check_internal_rates(audiohook_list, audiohook);
+		}
+		ast_audiohook_unlock(audiohook);
+	}
+	AST_LIST_TRAVERSE_SAFE_END;
+}
+
 /*!
  * \brief Pass an AUDIO frame off to be handled by the audiohook core
  *
@@ -782,6 +842,11 @@
 	int middle_frame_manipulated = 0;
 	int removed = 0;
 
+	/*
+	 * Do a pre-check on the audiohooks to see if sample rates need to be adjusted.
+	 */
+	audiohook_lists_check(frame, audiohook_list);
+
 	/* ---Part_1. translate start_frame to SLINEAR if necessary. */
 	if (!(middle_frame = audiohook_list_translate_to_slin(audiohook_list, direction, start_frame))) {
 		return frame;
@@ -799,7 +864,6 @@
 			ast_audiohook_unlock(audiohook);
 			continue;
 		}
-		audiohook_set_internal_rate(audiohook, audiohook_list->list_internal_samp_rate, 1);
 		ast_audiohook_write_frame(audiohook, direction, middle_frame);
 		ast_audiohook_unlock(audiohook);
 	}
@@ -819,7 +883,6 @@
 				ast_audiohook_unlock(audiohook);
 				continue;
 			}
-			audiohook_set_internal_rate(audiohook, audiohook_list->list_internal_samp_rate, 1);
 			if (ast_slinfactory_available(&audiohook->write_factory) >= samples && ast_slinfactory_read(&audiohook->write_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++)
@@ -848,7 +911,6 @@
 				audiohook->manipulate_callback(audiohook, chan, NULL, direction);
 				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)) {
 				/* XXX IGNORE FAILURE */

-- 
To view, visit https://gerrit.asterisk.org/462
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Idab4dfef068a7922c09cc631dda27bc920a6c76f
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 11
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>



More information about the asterisk-code-review mailing list