[Asterisk-code-review] res stasis snoop/audiohook: Single direction spying continua... (asterisk[master])

[Kevin Harwell]

Kevin Harwell has uploaded a new change for review.

  https://gerrit.asterisk.org/388

Change subject: res_stasis_snoop/audiohook: Single direction spying continually increases CPU
......................................................................

res_stasis_snoop/audiohook: Single direction spying continually increases CPU

Creating a snoop channel in ARI and spying only on a single direction (in or
out) results in CPU utilization continually increasing until the CPU is fully
consumed. This occurs because frames are being put in the opposing direction's
slin factory queue, but not being removed.

This initially was fixed by reading frames from the opposing direction's queue
and dropping them. However a better solution would be to never have those
frames added to the queue in the first place. So this patch fixes the problem
by setting a new flag on audiohook initialization that specifies whether or not
to make the audiohook read or write only. If set no frames are written to the
associated factory queue.

ASTERISK-24938 #closes

Change-Id: I49fd7053aa82f698334c855d93c263820c7c02e2
---
M include/asterisk/audiohook.h
M main/audiohook.c
M res/res_stasis_snoop.c
3 files changed, 26 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/88/388/1

diff --git a/include/asterisk/audiohook.h b/include/asterisk/audiohook.h
index 375b2dd..02628ba 100644
--- a/include/asterisk/audiohook.h
+++ b/include/asterisk/audiohook.h
@@ -63,6 +63,8 @@
 	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_READ_ONLY     = (1 << 7), /*!< audiohook will not write to write queue */
+	AST_AUDIOHOOK_WRITE_ONLY    = (1 << 8), /*!< audiohook will not write to read queue */
 };
 
 enum ast_audiohook_init_flags {
@@ -123,10 +125,22 @@
  * \param audiohook Audiohook structure
  * \param type Type of audiohook to initialize this as
  * \param source Who is initializing this audiohook
+ * \param init_flags flags used at initialization
+ * \param flags flags used while processing
+ * \return Returns 0 on success, -1 on failure
+ */
+int ast_audiohook_initialize(struct ast_audiohook *audiohook, enum ast_audiohook_type type, const char *source,
+			     enum ast_audiohook_init_flags init_flags, unsigned int flags);
+
+/*! \brief Initialize an audiohook structure
+ * \param audiohook Audiohook structure
+ * \param type Type of audiohook to initialize this as
+ * \param source Who is initializing this audiohook
  * \param init flags
  * \return Returns 0 on success, -1 on failure
  */
-int ast_audiohook_init(struct ast_audiohook *audiohook, enum ast_audiohook_type type, const char *source, enum ast_audiohook_init_flags flags);
+#define ast_audiohook_init(audiohook, type, source, init_flags) ast_audiohook_initialize(audiohook, type, source, init_flags, 0)
+
 
 /*! \brief Destroys an audiohook structure
  * \param audiohook Audiohook structure
diff --git a/main/audiohook.c b/main/audiohook.c
index b754f23..e4c4db1 100644
--- a/main/audiohook.c
+++ b/main/audiohook.c
@@ -104,7 +104,8 @@
  *
  * \return Returns 0 on success, -1 on failure
  */
-int ast_audiohook_init(struct ast_audiohook *audiohook, enum ast_audiohook_type type, const char *source, enum ast_audiohook_init_flags init_flags)
+int ast_audiohook_initialize(struct ast_audiohook *audiohook, enum ast_audiohook_type type, const char *source,
+			     enum ast_audiohook_init_flags init_flags, unsigned int flags)
 {
 	/* Need to keep the type and source */
 	audiohook->type = type;
@@ -115,6 +116,7 @@
 	ast_cond_init(&audiohook->trigger, NULL);
 
 	audiohook->init_flags = init_flags;
+	audiohook->flags = flags;
 
 	/* initialize internal rate at 8khz, this will adjust if necessary */
 	audiohook_set_internal_rate(audiohook, 8000, 0);
@@ -175,6 +177,11 @@
 	/* Update last feeding time to be current */
 	*rwtime = ast_tvnow();
 
+	if ((ast_test_flag(audiohook, AST_AUDIOHOOK_READ_ONLY) && (direction == AST_AUDIOHOOK_DIRECTION_WRITE)) ||
+	    (ast_test_flag(audiohook, AST_AUDIOHOOK_WRITE_ONLY) && (direction == AST_AUDIOHOOK_DIRECTION_READ))) {
+		    return 0;
+	}
+
 	our_factory_samples = ast_slinfactory_available(factory);
 	our_factory_ms = ast_tvdiff_ms(*rwtime, previous_time) + (our_factory_samples / (audiohook->hook_internal_samp_rate / 1000));
 	other_factory_samples = ast_slinfactory_available(other_factory);
diff --git a/res/res_stasis_snoop.c b/res/res_stasis_snoop.c
index 5311aed..543f9f8 100644
--- a/res/res_stasis_snoop.c
+++ b/res/res_stasis_snoop.c
@@ -259,13 +259,14 @@
 static int snoop_setup_audiohook(struct ast_channel *chan, enum ast_audiohook_type type, enum stasis_app_snoop_direction requested_direction,
 	enum ast_audiohook_direction *direction, struct ast_audiohook *audiohook)
 {
-	ast_audiohook_init(audiohook, type, "Snoop", 0);
-
 	if (requested_direction == STASIS_SNOOP_DIRECTION_OUT) {
+		ast_audiohook_initialize(audiohook, type, "Snoop", 0, AST_AUDIOHOOK_WRITE_ONLY);
 		*direction = AST_AUDIOHOOK_DIRECTION_WRITE;
 	} else if (requested_direction == STASIS_SNOOP_DIRECTION_IN) {
+		ast_audiohook_initialize(audiohook, type, "Snoop", 0, AST_AUDIOHOOK_READ_ONLY);
 		*direction = AST_AUDIOHOOK_DIRECTION_READ;
 	} else if (requested_direction == STASIS_SNOOP_DIRECTION_BOTH) {
+		ast_audiohook_init(audiohook, type, "Snoop", 0);
 		*direction = AST_AUDIOHOOK_DIRECTION_BOTH;
 	} else {
 		return -1;

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

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