Summary:	ASTERISK-27448: [patch] Add ability to send progress inband by setting a channel variable instead of fixed config per endpoint
Reporter:	nappsoft (nappsoft)	Labels:	patch pjsip
Date Opened:	2017-11-29 05:30:09.000-0600	Date Closed:	2020-01-14 11:13:35.000-0600
Priority:	Minor	Regression?
Status:	Closed/Complete	Components:	Channels/chan_pjsip
Versions:	13.18.2	Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) progressinband3.diff
Description:	I've faced a problem with several voip switches when using 180 Ringing messages after announcements sent over 183 Session Progress with SDP (no audible progress indication after Progress followed by Playback). The solution would be to use inband_progress=yes but this way progress will be sent as rtp for each call, so as well in cases in which it is not necessary. So my idea was to set a channel variable PROGRESSINBAND to 1 in situations in which we use Progress() followed by Playback(). I've added some code to chan_pjsip.c which seems to work. I've opened a ticket here for two reasons: one: the "hack" could be usefull for someone else and second: I wanted to ask whether it is safe to do it the way I solved this or whether I'd need to lock channel->session->channel or something else first?
Comments:	By: Asterisk Team (asteriskteam) 2017-11-29 05:30:10.797-0600 Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution. A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report. Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process]. By: Benjamin Keith Ford (bford) 2017-11-29 09:28:03.324-0600 Hey [~nappsoft], thanks for taking the time to contribute back to Asterisk! If you would like some feedback on your patch, feel free to upload it to Gerrit following the guidelines in the comment above by clicking on the Patch Contribution Process link. That way, many developers will be able to help answer any questions you have! By: nappsoft (nappsoft) 2017-11-29 14:29:27.738-0600 Hi Benjamin. I know about gerrit, a patch of mine has just been checked into the code a few days ago ;) However in this case here I'm neither sure whether someone will think that this patch might be usefull nor did I do enough research about the locking topology. So I guess that Jira is a better place for "usefull or not" and "what's the clean way to do this" discussions than a code review tool... (Or do you think that gerrit should be polluted with simple suggestions?) By: Benjamin Keith Ford (bford) 2017-11-29 14:49:39.807-0600 Do you have an IRC account? If you don't, you can find more information on this page: https://wiki.asterisk.org/wiki/display/AST/IRC. The #asterisk-dev channel is a more appropriate place to ask code-related questions, and you will get feedback much faster this way. By: Joshua C. Colp (jcolp) 2017-12-05 12:41:44.436-0600 You can also use the asterisk-dev mailing list[1] to elicit feedback on this. Personally I'm uncertain about it over all. How did chan_sip handle it? [1] http://lists.digium.com/mailman/listinfo/asterisk-dev By: nappsoft (nappsoft) 2017-12-21 00:49:40.106-0600 Just added the version of the patch currently in production. Do you think that this might be of use and submitting it with gerrit is worth it? By: Joshua C. Colp (jcolp) 2017-12-22 06:38:13.295-0600 I think some people could be interested, but I'd also be interested in knowing how chan_sip handled your scenario or if this is relevant to both. By: nappsoft (nappsoft) 2017-12-22 09:01:01.305-0600 ok, I'll test this with our Asterisk 11/chan_sip implementation during the next days. will let you know what happens there. By: nappsoft (nappsoft) 2018-01-04 04:31:33.909-0600 Hey Joshua. I did some tests on a Asterisk11/chan_sip based system with progressinband not set. chan_sip sends the ringing notification automatically inband after audio has been sent as early media (for example with Progress() followed by Plaback with the noanswer flag set). So this seems to be a behavior change when migrating from chan_sip to chan_pjsip which I am able to work around with my patch. (Of course adding the chan_sip behavior to chan_pjsip would be a cleaner solution) By: Joshua C. Colp (jcolp) 2018-01-04 05:33:27.872-0600 I think having an option on an endpoint in chan_pjsip to do this would be good then to mirror the behavior, but if you'd like to put your current change up you can. By: Asterisk Team (asteriskteam) 2018-01-18 12:00:01.202-0600 Suspended due to lack of activity. This issue will be automatically re-opened if the reporter posts a comment. If you are not the reporter and would like this re-opened please create a new issue instead. If the new issue is related to this one a link will be created during the triage process. Further information on issue tracker usage can be found in the Asterisk Issue Guidlines [1]. [1] https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines