[Home]

Summary:ASTERISK-02310: [patch] postgres music on hold
Reporter:constfilin (constfilin)Labels:
Date Opened:2004-09-01 03:00:22Date Closed:2011-06-07 14:05:25
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) astsql.h
( 1) res_musiconhold.c
( 2) res_musiconhold.c
( 3) res_musiconhold.patch
( 4) res_musiconhold.patch
Description:I integrated musiconhold.conf configuration with a postgres database and changed res_musiconhold.c code to re-read the database as necessary to
bring in the changes MOH classes.

To compile the new feature, please define POSTGRES_MOH at the beginning of
asterisk/res/Makefile. If POSTGRES_MOH is not defined then res_musiconhold.c works exactly as it used to.

Description of changes in /etc/asterisk/musiconhold.conf:

With this patch /etc/asterisk/musiconhold.conf can have 2 settings in [general]
section:

dboption              - this is postgres database connect string (E.G.:
                       hostaddr=1.2.3.4 dbname=foo user=bar password=baz)
get_mohclasses_query  - this is a database query that tells new code how to
                       get musiconhold classes from the database. E.G. :
                       SELECT * FROM asterisk_mohclasses. The returned record
                       set must contain columns
                       name - this is the name of MOH class
                       mode - mp3,quietmp3, custom etc
                       dir  - where to get mp3 files from
                       miscargs - other arguments to mpg123

Description of changes in asterisk/res/res_musiconhold.c

Each time when moh_alloc calls get_mohbyname(class), the MOH class is
looked for in the current list of "mohclasses". If MOH class is not found
in the list, a special procedure "sync_moh_classes" is started to re-read
the database and sync "mohclasses" with database content. After that
get_mohbyname attempts to find the class one more time.

Also, thread access to mohclass structure is now serialized by a mutex
that is a member of the mohclass structure, not by the global mutex. This
change increases the amount of parallelism and reduces the chance of
deadlocks.

The patch is not really tied up with Postgres, it can be easily ported
to MYSQL as well.

It would be great to see this in CVS.

Respectfully

Constantine



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

I am attaching the result of "cvs diff -u asterisk/res/res_musiconhold.c" and the entire new source
asterisk/res/res_musiconhold.c.

Comments:By: Brian West (bkw918) 2004-09-01 11:25:52

I'm sorry but this WOULD NOT be great to see in CVS.  You might want to consider using ODBC and not limiting someone to just using PGSQL.  I personally feel all the MYSQL and PGSQL code needs to be ripped out and totally redone using an ast_sql API that we need to build for all this.    Then you have the ast_sql* calls that then use registered DB modules similar to how AST_DATA does it.... but not such a big mess.

bkw
By: constfilin (constfilin) 2004-09-01 19:10:26

I agree that asterisk will benefit tremendously from an SQL API that all code
will use to talk to a database. For this purpose I propose the API attached
as "astsql.h" file and would appreciate your comments on it. Shall I post this
to asterisk developement list?
By: Mark Spencer (markster) 2004-09-01 23:54:47

How does this patch differ from using res_odbc_config?
By: Brian West (bkw918) 2004-09-02 00:24:35

It looks like hes loading the song list and file paths from SQL.

edited on: 09-02-04 00:24
By: Mark Spencer (markster) 2004-09-02 00:52:24

Couldn't this also be done using the existing config odbc stuff which would allow the musiconhold.conf to be read from the database?  

Songs are generally done by a directory which could contain symbolic links to songs in other directories.  Am I missing something?

Tigher SQL integration will probably have to happen post 1.0, although whether this patch makes it into asterisk or not, it may be useful to keep it in the bug tracker.
By: constfilin (constfilin) 2004-09-02 02:16:11

Thanks for the attention to the patch. As far as I understand,
res_odbc_config allows reading configuration from ODBC *ONE-TIME*
at asterisk (re)start. My patch re-reads the configuration sitting
in the database *IN RUN TIME* and no
database change goes unnoticed.

I am willing to implement the SQL interface outlined in astsql.h if you
approve it. Support of Postgres and MYSQL will take a day or two.
Support of MSSQL might take longer, but it will be done because of
requirements of the project I am working on. In the result there will be
3 databases supported by generic SQL api for asterisk, which is very
good. Once this is done, I can re-write my patches to the generic SQL api.

There was a discussion on dev list about using ODBC for the generic API but
the common opinion is that ODBC is slow.

Overall, I am working on a project were asterisk is supposed to see all
configuration changes *IMMEDIATELY* and run without restarts or reloads.
So far I supported real time (aka dynamic) confgiration for this:
  channels/chan_sip.c
  apps/app_voicemail.c
  res/res_musiconhold.c
I am also working on real time configuration of call queues and agents.
Effectively I am already working on asterisk post 1.0 and it would be
great if you agree on the APIs and features so that asterisk post 1.0
is out ASAP.

Respectfully

Constantine

edited on: 09-02-04 02:27
By: Brian West (bkw918) 2004-09-02 09:05:39

Just an FYI ODBC is not slow.  Its a common misconception... Sure its a layer that does take a hit in speed but unless your doing 6000+ queries persecond you'll never notice it.  Its quite fast.

bkw
By: Brian West (bkw918) 2004-09-02 09:07:45

Also res_musiconhold.c isn't reload enabled but Tony and I have a patch that will let you use any files that asterisk can play as MOH without the need for mpg123..... i'll see if we can get it up for someone to look at later today.  Also it adds the reload stuff
By: Mark Spencer (markster) 2004-09-02 09:55:42

Your GTK-like object reference counting is a great idea.  I think it could apply to all sorts of Asterisk data structures post 1.0, especially sip/iax user/peer's and in combination with an appropriate hash structure, but I wouldn't want to put it exclusively in res_musiconhold right now since I think it has a much broader, more centralizable value.

I think this bug should probably be post 1.0, but is there any chance you might be able to just make res_musiconhold supported through "reload" in the short term (that's a different bug in the bug tracker) and then we can address these more sophisticated data structures after 1.0?
By: Rob Gagnon (rgagnon) 2004-09-03 11:44:39

ast_data... mess?  what?
By: Brian West (bkw918) 2004-09-03 13:45:38

ast_data in my opinion could be better.. thats all i'm saying! :P
By: Rob Gagnon (rgagnon) 2004-09-03 15:02:41

thats like saying "asterisk" could be better....

lets move the conversation to the correct bug, and make some constructive comments.  I cannot read minds.
By: Mark Spencer (markster) 2004-09-07 23:24:00

Reminder sent to constfilin

Are you coming to Astricon?  We're having a developer meeting there and I would very much like to have your input and ideas as we look past 1.0
By: constfilin (constfilin) 2004-09-08 04:23:04

I re-wrote res_musiconhold.c to talk to databases using generic astodbc API
implemented and submitted in http://bugs.digium.com/bug_view_page.php?bug_id=0002398

With this change res_musiconhold.c becomes 100% independent of any proprietary
database API.

I am attaching new result of "cvs diff -u asterisk/res/res_musiconhold.c" and
the new "asterisk/res/res_musiconhold.c"

Respectfully

Constantine

edited on: 09-08-04 04:25
By: Brian West (bkw918) 2004-10-17 12:31:03

Can we update this to use realtime?  If thats not a good idea then let me know.  If you know postgres maybe you could do the res_config_pgsql.c?

bkw