[Asterisk-code-review] Revert "Revert "autoservice: Use frame deferral API"" (asterisk[master])

George Joseph asteriskteam at digium.com
Mon Nov 14 13:55:26 CST 2016


Hello Mark Michelson, Anonymous Coward #1000019, Joshua Colp,

I'd like you to do a code review.  Please visit

    https://gerrit.asterisk.org/4415

to review the following change.


Change subject: Revert "Revert "autoservice: Use frame deferral API""
......................................................................

Revert "Revert "autoservice: Use frame deferral API""

This reverts commit edca6911f392f47c1a5a25d1d3a357c72b04a78a.

Change-Id: I76030b87333a2c390cd05392b74b75678d78ddfa
---
M main/autoservice.c
1 file changed, 4 insertions(+), 62 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/15/4415/1

diff --git a/main/autoservice.c b/main/autoservice.c
index 11c9eab..c8a59a6 100644
--- a/main/autoservice.c
+++ b/main/autoservice.c
@@ -59,10 +59,6 @@
 	unsigned int use_count;
 	unsigned int orig_end_dtmf_flag:1;
 	unsigned int ignore_frame_types;
-	/*! Frames go on at the head of deferred_frames, so we have the frames
-	 *  from newest to oldest.  As we put them at the head of the readq, we'll
-	 *  end up with them in the right order for the channel's readq. */
-	AST_LIST_HEAD_NOLOCK(, ast_frame) deferred_frames;
 	AST_LIST_ENTRY(asent) list;
 };
 
@@ -77,19 +73,13 @@
 static void *autoservice_run(void *ign)
 {
 	ast_callid callid = 0;
-	struct ast_frame hangup_frame = {
-		.frametype = AST_FRAME_CONTROL,
-		.subclass.integer = AST_CONTROL_HANGUP,
-	};
 
 	while (!asexit) {
 		struct ast_channel *mons[MAX_AUTOMONS];
-		struct asent *ents[MAX_AUTOMONS];
 		struct ast_channel *chan;
 		struct asent *as;
-		int i, x = 0, ms = 50;
+		int x = 0, ms = 50;
 		struct ast_frame *f = NULL;
-		struct ast_frame *defer_frame = NULL;
 
 		AST_LIST_LOCK(&aslist);
 
@@ -104,7 +94,6 @@
 		AST_LIST_TRAVERSE(&aslist, as, list) {
 			if (!ast_check_hangup(as->chan)) {
 				if (x < MAX_AUTOMONS) {
-					ents[x] = as;
 					mons[x++] = as->chan;
 				} else {
 					ast_log(LOG_WARNING, "Exceeded maximum number of automatic monitoring events.  Fix autoservice.c\n");
@@ -132,51 +121,9 @@
 		ast_callid_threadassoc_change(callid);
 
 		f = ast_read(chan);
-
-		if (!f) {
-			/* No frame means the channel has been hung up.
-			 * A hangup frame needs to be queued here as ast_waitfor() may
-			 * never return again for the condition to be detected outside
-			 * of autoservice.  So, we'll leave a HANGUP queued up so the
-			 * thread in charge of this channel will know. */
-
-			defer_frame = &hangup_frame;
-		} else if (ast_is_deferrable_frame(f)) {
-			defer_frame = f;
-		} else {
-			/* Can't defer. Discard and continue with next. */
+		if (f) {
 			ast_frfree(f);
-			continue;
 		}
-
-		for (i = 0; i < x; i++) {
-			struct ast_frame *dup_f;
-
-			if (mons[i] != chan) {
-				continue;
-			}
-
-			if (!f) { /* defer_frame == &hangup_frame */
-				if ((dup_f = ast_frdup(defer_frame))) {
-					AST_LIST_INSERT_HEAD(&ents[i]->deferred_frames, dup_f, frame_list);
-				}
-			} else {
-				if ((dup_f = ast_frisolate(defer_frame))) {
-					AST_LIST_INSERT_HEAD(&ents[i]->deferred_frames, dup_f, frame_list);
-				}
-				if (dup_f != defer_frame) {
-					ast_frfree(defer_frame);
-				}
-			}
-
-			break;
-		}
-		/* The ast_waitfor_n() call will only read frames from
-		 * the channels' file descriptors. If ast_waitfor_n()
-		 * returns non-NULL, then one of the channels in the
-		 * mons array must have triggered the return. It's
-		 * therefore impossible that we got here while (i >= x).
-		 * If we did, we'd need to ast_frfree(f) if (f). */
 	}
 
 	ast_callid_threadassoc_change(0);
@@ -215,6 +162,7 @@
 	as->orig_end_dtmf_flag = ast_test_flag(ast_channel_flags(chan), AST_FLAG_END_DTMF_ONLY) ? 1 : 0;
 	if (!as->orig_end_dtmf_flag)
 		ast_set_flag(ast_channel_flags(chan), AST_FLAG_END_DTMF_ONLY);
+	ast_channel_start_defer_frames(chan);
 	ast_channel_unlock(chan);
 
 	AST_LIST_LOCK(&aslist);
@@ -248,7 +196,6 @@
 {
 	int res = -1;
 	struct asent *as, *removed = NULL;
-	struct ast_frame *f;
 	int chan_list_state;
 
 	AST_LIST_LOCK(&aslist);
@@ -300,12 +247,7 @@
 	}
 
 	ast_channel_lock(chan);
-	while ((f = AST_LIST_REMOVE_HEAD(&as->deferred_frames, frame_list))) {
-		if (!((1 << f->frametype) & as->ignore_frame_types)) {
-			ast_queue_frame_head(chan, f);
-		}
-		ast_frfree(f);
-	}
+	ast_channel_stop_defer_frames(chan);
 	ast_channel_unlock(chan);
 
 	ast_free(as);

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I76030b87333a2c390cd05392b74b75678d78ddfa
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>



More information about the asterisk-code-review mailing list