[Asterisk-code-review] channel.c: Route all control frames to a channel through the... (asterisk[master])
Richard Mudgett
asteriskteam at digium.com
Thu Feb 25 12:10:43 CST 2016
Richard Mudgett has uploaded a new change for review.
https://gerrit.asterisk.org/2304
Change subject: channel.c: Route all control frames to a channel through the same code.
......................................................................
channel.c: Route all control frames to a channel through the same code.
Frame hooks can conceivably return a control frame in exchange for an
audio frame inside ast_write(). Those returned control frames were not
handled quite the same as if they were sent to ast_indicate(). Now it
doesn't matter if you use ast_write() to send an AST_FRAME_CONTROL to a
channel or ast_indicate().
ASTERISK-25582
Change-Id: I5775f41421aca2b510128198e9b827bf9169629b
---
M main/channel.c
1 file changed, 68 insertions(+), 53 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/04/2304/1
diff --git a/main/channel.c b/main/channel.c
index f5a9afa..f9addb0 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -4473,67 +4473,30 @@
return res ? -1 : 0;
}
-int ast_indicate_data(struct ast_channel *chan, int _condition,
- const void *data, size_t datalen)
+static int indicate_data_internal(struct ast_channel *chan, int _condition, const void *data, size_t datalen)
{
/* By using an enum, we'll get compiler warnings for values not handled
* in switch statements. */
enum ast_control_frame_type condition = _condition;
struct ast_tone_zone_sound *ts = NULL;
int res;
- /* this frame is used by framehooks. if it is set, we must free it at the end of this function */
- struct ast_frame *awesome_frame = NULL;
-
- ast_channel_lock(chan);
-
- /* Don't bother if the channel is about to go away, anyway. */
- if ((ast_test_flag(ast_channel_flags(chan), AST_FLAG_ZOMBIE)
- || (ast_check_hangup(chan) && !ast_channel_is_leaving_bridge(chan)))
- && condition != AST_CONTROL_MASQUERADE_NOTIFY) {
- res = -1;
- goto indicate_cleanup;
- }
-
- if (!ast_framehook_list_is_empty(ast_channel_framehooks(chan))) {
- /* Do framehooks now, do it, go, go now */
- struct ast_frame frame = {
- .frametype = AST_FRAME_CONTROL,
- .subclass.integer = condition,
- .data.ptr = (void *) data, /* this cast from const is only okay because we do the ast_frdup below */
- .datalen = datalen
- };
-
- /* we have now committed to freeing this frame */
- awesome_frame = ast_frdup(&frame);
-
- /* who knows what we will get back! the anticipation is killing me. */
- if (!(awesome_frame = ast_framehook_list_write_event(ast_channel_framehooks(chan), awesome_frame))
- || awesome_frame->frametype != AST_FRAME_CONTROL) {
- res = 0;
- goto indicate_cleanup;
- }
-
- condition = awesome_frame->subclass.integer;
- data = awesome_frame->data.ptr;
- datalen = awesome_frame->datalen;
- }
switch (condition) {
case AST_CONTROL_CONNECTED_LINE:
if (indicate_connected_line(chan, data, datalen)) {
res = 0;
- goto indicate_cleanup;
+ return res;
}
break;
case AST_CONTROL_REDIRECTING:
if (indicate_redirecting(chan, data, datalen)) {
res = 0;
- goto indicate_cleanup;
+ return res;
}
break;
case AST_CONTROL_HOLD:
case AST_CONTROL_UNHOLD:
- ast_channel_hold_state_set(chan, condition);
+ ast_channel_hold_state_set(chan, _condition);
break;
default:
break;
@@ -4541,7 +4504,7 @@
if (is_visible_indication(condition)) {
/* A new visible indication is requested. */
- ast_channel_visible_indication_set(chan, condition);
+ ast_channel_visible_indication_set(chan, _condition);
} else if (condition == AST_CONTROL_UNHOLD || _condition < 0) {
/* Visible indication is cleared/stopped. */
ast_channel_visible_indication_set(chan, 0);
@@ -4549,7 +4512,7 @@
if (ast_channel_tech(chan)->indicate) {
/* See if the channel driver can handle this condition. */
- res = ast_channel_tech(chan)->indicate(chan, condition, data, datalen);
+ res = ast_channel_tech(chan)->indicate(chan, _condition, data, datalen);
} else {
res = -1;
}
@@ -4557,7 +4520,7 @@
if (!res) {
/* The channel driver successfully handled this indication */
res = 0;
- goto indicate_cleanup;
+ return res;
}
/* The channel driver does not support this indication, let's fake
@@ -4570,7 +4533,7 @@
/* Stop any tones that are playing */
ast_playtones_stop(chan);
res = 0;
- goto indicate_cleanup;
+ return res;
}
/* Handle conditions that we have tones for. */
@@ -4578,7 +4541,7 @@
case _XXX_AST_CONTROL_T38:
/* deprecated T.38 control frame */
res = -1;
- goto indicate_cleanup;
+ return res;
case AST_CONTROL_T38_PARAMETERS:
/* there is no way to provide 'default' behavior for these
* control frames, so we need to return failure, but there
@@ -4587,7 +4550,7 @@
* so just return right now. in addition, we want to return
* whatever value the channel driver returned, in case it
* has some meaning.*/
- goto indicate_cleanup;
+ return res;
case AST_CONTROL_RINGING:
ts = ast_get_indication_tone(ast_channel_zone(chan), "ring");
/* It is common practice for channel drivers to return -1 if trying
@@ -4669,6 +4632,53 @@
/* not handled */
ast_log(LOG_WARNING, "Unable to handle indication %u for '%s'\n", condition, ast_channel_name(chan));
}
+
+ return res;
+}
+
+int ast_indicate_data(struct ast_channel *chan, int _condition, const void *data, size_t datalen)
+{
+ int res;
+ /* this frame is used by framehooks. if it is set, we must free it at the end of this function */
+ struct ast_frame *awesome_frame = NULL;
+
+ ast_channel_lock(chan);
+
+ /* Don't bother if the channel is about to go away, anyway. */
+ if ((ast_test_flag(ast_channel_flags(chan), AST_FLAG_ZOMBIE)
+ || (ast_check_hangup(chan) && !ast_channel_is_leaving_bridge(chan)))
+ && _condition != AST_CONTROL_MASQUERADE_NOTIFY) {
+ res = -1;
+ goto indicate_cleanup;
+ }
+
+ if (!ast_framehook_list_is_empty(ast_channel_framehooks(chan))) {
+ /* Do framehooks now, do it, go, go now */
+ struct ast_frame frame = {
+ .frametype = AST_FRAME_CONTROL,
+ .subclass.integer = _condition,
+ .data.ptr = (void *) data, /* this cast from const is only okay because we do the ast_frdup below */
+ .datalen = datalen
+ };
+
+ /* we have now committed to freeing this frame */
+ awesome_frame = ast_frdup(&frame);
+
+ /* who knows what we will get back! the anticipation is killing me. */
+ awesome_frame = ast_framehook_list_write_event(ast_channel_framehooks(chan),
+ awesome_frame);
+ if (!awesome_frame
+ || awesome_frame->frametype != AST_FRAME_CONTROL) {
+ res = 0;
+ goto indicate_cleanup;
+ }
+
+ _condition = awesome_frame->subclass.integer;
+ data = awesome_frame->data.ptr;
+ datalen = awesome_frame->datalen;
+ }
+
+ res = indicate_data_internal(chan, _condition, data, datalen);
indicate_cleanup:
ast_channel_unlock(chan);
@@ -5012,10 +5022,15 @@
res = ast_senddigit_end(chan, fr->subclass.integer, fr->len);
ast_channel_lock(chan);
CHECK_BLOCKING(chan);
- } else if (fr->frametype == AST_FRAME_CONTROL && fr->subclass.integer == AST_CONTROL_UNHOLD) {
- /* This is a side case where Echo is basically being called and the person put themselves on hold and took themselves off hold */
- res = (ast_channel_tech(chan)->indicate == NULL) ? 0 :
- ast_channel_tech(chan)->indicate(chan, fr->subclass.integer, fr->data.ptr, fr->datalen);
+ } else if (fr->frametype == AST_FRAME_CONTROL
+ && fr->subclass.integer == AST_CONTROL_UNHOLD) {
+ /*
+ * This is a side case where Echo is basically being called
+ * and the person put themselves on hold and took themselves
+ * off hold.
+ */
+ indicate_data_internal(chan, fr->subclass.integer, fr->data.ptr,
+ fr->datalen);
}
res = 0; /* XXX explain, why 0 ? */
goto done;
@@ -5027,8 +5042,8 @@
CHECK_BLOCKING(chan);
switch (fr->frametype) {
case AST_FRAME_CONTROL:
- res = (ast_channel_tech(chan)->indicate == NULL) ? 0 :
- ast_channel_tech(chan)->indicate(chan, fr->subclass.integer, fr->data.ptr, fr->datalen);
+ indicate_data_internal(chan, fr->subclass.integer, fr->data.ptr, fr->datalen);
+ res = 0;
break;
case AST_FRAME_DTMF_BEGIN:
if (ast_channel_audiohooks(chan)) {
--
To view, visit https://gerrit.asterisk.org/2304
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5775f41421aca2b510128198e9b827bf9169629b
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
More information about the asterisk-code-review
mailing list