Merge "audiohook.c: Difference in read/write rates caused continuous buffer resets" into 11

include/asterisk/audiohook.h

@@ -63,6 +63,7 @@ enum ast_audiohook_flags {
 	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 {

main/audiohook.c

@@ -45,6 +45,8 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 #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;
@@ -56,7 +58,17 @@ struct ast_audiohook_list {
 	 * variable will be set and the sample rate will
 	 * be preserved during ast_audiohook_write_list()*/
 	int native_slin_compatible;
-	int list_internal_samp_rate;/*!< Internal sample rate used when writing to the audiohook list */
+
+	/*
+	 * The internal sample rate is used when writing to the audiohook lists and
+	 * should always be set to the highest value between formats and audiohooks.
+	 *
+	 * During audiohook invocation (see write_list) if native slin is turned off
+ 	 * the highest sample rate is determined as each audiohook is processed.
+	 * If it is turned on then the audiohooks are also updated with the current
+	 * highest sample rate.
+	*/
+	int list_internal_samp_rate;
 
 	struct ast_audiohook_translate in_translate[2];
 	struct ast_audiohook_translate out_translate[2];
@@ -115,7 +127,7 @@ int ast_audiohook_init(struct ast_audiohook *audiohook, enum ast_audiohook_type
 	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);
@@ -351,7 +363,19 @@ static struct ast_frame *audiohook_read_frame_helper(struct ast_audiohook *audio
 	struct ast_frame *read_frame = NULL, *final_frame = NULL;
 	struct ast_format tmp_fmt;
 
+	/*
+	 * 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_rate(format) > audiohook->hook_internal_samp_rate)) {
 		audiohook_set_internal_rate(audiohook, ast_format_rate(format), 1);
+	}
 
 	if (!(read_frame = (direction == AST_AUDIOHOOK_DIRECTION_BOTH ?
 		audiohook_read_frame_both(audiohook, samples, read_reference, write_reference) :
@@ -413,6 +437,22 @@ struct ast_frame *ast_audiohook_read_frame_all(struct ast_audiohook *audiohook,
 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)) {
@@ -443,7 +483,7 @@ int ast_audiohook_attach(struct ast_channel *chan, struct ast_audiohook *audioho
 		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 */
@@ -455,7 +495,11 @@ int ast_audiohook_attach(struct ast_channel *chan, struct ast_audiohook *audioho
 		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 */
@@ -646,7 +690,6 @@ int ast_audiohook_remove(struct ast_channel *chan, struct ast_audiohook *audioho
 
 	audiohook_list_set_samplerate_compatibility(ast_channel_audiohooks(chan));
 	ast_audiohook_update_status(audiohook, AST_AUDIOHOOK_STATUS_DONE);
-
 	ast_channel_unlock(chan);
 
 	return 0;
@@ -696,14 +739,14 @@ static struct ast_frame *audiohook_list_translate_to_slin(struct ast_audiohook_l
 	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
+	 * If we are capable of sample rates other that 8khz, update the internal
-	 * updating the list's rate, all the audiohooks in the list will be updated as well
+	 * audiohook_list's rate and higher sample rate audio arrives. If native
-	 * as the are written and read from. */
+	 * slin compatibility is turned on all audiohooks in the list will be
-	if (audiohook_list->native_slin_compatible) {
+	 * updated as well during read/write processing.
+	 */
 	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);
 
@@ -751,6 +794,36 @@ static struct ast_frame *audiohook_list_translate_to_native(struct ast_audiohook
 	return outframe;
 }
 
+/*!
+ *\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
  *
@@ -781,6 +854,7 @@ static struct ast_frame *audio_audiohook_write_list(struct ast_channel *chan, st
 	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))) {
@@ -788,6 +862,19 @@ static struct ast_frame *audio_audiohook_write_list(struct ast_channel *chan, st
 	}
 	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 */
 	AST_LIST_TRAVERSE_SAFE_BEGIN(&audiohook_list->spy_list, audiohook, list) {
@@ -799,7 +886,7 @@ static struct ast_frame *audio_audiohook_write_list(struct ast_channel *chan, st
 			ast_audiohook_unlock(audiohook);
 			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);
 	}
@@ -819,8 +906,9 @@ static struct ast_frame *audio_audiohook_write_list(struct ast_channel *chan, st
 				ast_audiohook_unlock(audiohook);
 				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(&audiohook->write_factory) >= samples && ast_slinfactory_read(&audiohook->write_factory, read_buf, samples)) {
+			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++)
 					ast_slinear_saturated_add(data1, data2);
@@ -848,15 +936,16 @@ static struct ast_frame *audio_audiohook_write_list(struct ast_channel *chan, st
 				audiohook->manipulate_callback(audiohook, chan, NULL, direction);
 				continue;
 			}
-			audiohook_set_internal_rate(audiohook, audiohook_list->list_internal_samp_rate, 1);
+			audiohook_list_set_hook_rate(audiohook_list, audiohook, &internal_sample_rate);
-			/* Feed in frame to manipulation. */
+			/*
-			if (audiohook->manipulate_callback(audiohook, chan, middle_frame, direction)) {
+			 * Feed in frame to manipulation.
-				/* XXX IGNORE FAILURE */
+			 *
-
+			 * XXX FAILURES ARE IGNORED XXX
-				/* If the manipulation fails then the frame will be returned in its original state.
+			 * 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. */
+			 * be taken here to exit early.
-			}
+			 */
+			audiohook->manipulate_callback(audiohook, chan, middle_frame, direction);
 			ast_audiohook_unlock(audiohook);
 		}
 		AST_LIST_TRAVERSE_SAFE_END;
@@ -881,6 +970,12 @@ static struct ast_frame *audio_audiohook_write_list(struct ast_channel *chan, st
 	/* 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;