[Asterisk-code-review] ARI: Allow for early bridging on dialed ARI channels. (asterisk[master])

[Mark Michelson]

Mark Michelson has posted comments on this change.

Change subject: ARI: Allow for early bridging on dialed ARI channels.
......................................................................


Patch Set 1:

(3 comments)

https://gerrit.asterisk.org/#/c/2620/1/main/dial.c
File main/dial.c:

Line 517: 		ast_bridge_unsuspend(bridge, chan);
> See ASTERISK-21271 about concerns for _external_ threads suspending/unsuspe
{quote}
If a bridge channel is in the process of leaving the bridging system then the call to ast_bridge_suspend() would not work and there isn't a way for the caller to know he didn't get control.
{quote}

The crux of all of this is based on the hypothetical of the channel leaving the bridging system during the call to ast_bridge_suspend().

The question I have is, how would that actually happen in this scenario?

{quote}
You could gain control of the channel in the bridge by using ast_bridge_channel_queue_callback() to handle the dial functionality
{quote}

Is the idea that this would allow for me to suspend the channel in the bridge from within the bridge channel's thread? Or did you have something else in mind with this suggestion?

{quote}
We probably need to discuss ways and means to get this functionality accomplished.
{quote}

That's fine, but I'd prefer to keep that discussion on this review if at all possible.


Line 535: 			unsuspend_bridge(channel, original);
> You probably want to set soft-hanup before unsuspending the channel.
Can you explain why? The way things work right now, when the dial state changes to some "final" state, ARI will inspect the state and set the softhangup on the channel if the state is not AST_DIAL_ANSWERED. By having ARI setting softhangup, it gets the softhangup set on the channel whether it was bridged or unbridged when dialed.


PS1, Line 994: 			int hangup_channel;
             : 
             : 			if (channel->bridge_suspended) {
             : 				unsuspend_bridge(channel, channel->owner ?: who);
             : 				ast_channel_cleanup(channel->owner);
             : 				hangup_channel = 0;
             : 			} else{
             : 				hangup_channel = 1;
             : 			}
> This is too early.  After unsuspending a channel you have no control over t
What is "this"? Unsuspending the channel in the bridge?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3b05862644ca5080c7743a1b9b22465486e56ba
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes