[Home]

Summary:ASTERISK-04182: [request] A hook flash sent using RTP for telephony signals (RFC2833) does not flash zap channel.
Reporter:kenalker (kenalker)Labels:
Date Opened:2005-05-16 02:06:50Date Closed:2011-06-07 14:05:05
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:
Description:A hook flash sent using RTP for telephony signals (RFC2833) does not flash zap channel.  Only one SIP device and one ZAP channel are necessary to reproduce this problem.

The telco supplying the PSTN line plugged into an Asterisk (Zap) FXO port offers call feature services that must be activated via a flash hook (for example, "flashing" the line to answer call waiting, or to dial the second leg of a three-way-call, or to dial CLASS codes to toggle telco features while on a call).

Problem was discovered when using a Linksys PAP2-NA SIP phone adapter (aka "ATA") and an analog phone to send a hook flash signal to Asterisk.

When the analog phone is flashed, the Linksys generates an RTP "DTMF named event" 16 (per RFC2833).  Asterisk receives three RTP packets, each with event 16, and each with the "end" bit set.

The function process_rfc2833() in rtp.c processes these packets.  I believe that the way process_rfc2833() is written, it expects that a packet for an event will always be received without the "end" bit set before a packet for that same event is received with the "end" bit set.  For example, when pressing a DTMF key on an analog phone connected to a Linksys PAP2-NA, many RTP packets are received by Asterisk, the last three of which have the "end" bit set.  When process_rfc2833() sees a packet without the "end" bit set it creates a state where it then waits for a packet with the "end" bit set, at which point it calls send_dtmf() and the DTMF tone is generated out the Zap channel properly by Asterisk.

When an RTP flash packet is generated by the Linksys, process_rfc2833() is executed but never sees a packet without an "end" bit, so it never sets itself up to look for a packet with an "end" bit, and thus, never calls send_dtmf(), and thusly, Asterisk never flashes the Zap channel.

I wrote four lines of code to fix this problem, however, I do not know enough about the interworkings of process_2833(), RFC2833, and send_dtmf() to produce code that would cure this problem without introducing other problems.  I'd be more than happy to collaborate with the person who wrote this chunk of code to help fix and test this problem, but I don't know how to find out who that person might be.

Note that I am assuming that the Linksys is not violating protocol by sending RTP packets with the "end" bit set without having first sent RTP packets for the same event without the "end" bit set.  Hopefully, someone with more experience with RTP can evaluate this assumption.

Lastly, I believe it will be important to keep in mind the fact that multiple RTP flash packets are sent after a single flash of the analog phone, and, thus, it will be important to ensure that the modified code does not flash the ZAP channel multiple times.

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

In order to remove the Linksys PAP2-NA from the media path so it does not try to interpret the flash hook from the analog phone (but instead passes it through to Asterisk), the below settings were made in the Linksys PAP2-NA:

Hook_Flash_Tx_Method = AVT
Call_Waiting_Serv = No
Three_Way_Call_Serv = No
Three_Way_Conf_Serv = No

Comments:By: kenalker (kenalker) 2005-05-16 02:22:23

*CLI> show version
Asterisk CVS-HEAD-05/14/05-19:23:35 built by root@xxxxxxxx on a i686 running Linux
By: Olle Johansson (oej) 2005-05-17 16:59:25

Hook flash is an AST_CONTROL event. I can't easily figure out how to send that from rtp over chan_sip to the PBX, but that's what we need to do in order to support this. Guess we also need to support incoming AST_CONTROL_FLASH events from other channels and send that along to the device.

However, the FLASH event is normally handled locally by the SIP device so that it is documented in the RFC outside of the required DTMF events to support since so few devices/users need it as a RTP event. Doesn't say we can't do this or won't do this, but it may be an explanation why we don't support it today... :-)
By: Russell Bryant (russell) 2005-05-19 01:30:20

This is really more of a feature request than a bug ... I'm marking this down from major
By: kenalker (kenalker) 2005-05-19 11:25:55

oej:  In the CVS verision I'm using, someone added the AST_CONTROL_FLASH event to chan_zap.c already.  Were you looking at the CVS I'm using (it's not in V1-stable)?  I'm not following you when you say it needs to be sent over chan_sip.  It is actually already supported in CVS, but the code in chan_zap.c isn't executed due to the way process_rfc2833() (in rtp.c) is written (since the Linksys PAP2-NA only sends the RTP flash packet with no "end" bit set).

drumkilla:  I'd agree that this could be minor and not major (I made it major based on the strict definitions in the bug guidelines), however, I would not agree that it is a feature request simply because this feature already exists and is coded.  However, I believe the code is most likely flawed, and thus, the intended implementation isn't taking the correct path through the code.  Finally, I beleive the code may actually be violating protocol and will cause future problems whenever receiving an rtp message for any "short/instantaneous" event/command (not just FLASH) rather than a string of rtp packets due to a "long" event (such as a DTMF button press).

I should have given a code example, but it is actually very complex, imho, and would be easier to discect verbally, but I'm going to attempt in my next NOTE to show that the problem is more of a bug than a feature request.  I'm new at this (lurking for >1yr), so I'm not arguing to change the status nor do I want to set off a political debate; just trying to clarify my original submission.
By: kenalker (kenalker) 2005-05-19 12:45:04

In process_rfc2833() (in rtp.c), incoming rtp packets are parsed to find out what event was sent.  The event type is recorded in variable "resp".  In particular, "resp" is set to 'X' when the event is a FLASH.  After the event type is determined there is some "if, else if, else if, if" logic following to process the event.  In this logic, there are three places where send_dtmf() can be called (which is necessary to for the FLASH to occur on the zap channel), however, as I'll show, send_dtmf() is never called for a FLASH event as it is for DTMF events.

The best way to understand this is to keep in mind that (in my case, with my device): a) DTMF events work, b) FLASH events do not, and b) DTMF events come as multiple RTP packets where only the last few packets have the "end" bit set, d) a FLASH event generates three rtp FLASH packets, and all have the "end" bit set, e) process_dtmf() is called multiple times and different paths are taken depending on whether the event type has changed and whether or not the "end" bit is set (amongst other things).

The two pieces of code I'm going to concentrate on are the "else if(event_end & 0x80)" and the "if (!(event_end & 0x80))".  In my case, neither the first "if", nor the "if" dealing with "dtmfduration" are ever traversed, so I'm ignoring these for now (although, properly rewritten code will most likely need to consider these as well).

The "if(event_end & 0x80)" checks for the end of the event ("end" bit is set).  If the end of the event has occurred, we then check to see if variable "rtp->resp" was populated, and if so, send_dtmf() is called.  The "if (!(event_end & 0x80))" code checks to see if the packet is NOT signaling the end of the rtp event.  If this is the case, then "rtp->resp" is set equal to "resp", the current event type which was parsed at the beginning of the routine.  Understanding this is critical.

Based on the above paragraph, when DTMF rtp packets are received, the first string of packets that don't have the "end" bit set cause us to go through the "if (!(event_end & 0x80))" code and set "rtp->resp" to "resp".  Finally, when we get a packet with an "end" bit set, the "else if(event_end & 0x80)" is executed which calls send_dtmf(), but ONLY because rtp->resp was first populated in the "if (!(event_end & 0x80))" code.

When FLASH rtp packets are received, all packets have the "end" bit set (with my device), and, thus, the "if (!(event_end & 0x80))" is NEVER executed and "rtp->resp" is NEVER populated.  The only part of the code that is ever executed is the "else if(event_end & 0x80)", and it only calls send_dtmf() if rtp->resp is populated.

Thus, we have a situation where the code is obviously expecting that at some point the "if (!(event_end & 0x80))" section WILL be executed which must be based on the assumption that every string of rtp packets WILL have some packets without the "end" bit set.  This could be construed as protocol violation, but I'm not familiar enough with RFC2833 to argue this point.  I'm not sure it even covers this situation (after skimming the RFC several times), and if not, the code needs to deal with it.

In a situation like we have here, and may have for other non-DTMF events that don't come in waives of rtp packets, but instead are signalled by a single rtp packet (or three packets, in my case) that all have the "end" bit set (since the event is instantaneous and not long in duration), these event types will never get executed since send_dtmf() will never be called.

I wrote a few lines of code that force the FLASH to be executed simply to prove to myself that only process_rfc2833() needs updating and that the rest of the functionality necessary to FLASH the zap channel is already in the (CVS) code.  In fact, adding these few lines was all that was needed to get the FLASH to percolate all the way through to the ZAP channel.

Note that this code ONLY concerns itself with the FLASH case, not all the other possible non-DTMF cases.  This is definitely NOT the way to fix this problem; it is just a band-aide for this particular case.  We need someone more intimate with this code, and with RFC2833, to write a proper fix.  I'd love to help, but I'd need to consult with the original author (whomever that might be).

Here is what the code looked like originally:

       else if(event_end & 0x80)
       {
               if (rtp->resp) {
                       f = send_dtmf(rtp);
                       rtp->resp = 0;
               }
               resp = 0;
               duration = 0;
       }

and here is how I changed it:

       else if(event_end & 0x80)
       {
               ast_log(LOG_DEBUG, "RTP PACKET HAS end of event BIT SET!\n");      
               if (rtp->resp) {
                       f = send_dtmf(rtp);
                       rtp->resp = 0;
               }
               else if (resp == 'X')  {
                       rtp->resp = 'X';
                       f = send_dtmf(rtp);
                       rtp->resp = 0;
                       }
               resp = 0;
               duration = 0;
       }
By: Michael Jerris (mikej) 2005-05-19 12:51:34

oej, can you please take a look at this one and provide some comment.
By: Olle Johansson (oej) 2005-05-19 15:00:40

The SIP/RTP code does not parse the DTMF event and do not send the control frame across the channel to chan_zap. I've added a comment in rtp.c /cvs head/ so you see where we get the DTMF flash event. We need to find out how to get that event back to chan_sip from rtp.c and then send a control frame across the pbx. Some people might want that to be handled as a PBX transfer, I guess.

Guess we could handle that in the res_features code...
By: kenalker (kenalker) 2005-05-19 17:05:53

I think that the RTP code *is* parsing the DTMF event.  In rtp.c, you've added the "Event 16: Hook flash" comment exactly where resp is set to "X".  This code *is* executing and resp *is* being set to "X", in my case.  Then, send_dtmf() *should* be called at some point below that, but it is not.  Note that send_dtmf() *will* set rtp->f.frametype = AST_FRAME_CONTROL, and rtp->f.subclass = AST_CONTROL_FLASH when it is called with rtp->resp == 'X'.  Then, chan_zap.c sees these settings and FLASHes the Zap channel properly.  In this scenario, the key is to somehow get send_dtmf() called.  My comments above regarding the "end" flag detail why send_dtmf() is not being called.

The above procedure may not be the correct way to instruct the zap channel driver to flash the line, however, someone did (after V1; and possibly, incorrectly?) add code to send_dtmf() (in rtp.c) to differentiate between DTMF tones and CONTROL signals (FLASH, in this case) by setting the frametype and subclass appropriately.  Chan_zap.c has also been changed to FLASH the channel when frametype and subclass are set properly.

I'm not sure why chan_sip.c needs to be involved since everything is already in chan_zap.c (and rtp.c), unless you are suggesting a different way to implement this.
By: Michael Jerris (mikej) 2005-07-11 23:30:25

It appears that no one is willing to take this on.  I am going to suspend this request pending somone producing a patch for this functionality.  If you are unable to create a patch, I can suggest creating a bounty on the wiki or contracting the work out through an asterisk consultant\developer.

Thanks