[Asterisk-code-review] audiohooks: Remove redundant codec translations when using a... (asterisk[14])

[Joshua Colp]

Joshua Colp has uploaded a new change for review.

  https://gerrit.asterisk.org/4058

Change subject: audiohooks: Remove redundant codec translations when using audiohooks
......................................................................

audiohooks: Remove redundant codec translations when using audiohooks

The main frame read and write handlers in main/channel.c don't use the
optimum placement in the processing flow for calling audiohooks
callbacks, as far as codec translation is concerned. This change places
the audiohooks callback code:
 * After the channel read translation if the frame is not linear before
the translation, thereby increasing the chance that the frame is linear
as required by audiohooks
 * Before the channel write translation if the frame is linear at this
point
This prevents the audiohooks code from instantiating additional
translation paths to/from linear where a linear frame format is already
available, saving valuable CPU cycles

ASTERISK-26419

Change-Id: I6edd5771f0740e758e7eb42558b953f046c01f8f
---
M main/channel.c
1 file changed, 38 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/58/4058/1

diff --git a/main/channel.c b/main/channel.c
index 94505ad..e2390ad 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -3914,6 +3914,7 @@
 		struct ast_frame *readq_tail = AST_LIST_LAST(ast_channel_readq(chan));
 		struct ast_control_read_action_payload *read_action_payload;
 		struct ast_party_connected_line connected;
+		int hooked = 0;
 
 		/* if the channel driver returned more than one frame, stuff the excess
 		   into the readq for the next ast_read call
@@ -4191,15 +4192,22 @@
 					break;
 				}
 			}
-			/* Send frame to audiohooks if present */
-			if (ast_channel_audiohooks(chan)) {
+			/*
+			 * Send frame to audiohooks if present, if frametype is linear, to preserve
+			 * functional compatibility with previous behavior. If not linear, hold off
+			 * until transcoding is done where we are more likely to have a linear frame
+			 */
+			if (ast_channel_audiohooks(chan) && ast_format_cache_is_slinear(f->subclass.format)) {
+				/* Place hooked after declaration */
 				struct ast_frame *old_frame = f;
+				hooked = 1;
 
 				f = ast_audiohook_write_list(chan, ast_channel_audiohooks(chan), AST_AUDIOHOOK_DIRECTION_READ, f);
 				if (old_frame != f) {
 					ast_frfree(old_frame);
 				}
 			}
+
 			if (ast_channel_monitor(chan) && ast_channel_monitor(chan)->read_stream) {
 				/* XXX what does this do ? */
 #ifndef MONITOR_CONSTANT_DELAY
@@ -4239,6 +4247,16 @@
 				f = ast_translate(ast_channel_readtrans(chan), f, 1);
 				if (!f) {
 					f = &ast_null_frame;
+				}
+			}
+
+			/* Second chance at hooking a linear frame, also the last chance */
+			if (ast_channel_audiohooks(chan) && !hooked) {
+				struct ast_frame *old_frame = f;
+
+				f = ast_audiohook_write_list(chan, ast_channel_audiohooks(chan), AST_AUDIOHOOK_DIRECTION_READ, f);
+				if (old_frame != f) {
+					ast_frfree(old_frame);
 				}
 			}
 
@@ -5032,6 +5050,7 @@
 	int res = -1;
 	struct ast_frame *f = NULL;
 	int count = 0;
+	int hooked = 0;
 
 	/*Deadlock avoidance*/
 	while(ast_channel_trylock(chan)) {
@@ -5149,6 +5168,22 @@
 			apply_plc(chan, fr);
 		}
 
+		/*
+		 * Send frame to audiohooks if present, if frametype is linear (else, later as per
+		 * previous behavior)
+		 */
+		if (ast_channel_audiohooks(chan)) {
+			if (ast_format_cache_is_slinear(fr->subclass.format)) {
+				struct ast_frame *old_frame;
+				hooked = 1;
+				old_frame = fr;
+				fr = ast_audiohook_write_list(chan, ast_channel_audiohooks(chan), AST_AUDIOHOOK_DIRECTION_WRITE, fr);
+				if (old_frame != fr) {
+					ast_frfree(old_frame);
+				}
+			}
+		}
+
 		/* If the frame is in the raw write format, then it's easy... just use the frame - otherwise we will have to translate */
 		if (ast_format_cmp(fr->subclass.format, ast_channel_rawwriteformat(chan)) == AST_FORMAT_CMP_EQUAL) {
 			f = fr;
@@ -5186,7 +5221,7 @@
 			break;
 		}
 
-		if (ast_channel_audiohooks(chan)) {
+		if (ast_channel_audiohooks(chan) && !hooked) {
 			struct ast_frame *prev = NULL, *new_frame, *cur, *dup;
 			int freeoldlist = 0;
 

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6edd5771f0740e758e7eb42558b953f046c01f8f
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 14
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Michael Walton <mike at farsouthnet.com>