[Asterisk-code-review] audiohook: add directional awareness (asterisk[master])

[Friendly Automation]

Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/19445 )

Change subject: audiohook: add directional awareness
......................................................................

audiohook: add directional awareness

Add enum to allow setting optional direction. If set to only one
direction, only feed matching-direction frames to the associated
slin factory.

This prevents mangling the transcoder on non-mixed frames when the
READ and WRITE frames would have otherwise required it.  Also
removes the need to mute or discard the un-wanted frames as they
are no longer added in the first place.

res_stasis_snoop is changed to use this addition to set direction
on audiohook based on spy direction.

If no direction is set, the ast_audiohook_init will init this enum
to BOTH which maintains existing functionality.

ASTERISK-30252

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

Approvals:
  George Joseph: Looks good to me, approved
  Friendly Automation: Approved for Submit




diff --git a/include/asterisk/audiohook.h b/include/asterisk/audiohook.h
index 8e93d56..a5d7e0c 100644
--- a/include/asterisk/audiohook.h
+++ b/include/asterisk/audiohook.h
@@ -118,6 +118,7 @@
 	ast_audiohook_manipulate_callback manipulate_callback; /*!< Manipulation callback */
 	struct ast_audiohook_options options;                  /*!< Applicable options */
 	unsigned int hook_internal_samp_rate;                  /*!< internal read/write sample rate on the audiohook.*/
+	enum ast_audiohook_direction direction;                /*!< Intended audiohook direction, BOTH by default on init */
 	AST_LIST_ENTRY(ast_audiohook) list;                    /*!< Linked list information */
 };
 
@@ -140,6 +141,14 @@
  */
 int ast_audiohook_destroy(struct ast_audiohook *audiohook);
 
+/*! \brief Sets direction on audiohook
+ * \param audiohook
+ * \param direction In which direction should the audiohook feed frames, ie if we are snooping 'in', set direction to READ so that only the 'in' frames are fed to the slin factory
+ * \retval 0 on success
+ * \retval -1 on failure due to audiohook already in use or in shutdown. Can only set direction on NEW audiohooks
+ */
+int ast_audiohook_set_frame_feed_direction(struct ast_audiohook *audiohook, enum ast_audiohook_direction direction);
+
 /*! \brief Writes a frame into the audiohook structure
  * \param audiohook
  * \param direction Direction the audio frame came from
diff --git a/main/audiohook.c b/main/audiohook.c
index 966d5f2..3154ede 100644
--- a/main/audiohook.c
+++ b/main/audiohook.c
@@ -109,6 +109,9 @@
 
 	audiohook->init_flags = init_flags;
 
+	/* Set direction to BOTH so that we feed frames in both directions */
+	audiohook->direction = AST_AUDIOHOOK_DIRECTION_BOTH;
+
 	/* initialize internal rate at 8khz, this will adjust if necessary */
 	audiohook_set_internal_rate(audiohook, DEFAULT_INTERNAL_SAMPLE_RATE, 0);
 
@@ -144,6 +147,18 @@
 	return 0;
 }
 
+int ast_audiohook_set_frame_feed_direction(struct ast_audiohook *audiohook, enum ast_audiohook_direction direction)
+{
+	/* Only set the direction on new audiohooks */
+	if (audiohook->status != AST_AUDIOHOOK_STATUS_NEW) {
+		ast_debug(3, "Can not set direction on attached Audiohook %p\n", audiohook);
+		return -1;
+	}
+
+	audiohook->direction = direction;
+	return 0;
+}
+
 #define SHOULD_MUTE(hook, dir) \
 	((ast_test_flag(hook, AST_AUDIOHOOK_MUTE_READ) && (dir == AST_AUDIOHOOK_DIRECTION_READ)) || \
 	(ast_test_flag(hook, AST_AUDIOHOOK_MUTE_WRITE) && (dir == AST_AUDIOHOOK_DIRECTION_WRITE)) || \
@@ -159,6 +174,13 @@
 	int other_factory_samples;
 	int other_factory_ms;
 
+	/* Don't feed the frame if we are set to read and this is a write frame or if set to
+	   write and this is a read frame as we don't want it. Plus, it can cause mis-resampling
+	   if the READ and WRITE frames have different bitrates */
+	if (audiohook->direction != AST_AUDIOHOOK_DIRECTION_BOTH && audiohook->direction != direction) {
+		return 0;
+	}
+
 	/* Update last feeding time to be current */
 	*rwtime = ast_tvnow();
 
diff --git a/res/res_stasis_snoop.c b/res/res_stasis_snoop.c
index 8d0c6eb..70cdd7c 100644
--- a/res/res_stasis_snoop.c
+++ b/res/res_stasis_snoop.c
@@ -188,21 +188,9 @@
 	}
 
 	ast_audiohook_lock(&snoop->spy);
-	if (snoop->spy_direction != AST_AUDIOHOOK_DIRECTION_BOTH) {
-		/*
-		 * When a singular direction is chosen frames are still written to the
-		 * opposing direction's queue. Those frames must be read so the queue
-		 * does not continue to grow, however since they are not needed for the
-		 * selected direction they can be dropped.
-		 */
-		enum ast_audiohook_direction opposing_direction =
-			snoop->spy_direction == AST_AUDIOHOOK_DIRECTION_READ ?
-			AST_AUDIOHOOK_DIRECTION_WRITE : AST_AUDIOHOOK_DIRECTION_READ;
-		ast_frame_dtor(ast_audiohook_read_frame(&snoop->spy, snoop->spy_samples,
-							opposing_direction, snoop->spy_format));
-	}
 
 	frame = ast_audiohook_read_frame(&snoop->spy, snoop->spy_samples, snoop->spy_direction, snoop->spy_format);
+
 	ast_audiohook_unlock(&snoop->spy);
 
 	return frame ? frame : &snoop->silence;
@@ -287,6 +275,14 @@
 		return -1;
 	}
 
+	/* Set the audiohook direction so we don't write unnecessary frames */
+	if (ast_audiohook_set_frame_feed_direction(audiohook, *direction)) {
+		/* If we are unable to set direction, the audiohook either failed to init
+		   or someone else started using it already.  If we don't bail here, we risk
+		   feeding frames that will never be read */
+		return -1;
+	}
+
 	return ast_audiohook_attach(chan, audiohook);
 }
 

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/19445
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: If8716bad334562a5d812be4eeb2a92e4f3be28eb
Gerrit-Change-Number: 19445
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Bradeen <mbradeen at sangoma.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20221011/ebc7b9b5/attachment.html>