[Asterisk-code-review] AGI: Only defer frames when in an interception routine. (asterisk[14])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/4342 )

Change subject: AGI: Only defer frames when in an interception routine.
......................................................................


AGI: Only defer frames when in an interception routine.

AGI recently was modified to defer important frames. This was because
when AGI was used in a connected line interception routine, the
resulting connected line frame would end up getting discarded by the
AGI.

However, this caused bad behavior in other cases. Specifically, during a
transfer, if someone attempted to manually set the Caller ID on a
channel in an AGI, the deferred connected line frame would end up
overwriting what had been manually set in the AGI.

Since the initial issue was specific to interception routines, this
change removes the manual frame deferral from AGI and instead uses the
new frame deferral API in interception routines.

ASTERISK-26343 #close
Reported by Morton Tryfoss

Change-Id: Iab7d39436d0ee99bfe32ad55ef91e9bd88db4208
---
M main/channel.c
M res/res_agi.c
2 files changed, 25 insertions(+), 37 deletions(-)

Approvals:
  George Joseph: Looks good to me, approved
  Anonymous Coward #1000019: Verified



diff --git a/main/channel.c b/main/channel.c
index f00f222..9ac8a64 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -10261,9 +10261,15 @@
 
 		ast_party_connected_line_copy(ast_channel_connected(macro_chan), connected);
 	}
+	ast_channel_start_defer_frames(macro_chan);
 	ast_channel_unlock(macro_chan);
 
 	retval = ast_app_run_macro(autoservice_chan, macro_chan, macro, macro_args);
+
+	ast_channel_lock(macro_chan);
+	ast_channel_stop_defer_frames(macro_chan);
+	ast_channel_unlock(macro_chan);
+
 	if (!retval) {
 		struct ast_party_connected_line saved_connected;
 
@@ -10311,9 +10317,15 @@
 
 		ast_party_redirecting_copy(ast_channel_redirecting(macro_chan), redirecting);
 	}
+	ast_channel_start_defer_frames(macro_chan);
 	ast_channel_unlock(macro_chan);
 
 	retval = ast_app_run_macro(autoservice_chan, macro_chan, macro, macro_args);
+
+	ast_channel_lock(macro_chan);
+	ast_channel_stop_defer_frames(macro_chan);
+	ast_channel_unlock(macro_chan);
+
 	if (!retval) {
 		struct ast_party_redirecting saved_redirecting;
 
@@ -10354,9 +10366,15 @@
 
 		ast_party_connected_line_copy(ast_channel_connected(sub_chan), connected);
 	}
+	ast_channel_start_defer_frames(sub_chan);
 	ast_channel_unlock(sub_chan);
 
 	retval = ast_app_run_sub(autoservice_chan, sub_chan, sub, sub_args, 0);
+
+	ast_channel_lock(sub_chan);
+	ast_channel_stop_defer_frames(sub_chan);
+	ast_channel_unlock(sub_chan);
+
 	if (!retval) {
 		struct ast_party_connected_line saved_connected;
 
@@ -10397,9 +10415,15 @@
 
 		ast_party_redirecting_copy(ast_channel_redirecting(sub_chan), redirecting);
 	}
+	ast_channel_start_defer_frames(sub_chan);
 	ast_channel_unlock(sub_chan);
 
 	retval = ast_app_run_sub(autoservice_chan, sub_chan, sub, sub_args, 0);
+
+	ast_channel_lock(sub_chan);
+	ast_channel_stop_defer_frames(sub_chan);
+	ast_channel_unlock(sub_chan);
+
 	if (!retval) {
 		struct ast_party_redirecting saved_redirecting;
 
diff --git a/res/res_agi.c b/res/res_agi.c
index e0eb8e2..cb23a07 100644
--- a/res/res_agi.c
+++ b/res/res_agi.c
@@ -4093,23 +4093,6 @@
 	return AGI_RESULT_SUCCESS;
 }
 
-AST_LIST_HEAD_NOLOCK(deferred_frames, ast_frame);
-
-static void queue_deferred_frames(struct deferred_frames *deferred_frames,
-	struct ast_channel *chan)
-{
-	struct ast_frame *f;
-
-	if (!AST_LIST_EMPTY(deferred_frames)) {
-		ast_channel_lock(chan);
-		while ((f = AST_LIST_REMOVE_HEAD(deferred_frames, frame_list))) {
-			ast_queue_frame_head(chan, f);
-			ast_frfree(f);
-		}
-		ast_channel_unlock(chan);
-	}
-}
-
 static enum agi_result run_agi(struct ast_channel *chan, char *request, AGI *agi, int pid, int *status, int dead, int argc, char *argv[])
 {
 	struct ast_channel *c;
@@ -4128,9 +4111,6 @@
 	const char *sighup_str;
 	const char *exit_on_hangup_str;
 	int exit_on_hangup;
-	struct deferred_frames deferred_frames;
-
-	AST_LIST_HEAD_INIT_NOLOCK(&deferred_frames);
 
 	ast_channel_lock(chan);
 	sighup_str = pbx_builtin_getvar_helper(chan, "AGISIGHUP");
@@ -4192,20 +4172,8 @@
 					/* Write, ignoring errors */
 					if (write(agi->audio, f->data.ptr, f->datalen) < 0) {
 					}
-					ast_frfree(f);
-				} else if (ast_is_deferrable_frame(f)) {
-					struct ast_frame *dup_f;
-
-					if ((dup_f = ast_frisolate(f))) {
-						AST_LIST_INSERT_HEAD(&deferred_frames, dup_f, frame_list);
-					}
-
-					if (dup_f != f) {
-						ast_frfree(f);
-					}
-				} else {
-					ast_frfree(f);
 				}
+				ast_frfree(f);
 			}
 		} else if (outfd > -1) {
 			size_t len = sizeof(buf);
@@ -4253,8 +4221,6 @@
 				buf[buflen - 1] = '\0';
 			}
 
-			queue_deferred_frames(&deferred_frames, chan);
-
 			if (agidebug)
 				ast_verbose("<%s>AGI Rx << %s\n", ast_channel_name(chan), buf);
 			cmd_status = agi_handle_command(chan, agi, buf, dead);
@@ -4276,8 +4242,6 @@
 			}
 		}
 	}
-
-	queue_deferred_frames(&deferred_frames, chan);
 
 	if (agi->speech) {
 		ast_speech_destroy(agi->speech);

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Iab7d39436d0ee99bfe32ad55ef91e9bd88db4208
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: 14
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>