[Home]

Summary:ASTERISK-16919: [patch] No MOH When Call Parked
Reporter:Francesco Romano (francesco_r)Labels:
Date Opened:2010-11-05 05:01:12Date Closed:2011-02-28 13:09:32.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Resources/res_features
Versions:Frequency of
Occurrence
Related
Issues:
cannot be resolved before completion ofASTERISK-17474 when using mpg123 as a streaming MOH source, issuing 'moh reload' from CLI causes stream to die
Environment:Attachments:( 0) patch_park_moh-1.4.38.txt
( 1) patch_park_moh-1.4.39-rc1.txt
( 2) patch_park_moh-trunk.txt
( 3) patch_park_moh-trunk-2.txt
Description:I have upgraded from Asterisk 1.4.34 to Asterisk 1.6.2.14-rc1 using the same configuration (FreePBX 2.7) but now when i park a call there is no music on hold. The moh works with regular sip hold and transfers and queues. This is the console output:
   -- Started music on hold, class 'default', on SIP/503-00000760
 == Parked SIP/503-00000760 on 71 (lot default). Will timeout back to extension [from-internal] 509, 1 in 240 seconds
   -- Added extension '71' priority 1 to parkedcalls (0x8820198)
   -- <SIP/509-00000761> Playing 'digits/7.slin' (language 'it')
   -- <SIP/509-00000761> Playing 'digits/1.slin' (language 'it')

As you can see the default music class start but the parked person hear only silence. I'm not the only person that have this problem, other persons using Asterisk 1.8.0 have reported the same issue:
http://pbxinaflash.com/forum/showthread.php?p=52404
http://www.freepbx.org/trac/ticket/4623
Comments:By: Matt Kemner (mkemner) 2010-11-12 03:51:56.000-0600

I had the same issue on my 1.8.0 test server.

It seems to be related to the dahdi timer (res_timing_dahdi)

After switching to res_timing_pthread (for other reasons) suddenly I get music on hold with parked calls.
By: Francesco Romano (francesco_r) 2010-11-12 04:56:28.000-0600

You are right MKemner, i have unload res_timing_dahdi and now moh works.
By: Leif Madsen (lmadsen) 2010-11-23 13:34:11.000-0600

I'll acknowledge this so someone else from the development team can look at this to determine if documentation needs to be created or a bug exists here.
By: Andrew Latham (lathama) 2010-12-15 13:59:19.000-0600

Just hit this on an install with 1.8.1 and DAHDI 2.4.0

This is only affecting parking MOH at the moment..  We tested every other case.
By: Joe Cracchiolo (jjcinaz) 2010-12-16 12:47:09.000-0600

This is also affecting 1.4.38 and DAHDI 2.3.0 or 2.4.0 on a system with DAHDI_DUMMY only.  Unable to test this with a real timing source at this time.

This problem does not occur with 1.4.33 and DAHDI 2.4.0.
By: Christian Jacobsen (cjacobsen) 2010-12-16 13:09:31.000-0600

the workaround works:

noload => res_timing_dahdi.so
By: Joe Cracchiolo (jjcinaz) 2010-12-16 13:14:36.000-0600

That workaround is not possible in 1.4.x
By: Robert Frantik (rfrantik) 2010-12-16 17:14:40.000-0600

Running 1.6.2.14 and ran into this problem today.  We are testing a multi-tenant system and context is very important.  Has anyone noticed a context shift once they did the noload => res_timing_dahdi.so?

Earlier when I was testing and parked a call I saw the following trace:
-- Started music on hold, class 'default-tenant1', on SIP/142-tenant1-00000047

Now I see the following:
-- Started music on hold, class 'default', on SIP/142-tenant1-00000047

So while I do have moh for the parked callers, I no longer seem to have the control that it did.  This same type of trace info also comes up just when placing a caller on hold as well.  I used to have a unique context, now I just get 'default'.

So to me this appears to just be a work around?  Or there was a major change from 1.6.2.13 to 1.6.2.14?  I'm also no expert at this, it could be that i now have to make an adjustment someplace else.
By: Joe Cracchiolo (jjcinaz) 2010-12-17 14:59:31.000-0600

The problem looks to be that the DAHDI dummy timer sends the timer messages via an out-of-band or priority I/O as opposed to an error I/O.  The following code in the parking thread doesn't allow the message to come through, thus the channel's exception flag doesn't get set and the ast_read() doesn't call the MOH generator:

if (!(fds[y].revents & (POLLIN | POLLERR))) {
/* Next x */
continue;
}
if (fds[y].revents & POLLERR) {

Changing to the following causes MOH to work again:

if (!(fds[y].revents & (POLLIN | POLLERR | POLLPRI))) {
/* Next x */
continue;
}
if (fds[y].revents & (POLLERR | POLLPRI)) {

I've attached patches for 1.4.38, 1.4.39-rc1, and trunk to fix this.
By: Andrew Latham (lathama) 2010-12-17 15:07:01.000-0600

Maybe related https://issues.asterisk.org/view.php?id=18457
By: Joe Cracchiolo (jjcinaz) 2010-12-17 15:13:49.000-0600

I don't believe it's related to 18457.  That issue has to do with the APP class of MOH where an external application is feeding in data.  This issue would occur with the Files class of MOH which doesn't open any DAHDI handles.
By: Robert Frantik (rfrantik) 2010-12-17 16:07:24.000-0600

I'm assuming I can change the code and recompile... but which file has this part of the code in it?  running v1.6.2.14.  Thanks
By: Joe Cracchiolo (jjcinaz) 2010-12-17 17:32:22.000-0600

Sorry, I didn't work up a patch for 1.6.2, but you can find the code in the function manage_parkinglot() in the main/features.c file.
By: Robert Frantik (rfrantik) 2010-12-17 21:48:14.000-0600

Yep, I found it in main/features.c and it looks like it starts on line 3193... it does appear to be slightly different.

if (!(pfds[y].revents & (POLLIN | POLLERR))) {
/* Next x */
continue;
}
if (pfds[y].revents & POLLERR) {
ast_set_flag(chan, AST_FLAG_EXCEPTION);
} else {
ast_clear_flag(chan, AST_FLAG_EXCEPTION);
}
chan->fdno = x;

I changed the two lines as follows...

if (!(pfds[y].revents & (POLLIN | POLLERR | POLLPRI))) {
               /* Next x */
               continue;
               }
          if (pfds[y].revents & (POLLERR | POLLPRI)) {

I saved it... did a Make Clean, Make Install, and rebooted.  Didn't seem to help, but I'm not sure I did it all correctly.  Thanks!
By: Andrew Latham (lathama) 2010-12-20 08:30:34.000-0600

Just tested in 1.8.1 with no effect.  :(
By: Christian Jacobsen (cjacobsen) 2010-12-20 09:13:02.000-0600

In trunk, i've changed the patch this way (patch_park_moh-trunk-2.txt) to make it work.


By: Mitch Sharp (bluecrow76) 2010-12-22 11:53:57.000-0600

I just verified the latest patch works against my lab 1.8.0 installation.  I will be testing against 1.6.2.14 and 1.6.2.15 tonight.
By: Robert Frantik (rfrantik) 2010-12-22 11:56:58.000-0600

bluecrow76...

Is "the latest patch" the same patch JJCinAZ posted on 12/17?  Can you let us know how you applied it?

Thank.
By: Mitch Sharp (bluecrow76) 2010-12-22 12:01:25.000-0600

The code in patch_park_moh-trunk-2.txt posted on 12/20 is what I used, which has a third change that the other patches above do not have.  I just manually edited main/features.c and added the POLLPRI value to the appropriate lines, so technically I didn't "apply the patch."  I tested the code.
By: Mitch Sharp (bluecrow76) 2010-12-22 12:03:27.000-0600

I just used the "wget patch" code above to patch 1.6.2.15 and it applied fine:

wget 'https://issues.asterisk.org/file_download.php?file_id=27993&type=bug' -O - | patch -p0

patching file main/features.c
Hunk #1 succeeded at 3190 (offset -606 lines).
Hunk #2 succeeded at 3853 (offset -2 lines).
By: Robert Frantik (rfrantik) 2010-12-22 13:27:24.000-0600

Thanks for the patch info... I applied it and got the same output you did... so it should have worked?  Is there one more step I need to do?  Or does this mean Asterisk is now running with the new code?
By: Mitch Sharp (bluecrow76) 2010-12-22 14:34:02.000-0600

You have patched source code, now recompile and reinstall:

make
make install


By: Robert Frantik (rfrantik) 2010-12-22 15:39:16.000-0600

Yep, still new to all this. Got it working now on my test machine and production machine... v1.6.2.14 and v1.6.2.15.  My MOH context's seem to be back to normal as well.
By: Francesco Romano (francesco_r) 2010-12-24 09:10:42.000-0600

I have patched Asterisk 1.6.2.16-rc1 with patch_park_moh-trunk-2.txt and now i have moh using dahdi timing. Thank you
By: Steve Davies (one47) 2010-12-30 06:00:46.000-0600

The supplied patch seems to set an exception on the channel if POLLPRI is received. Is that correct? If I understand this correctly, POLLPRI is equivalent to POLLIN, and therefore should not
  ast_set_flag(chan, AST_FLAG_EXCEPTION)
I have applied the patch here without that part of the change, and it does indeed fix MOH for parked calls.
By: Shaun Ruffell (sruffell) 2010-12-30 11:43:59.000-0600

one47: +1  (patch without exception looks like the best solution to me)

Just taking a note for myself / the future:

I looked through the drivers to see about adding the POLLIN flag when a timer event has fired, but it doesn't appear to be the right thing.  POLLPRI has been returned since http://svn.asterisk.org/view/zaptel?view=revision&revision=158 and as far as I can tell the rationale appears that Zaptel / DAHDI uses POLLPRI to indicate there are events on an event queue.   The timer returns DAHDI_TIMER_EVENT_EXPIRED in response to the DAHDI_GETEVENT call since asterisk doesn't "read" from the open timer file descriptor.

I've linked this to ASTERISK-16398 since that appears when the change in user space behavior came in.
By: Digium Subversion (svnbot) 2011-01-20 13:56:36.000-0600

Repository: asterisk
Revision: 303106

U   branches/1.6.2/main/features.c

------------------------------------------------------------------------
r303106 | sruffell | 2011-01-20 13:56:35 -0600 (Thu, 20 Jan 2011) | 15 lines

main/features: Use POLLPRI when waiting for events on parked channels.

This change resolves a regression in the 1.6.2 when converting from
select to poll.  The DAHDI timers use POLLPRI to indicate that the timer
fired, but features was not waiting for that flag.  The result was no
audio for MOH when a call was parked and res_timing_dahdi was in use.

This patch is slightly modified from the one on the mantis issue.  It does
not set an exception on the channel if the POLLPRI flag is set.

(closes issue ASTERISK-16919)
Reported by: francesco_r
Patches:
     patch_park_moh-trunk-2.txt uploaded by cjacobsen (license 1029)
     Tested by: francesco_r, rfrantik, one47
------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=303106
By: Digium Subversion (svnbot) 2011-01-20 13:57:32.000-0600

Repository: asterisk
Revision: 303107

_U  branches/1.8/
U   branches/1.8/main/features.c

------------------------------------------------------------------------
r303107 | sruffell | 2011-01-20 13:57:31 -0600 (Thu, 20 Jan 2011) | 23 lines

Merged revisions 303106 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.6.2

........
 r303106 | sruffell | 2011-01-20 13:56:34 -0600 (Thu, 20 Jan 2011) | 15 lines
 
 main/features: Use POLLPRI when waiting for events on parked channels.
 
 This change resolves a regression in the 1.6.2 when converting from
 select to poll.  The DAHDI timers use POLLPRI to indicate that the timer
 fired, but features was not waiting for that flag.  The result was no
 audio for MOH when a call was parked and res_timing_dahdi was in use.
 
 This patch is slightly modified from the one on the mantis issue.  It does
 not set an exception on the channel if the POLLPRI flag is set.
 
 (closes issue ASTERISK-16919)
 Reported by: francesco_r
 Patches:
       patch_park_moh-trunk-2.txt uploaded by cjacobsen (license 1029)
       Tested by: francesco_r, rfrantik, one47
........

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

http://svn.digium.com/view/asterisk?view=rev&revision=303107
By: Digium Subversion (svnbot) 2011-01-20 13:58:55.000-0600

Repository: asterisk
Revision: 303108

_U  trunk/
U   trunk/main/features.c

------------------------------------------------------------------------
r303108 | sruffell | 2011-01-20 13:58:54 -0600 (Thu, 20 Jan 2011) | 30 lines

Merged revisions 303107 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8

................
 r303107 | sruffell | 2011-01-20 13:57:31 -0600 (Thu, 20 Jan 2011) | 23 lines
 
 Merged revisions 303106 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.6.2
 
 ........
   r303106 | sruffell | 2011-01-20 13:56:34 -0600 (Thu, 20 Jan 2011) | 15 lines
   
   main/features: Use POLLPRI when waiting for events on parked channels.
   
   This change resolves a regression in the 1.6.2 when converting from
   select to poll.  The DAHDI timers use POLLPRI to indicate that the timer
   fired, but features was not waiting for that flag.  The result was no
   audio for MOH when a call was parked and res_timing_dahdi was in use.
   
   This patch is slightly modified from the one on the mantis issue.  It does
   not set an exception on the channel if the POLLPRI flag is set.
   
   (closes issue ASTERISK-16919)
   Reported by: francesco_r
   Patches:
         patch_park_moh-trunk-2.txt uploaded by cjacobsen (license 1029)
         Tested by: francesco_r, rfrantik, one47
 ........
................

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

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