[Home]

Summary:ASTERISK-14033: [patch] [regression] #0013747 not fixed for local channel (Indications are not passed from old peer to new peer during masquerad
Reporter:David Woolley (davidw)Labels:
Date Opened:2009-04-29 06:38:49Date Closed:2010-02-16 17:07:00.000-0600
Priority:MajorRegression?Yes
Status:Closed/CompleteComponents:Channels/chan_local
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) Issue14992-trunk.patch
Description:The lack of ring back tone on a ringing call transferred to a parking call is still not working in the case that interests us, and for which we produced a test case in comment 94129 of issues ASTERISK-12930.  Looking at http://reviewboard.digium.com/r/90/ it seems that it was only tested in the simpler case, where the ringing channel is a real channel.  Our case uses a local channel.

****** ADDITIONAL INFORMATION ******

The basic problem is that the -1 side of the local channel is left in a Down state, rather than a Ringing state, but the fix only checks the immediate peer for a Ringing state before regenerating the indication.

These are the channel states, for the test case, just before the transfer:

Channel              Location             State   Application(Data)            
SIP/xxxxxxxxxxxxx-08 *6185@yyyyyyyyyyyyyy Ringing AppDial((Outgoing Line))      
Local/£6185@default- £6185@default:1      Ring    Wait(3)                      
Local/£6185@default- 6185@default:1       Down    AppDial((Outgoing Line))      
Local/*6185@default- *6185@default:2      Ring    Dial(SIP/xxxxxxxxxxxxx,15)    
Local/*6185@default- 6185@default:1       Down    AppDial((Outgoing Line))      
SIP/6106-08473608    6185@internal:1      Ring    Dial(Local/*6185@default&Local
SIP/6105-08487740    s@yyyyyyyyyyyyyyyy:1 Up      Parked Call()                

xxxxxxxxxxxxx corresponds to 4222 in the test case.
6105 is the parked extension and 6106 is the one calling 6185 to run the test.
hash has been replaced by pounds to avoid being treated as an issue number marker.

Our original reason for using a local channel here was as a workround for ASTERISK-1247548, however, we have subsequently come to rely on the extra level of indirection to keep control of the call.
Comments:By: Leif Madsen (lmadsen) 2009-06-01 14:03:54

Just wanted to ping this issue as it's target release is 1.6.0.10. Thanks!
By: David Woolley (davidw) 2009-10-29 13:45:46

I think that the patch for ASTERISK-13788 adds a new case where the presence of a local channel may prevent an indication, in this case music on hold, being re-instated on a new channel when a local channel is masqueraded.

I would expect if you connected through a, non-optimised, local channel, using the new m option, the destination channel went into hold, and you then transferred the ;1 side of the local channel to another channel, the new peer of the ;1 side would not receive MOH.  In fact, I think they would receive no RTP at all, so might timeout.
By: David Woolley (davidw) 2009-10-30 06:32:47

I think, in the MOH case, one has to transfer the ;2 side for things to go funny.
By: Digium Subversion (svnbot) 2010-02-04 17:20:23.000-0600

Repository: asterisk
Revision: 244785

U   branches/1.4/channels/chan_local.c

------------------------------------------------------------------------
r244785 | jpeeler | 2010-02-04 17:20:22 -0600 (Thu, 04 Feb 2010) | 22 lines

Change channel state on local channels for busy,answer,ring.

Previously local channels channel state never changed. This became problematic
when the state of the other side of the local channel was lost, for example
during a masquerade. Changing the state of the local channel allows for the
scenario to be detected when the channel state is set to ringing, but the peer
isn't ringing. The specific problem scenario is described in 164201. Although
this was noted on one of the issues, here is the tested dialplan verified to
work:

exten => 9700,1,Dial(Local/*9700@default&Local/ASTERISK-9414@default)

exten => *9700,1,Set(GLOBAL(TESTCHAN)=${CHANNEL:0:${MATH(${LEN(${CHANNEL})}-1):0:2}}1)
exten => *9700,n,wait(3) ;3 works, 1 did not
exten => *9700,n,Dial(SIP/5001)

exten => ASTERISK-9414,1,Wait(1) ;1 works, 3 did not
exten => ASTERISK-9414,n,ChannelRedirect(${TESTCHAN},parkedcalls,701,1)

(closes issue ASTERISK-14033)
Reported by: davidw

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=244785
By: Digium Subversion (svnbot) 2010-02-04 17:21:51.000-0600

Repository: asterisk
Revision: 244792

_U  trunk/

------------------------------------------------------------------------
r244792 | jpeeler | 2010-02-04 17:21:50 -0600 (Thu, 04 Feb 2010) | 28 lines

Blocked revisions 244785 via svnmerge

........
 r244785 | jpeeler | 2010-02-04 17:20:21 -0600 (Thu, 04 Feb 2010) | 22 lines
 
 Change channel state on local channels for busy,answer,ring.
 
 Previously local channels channel state never changed. This became problematic
 when the state of the other side of the local channel was lost, for example
 during a masquerade. Changing the state of the local channel allows for the
 scenario to be detected when the channel state is set to ringing, but the peer
 isn't ringing. The specific problem scenario is described in 164201. Although
 this was noted on one of the issues, here is the tested dialplan verified to
 work:
 
 exten => 9700,1,Dial(Local/*9700@default&Local/ASTERISK-9414@default)
 
 exten => *9700,1,Set(GLOBAL(TESTCHAN)=${CHANNEL:0:${MATH(${LEN(${CHANNEL})}-1):0:2}}1)
 exten => *9700,n,wait(3) ;3 works, 1 did not
 exten => *9700,n,Dial(SIP/5001)
 
 exten => ASTERISK-9414,1,Wait(1) ;1 works, 3 did not
 exten => ASTERISK-9414,n,ChannelRedirect(${TESTCHAN},parkedcalls,701,1)
 
 (closes issue ASTERISK-14033)
 Reported by: davidw
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=244792
By: David Woolley (davidw) 2010-02-09 11:13:55.000-0600

The change failed to merge into trunk.  A patch from the change applies OK, but it won't compile and, in the SVN, only the revision history meta data was updated:

chan_local.c:261: error: switch quantity not an integer

on this line:

                       switch (f->subclass) {


By: David Woolley (davidw) 2010-02-09 11:44:38.000-0600

I've uploaded a tweaked patch that does compile on trunk.  The only change I've made is to account for subclass being a union.  I haven't tested it beyond confirming that it will compile.
By: David Woolley (davidw) 2010-02-09 12:00:30.000-0600

Note that 1.6.0 and 1.6.1 need to be merged from 1.4, not from trunk, as they do not have the union.  I haven't checked 1.6.2.
By: Jeff Peeler (jpeeler) 2010-02-09 13:18:54.000-0600

I thought I tested trunk and the change did not need to be applied there, which is why I blocked the change from trunk. Are you seeing a problem present for sure in trunk as well?
By: David Woolley (davidw) 2010-02-09 13:30:34.000-0600

No.  I haven't tested on trunk.  I think I may not have understood the significance of "blocked" on the svnbot output.  (It is confusing when you still get the full details in the log.)

The problem definitely applies on 1.6.1.x, and it looks like the block on trunk has stopped it being propagated to that branch.

If it really isn't needed for trunk, it would have helped if that had been included as a bug note, as I've found several cases where changes appear not to have propagated purely due to conflicting changes, unrelated to the issue in question.
By: Jeff Peeler (jpeeler) 2010-02-09 14:00:34.000-0600

Ok, apparently I messed this up. All the branches need it after all. So I'll fix it right up. Thanks for letting me know.

The merging process is manual. There are no automatic changes propagated to the other branches, so it's entirely my fault. However, when a change is blocked that information is logged to the bug note as you see above.
By: Digium Subversion (svnbot) 2010-02-10 10:47:39.000-0600

Repository: asterisk
Revision: 246070

U   trunk/channels/chan_local.c

------------------------------------------------------------------------
r246070 | jpeeler | 2010-02-10 10:47:38 -0600 (Wed, 10 Feb 2010) | 22 lines

Change channel state on local channels for busy,answer,ring.
 
Previously local channels channel state never changed. This became problematic
when the state of the other side of the local channel was lost, for example
during a masquerade. Changing the state of the local channel allows for the
scenario to be detected when the channel state is set to ringing, but the peer
isn't ringing. The specific problem scenario is described in 164201. Although
this was noted on one of the issues, here is the tested dialplan verified to
work:

exten => 9700,1,Dial(Local/*9700@default&Local/0009700@default)

exten => *9700,1,Set(GLOBAL(TESTCHAN)=${CHANNEL:0:${MATH(${LEN(${CHANNEL})}-1):0:2}}1)
exten => *9700,n,wait(3) ;3 works, 1 did not
exten => *9700,n,Dial(SIP/5001)

exten => 0009700,1,Wait(1) ;1 works, 3 did not
exten => 0009700,n,ChannelRedirect(${TESTCHAN},parkedcalls,701,1)

(closes issue ASTERISK-14033)
Reported by: davidw

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=246070
By: Digium Subversion (svnbot) 2010-02-10 10:53:01.000-0600

Repository: asterisk
Revision: 246071

_U  branches/1.6.0/
U   branches/1.6.0/channels/chan_local.c

------------------------------------------------------------------------
r246071 | jpeeler | 2010-02-10 10:53:00 -0600 (Wed, 10 Feb 2010) | 29 lines

Merged revisions 246070 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

........
 r246070 | jpeeler | 2010-02-10 10:47:37 -0600 (Wed, 10 Feb 2010) | 22 lines
 
 Change channel state on local channels for busy,answer,ring.
   
 Previously local channels channel state never changed. This became problematic
 when the state of the other side of the local channel was lost, for example
 during a masquerade. Changing the state of the local channel allows for the
 scenario to be detected when the channel state is set to ringing, but the peer
 isn't ringing. The specific problem scenario is described in 164201. Although
 this was noted on one of the issues, here is the tested dialplan verified to
 work:
 
 exten => 9700,1,Dial(Local/*9700@default&Local/0009700@default)
 
 exten => *9700,1,Set(GLOBAL(TESTCHAN)=${CHANNEL:0:${MATH(${LEN(${CHANNEL})}-1):0:2}}1)
 exten => *9700,n,wait(3) ;3 works, 1 did not
 exten => *9700,n,Dial(SIP/5001)
 
 exten => 0009700,1,Wait(1) ;1 works, 3 did not
 exten => 0009700,n,ChannelRedirect(${TESTCHAN},parkedcalls,701,1)
 
 (closes issue ASTERISK-14033)
 Reported by: davidw
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=246071
By: Digium Subversion (svnbot) 2010-02-10 10:55:28.000-0600

Repository: asterisk
Revision: 246072

_U  branches/1.6.1/
U   branches/1.6.1/channels/chan_local.c

------------------------------------------------------------------------
r246072 | jpeeler | 2010-02-10 10:55:28 -0600 (Wed, 10 Feb 2010) | 29 lines

Merged revisions 246070 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

........
 r246070 | jpeeler | 2010-02-10 10:47:37 -0600 (Wed, 10 Feb 2010) | 22 lines
 
 Change channel state on local channels for busy,answer,ring.
   
 Previously local channels channel state never changed. This became problematic
 when the state of the other side of the local channel was lost, for example
 during a masquerade. Changing the state of the local channel allows for the
 scenario to be detected when the channel state is set to ringing, but the peer
 isn't ringing. The specific problem scenario is described in 164201. Although
 this was noted on one of the issues, here is the tested dialplan verified to
 work:
 
 exten => 9700,1,Dial(Local/*9700@default&Local/0009700@default)
 
 exten => *9700,1,Set(GLOBAL(TESTCHAN)=${CHANNEL:0:${MATH(${LEN(${CHANNEL})}-1):0:2}}1)
 exten => *9700,n,wait(3) ;3 works, 1 did not
 exten => *9700,n,Dial(SIP/5001)
 
 exten => 0009700,1,Wait(1) ;1 works, 3 did not
 exten => 0009700,n,ChannelRedirect(${TESTCHAN},parkedcalls,701,1)
 
 (closes issue ASTERISK-14033)
 Reported by: davidw
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=246072
By: Digium Subversion (svnbot) 2010-02-10 10:58:55.000-0600

Repository: asterisk
Revision: 246073

_U  branches/1.6.2/
U   branches/1.6.2/channels/chan_local.c

------------------------------------------------------------------------
r246073 | jpeeler | 2010-02-10 10:58:55 -0600 (Wed, 10 Feb 2010) | 29 lines

Merged revisions 246070 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

........
 r246070 | jpeeler | 2010-02-10 10:47:37 -0600 (Wed, 10 Feb 2010) | 22 lines
 
 Change channel state on local channels for busy,answer,ring.
   
 Previously local channels channel state never changed. This became problematic
 when the state of the other side of the local channel was lost, for example
 during a masquerade. Changing the state of the local channel allows for the
 scenario to be detected when the channel state is set to ringing, but the peer
 isn't ringing. The specific problem scenario is described in 164201. Although
 this was noted on one of the issues, here is the tested dialplan verified to
 work:
 
 exten => 9700,1,Dial(Local/*9700@default&Local/0009700@default)
 
 exten => *9700,1,Set(GLOBAL(TESTCHAN)=${CHANNEL:0:${MATH(${LEN(${CHANNEL})}-1):0:2}}1)
 exten => *9700,n,wait(3) ;3 works, 1 did not
 exten => *9700,n,Dial(SIP/5001)
 
 exten => 0009700,1,Wait(1) ;1 works, 3 did not
 exten => 0009700,n,ChannelRedirect(${TESTCHAN},parkedcalls,701,1)
 
 (closes issue ASTERISK-14033)
 Reported by: davidw
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=246073