Summary:ASTERISK-01481: [patch] remove buffer from mpg123 so that it will never suck all cpu time
Reporter:sbingner (sbingner)Labels:
Date Opened:2004-04-28 00:48:52Date Closed:2004-09-25 02:49:37
Versions:Frequency of
Environment:Attachments:( 0) nobufopt2.diff.txt
( 1) res_musiconhold-nobuffer.diff
Description:This patch changes res_musiconhold to not use the -b option to mpg123.  This keeps mpg123 from spawning off a second copy of itself, and allows res_musiconhold to kill it off when it stops.  If the buffer was needed for some reason, this should not be applied... but if not it will fix 99% of the mpg123 problems.

****** STEPS TO REPRODUCE ******

start asterisk
killall -9 asterisk

mpg123 will hang with 100% cpu time
Comments:By: twisted (twisted) 2004-04-28 01:27:23

Tested and works fine, but with patch installed i get jittery MOH.  Otherwise works as expected.  

I would not reccomend this for CVS as systems with lower specs may not be able to handle playing without jitters in the MOH.

edited on: 04-28-04 01:42

By: Brian West (bkw918) 2004-04-28 01:40:35

works prefect here.. only 1 mpg123 process now :)

By: sbingner (sbingner) 2004-04-28 05:21:20

singleopt.diff.txt has the same functionality, but it is off by default.  It can be enable by adding "nb" (for "No Buffer") to the end of the class eg: mp3nb:/mp3s,-Z

By: Mark Spencer (markster) 2004-04-28 11:30:14

Eh, the string compares give me a little bit of heartburn because they're not exact.  Fix up the handling of the optional mode and i'll be happy to add it in.

By: sbingner (sbingner) 2004-04-28 15:14:20

Updated to use an additional option "nobuff"... is that better?

normal => mp3:/mp3s
random => mp3:/mp3s,-z
nobuff => mp3:/mp3s,nobuff
nobuffrandom => mp3:/mp3s,-z,nobuff

By: Mark Spencer (markster) 2004-04-28 15:57:40

No, this isn't really any good either.  I think the original concept of mp3nb and quietmp3nb are fine, but they need real strcasecmp's just like the others.  httpmp3 always needs buffering.

By: sbingner (sbingner) 2004-04-28 16:36:02

This should do it...  considered using a strncasecmp(mode, "quiet", 5) for quiet but I was afraid you wouldn't like that either ;)

For my edification, is there a reason strncasecmp is bad?

By: Mark Spencer (markster) 2004-04-28 16:42:30

Not in-and-of-itself, but "mp3nb" and "mp3foonb" are not the same thing, but the test for "mp3" at the beginning and "nb" at the end of both will pass, suggesting that it is.

By: Mark Spencer (markster) 2004-04-28 16:43:09

One last thing, include in your patch an update to the sample config, if you don't mind.

By: sbingner (sbingner) 2004-04-28 17:18:53

I did already :)

By: twisted (twisted) 2004-04-29 00:13:36

Fixed in cvs by Markster