[Home]

Summary:ASTERISK-08276: [patch] Voicemail does not playback via ODBC voicemail storage with Postgresql database
Reporter:Leif Madsen (lmadsen)Labels:
Date Opened:2006-12-04 20:45:36.000-0600Date Closed:2007-01-16 15:34:12.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Applications/app_voicemail
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 10Jan2006_8513_notices_1.2.patch
( 1) 10Jan2007_8513.patch
( 2) 20061220__bug8513.diff.txt
( 3) 22Dec2006_8513.patch
( 4) bt.txt
( 5) connectiontest.c
( 6) connectiontest2.c
( 7) connectiontest3.c
( 8) connectiontest4.c
( 9) detailed_instructions_v1.txt
(10) tb_voicemail_messages.txt
(11) try_voicemail.diff.txt
(12) voicemail_storage_read__failure.txt
Description:Trying to read the data back from a Postgresql database with voicemail ODBC storage gives the error located in the dump attached to this bug. No errors when writing to the database.
Comments:By: Serge Vecher (serge-v) 2006-12-05 08:20:13.000-0600

does the commit in 8385 impact this in any way?
By: Leif Madsen (lmadsen) 2006-12-05 11:29:03.000-0600

Serge-v, unfortunately not. I don't see the mmap() error, and did test the 8385 commit once I saw it in the svn-commit messages, but still can't read from the DB.
By: Tilghman Lesher (tilghman) 2006-12-07 19:34:58.000-0600

This is an issue with the Postgresql ODBC driver.  It will need to be reported upstream that we are unable to use ODBC to store blob fields.
By: Tilghman Lesher (tilghman) 2006-12-08 07:59:07.000-0600

Reopening for caio1982
By: Caio Begotti (caio1982) 2006-12-08 08:02:23.000-0600

Thanks, Corydon :-) you wrote that "this is an issue with the Postgresql ODBC driver. It will need to be reported upstream that we are unable to use ODBC to store blob fields" but I've tried the 1.2 patch and it worked fine. Using exactly the same version of PGSQL and the ODBC driver.

Only the code of app_voicemail.c changed and it worked. Well... is there some workaround for it since you guys think that's not fixable even so? I don't think the maintainers of the ODBC connector will release a patch or a new version because that just in time before the new Debian Etch release, which worry me a lot, especially because it worked with 1.2 as stated.
By: Leif Madsen (lmadsen) 2006-12-11 11:02:32.000-0600

caio1982: you got this working on PGSQL (read/write) on 1.2? If so, did you use the byeta field type to store the data?
By: Caio Begotti (caio1982) 2006-12-11 11:13:24.000-0600

Nope, blizrage. I just changed my Asterisk installation to 1.2 with the patch provided in ASTERISK-8154, no more changes. My PGSQL structure remained the same and the versions of the installed packages ditto. As far as I know, the actual field type in use is 'oid' for large objects.

Do you want the output of some psql command to check anything else?
By: Caio Begotti (caio1982) 2006-12-11 12:08:21.000-0600

Just uploaded my table structure to this ticket (exported from phpPgAdmin, structure only).


By: Leif Madsen (lmadsen) 2006-12-11 16:31:29.000-0600

caio1982: you actually got this working? I'm not able to get the audio back out from postgresql in either 1.2 or 1.4... so not entirely sure what you're doing different than me. I just have the voicemessages table, pointing at that from voicemail.conf, etc...

If I run \dl from psql, then I see the objects, but no idea what you're doing which causes you to be able to get the audio back out.
By: Caio Begotti (caio1982) 2006-12-11 16:34:26.000-0600

Actually I was able to playback the audio once when I tried that patch, not today (I'm not using 1.2 anymore). I'll try to save some time at work to test 1.2 again in my environment and post the results here tomorrow, maybe.
By: Leif Madsen (lmadsen) 2006-12-11 16:36:22.000-0600

Which patch though? I downloaded 1.2 from SVN where the patch should already be applied, and see the data go into the DB, but still getting the same WARNINGs as in 1.4 when reading the data back out.
By: Leif Madsen (lmadsen) 2006-12-11 19:46:15.000-0600

OK, so it seems the msg0000.WAV file (or whatever filename) is just pulling the OID of the BLOB, and not the actual blob... which is why the read of the file fails, so we need to figure out how to get the actual BLOB data out of the DB, and verify that there really is BLOB info in there.

Anyone know how to check that?

Update: OK, verified the data really is in the database by pulling it out with the lo_export function and then using Playback() app to listen to the file. So the data is getting put in correctly, we're just not pulling it back out (just the OID).


By: Caio Begotti (caio1982) 2006-12-12 05:48:25.000-0600

Corydon, I think it's better to assume that the patch never really worked for me. I really can't reproduce again the whole environment to check the steps necessary to analyse this. It's a shame, I know... although yet I don't believe it worked just reading the .wav from the filesystem not from the database, as stated by blitzrage :-)

Both the current maitainer of the Debian package of odbc-postgresql and the real ODBC authors were contacted by e-mail at this morning. I'm now waiting for some guiding or any kind of hint.
By: Caio Begotti (caio1982) 2006-12-13 05:36:39.000-0600

For more information, related to this issue, please see:
http://archives.postgresql.org/pgsql-odbc/2006-12/threads.phpASTERISK-36040
By: Denis Galvao (denisgalvao) 2006-12-13 10:23:59.000-0600

I got the same results when testing trunk.

I tried it on 1.2 and didn't get the same problem.

This is a trunk related problem.
By: Serge Vecher (serge-v) 2006-12-13 11:41:45.000-0600

did you try this patch (20061211__bug8513.diff.txt) ?
By: Jared Smith (jsmith) 2006-12-13 23:00:52.000-0600

Here's how I've created my PostgreSQL table for testing... not sure if it's right or not, but it works:

CREATE FUNCTION loin (cstring) RETURNS lo AS 'oidin' LANGUAGE internal IMMUTABLE STRICT;
CREATE FUNCTION loout (lo) RETURNS cstring AS 'oidout' LANGUAGE internal IMMUTABLE STRICT;
CREATE FUNCTION lorecv (internal) RETURNS lo AS 'oidrecv' LANGUAGE internal IMMUTABLE STRICT;
CREATE FUNCTION losend (lo) RETURNS bytea AS 'oidrecv' LANGUAGE internal IMMUTABLE STRICT;

CREATE TYPE lo ( INPUT = loin, OUTPUT = loout, RECEIVE = lorecv, SEND = losend, INTERNALLENGTH = 4, PASSEDBYVALUE );
CREATE CAST (lo AS oid) WITHOUT FUNCTION AS IMPLICIT;
CREATE CAST (oid AS lo) WITHOUT FUNCTION AS IMPLICIT;

CREATE TABLE lo_test(
id serial,
data lo);

INSERT INTO lo_test VALUES (DEFAULT, lo_import('/var/lib/asterisk/sounds/tt-weasels.gsm'));

You can then run the little connection test script I've attached, and see that it copies the tt-weasels.gsm file that existed to /tmp/george.gsm.

To compile connectiontest.c, run:
# gcc -lodbc -o connectiontest connectiontest.c
By: Jared Smith (jsmith) 2006-12-13 23:03:33.000-0600

I forgot to note that there's a #define near the top of connectiontest.c.  Set it to 0 to get the expected result, or 1 to watch it fail... the code should be pretty self-explanatory.
By: Jared Smith (jsmith) 2006-12-13 23:48:45.000-0600

Sorry Corydon... that latest patch just causes segfaults.  I've attached a backtrace after compiling Asterisk with the dont-optimize flag.  I should be in IRC tomorrow if you'd like to try some other things.
By: Tilghman Lesher (tilghman) 2006-12-14 12:07:04.000-0600

Okay, what the latest results show is that there is a buffer overflow error in the PGSQL ODBC driver.  We feed them a buffer, tell them the maximum size of that buffer and the driver is writing beyond the end of that buffer.
By: Hiroshi Inoue (hinoue) 2006-12-15 01:51:42.000-0600

Hello,
I'm a developer of psqlodbc driver.

Where can I find the current source of app_voicemail.c ?
By: Caio Begotti (caio1982) 2006-12-15 06:10:23.000-0600

Hiroshi, here you check the latest trunk code:
http://svn.digium.com/view/asterisk/trunk/apps/app_voicemail.c?view=markup

But I believe this issue is focusing in the 1.2 branch, to later merge the solution in trunk:
http://svn.digium.com/view/asterisk/branches/1.2/apps/app_voicemail.c?view=markup

(there are relavant differences between them but maybe the ODBC part is almost equal, you'll have to check this with Corydon)


By: Hiroshi Inoue (hinoue) 2006-12-15 06:59:53.000-0600

SQLGetData() doesn't add NULL terminater at the end of binary data.
Please change CHUNKSIZE + 1 to CHUNKSIZE.

BTW the 3rd parameter of SQLGetData is SQL_C_BINARY not SQL_BINARY
though the values are the same.
By: Jared Smith (jsmith) 2006-12-15 08:36:37.000-0600

hinoue,

I think it may be easier to show the problem in the "connectiontest2.c" code attached to this bug, instead of the Asterisk source code itself.  In that case, we're clearly getting a BUS error when trying to use SQLGetData to write to an mmap'd file.  You can setup your table as explained here in the bug report (the lo_test table).

-Jared
By: Anthony LaMantia (alamantia) 2006-12-15 20:02:02.000-0600

hinoue,
have you tired using the connection2.c code that was posted to this bug?
it seems to have the modifacitions made that resolve the CHUNKSIZE+1 issue,
as SQLGetData just passes 1024. rathern then CHUNKSIZE+1 (where chunksize #define CHUNKSIZE 1024)
By: Hiroshi Inoue (hinoue) 2006-12-15 21:56:35.000-0600

I tried connection2.c and found it crashed.
Hmm it seems because the underlying file is size 0.
I changed to write 1025 bytes area of the file first
and the result is successful.
By: Anthony LaMantia (alamantia) 2006-12-16 02:33:14.000-0600

Can you show us the changes that you have made?
By: Hiroshi Inoue (hinoue) 2006-12-16 05:01:48.000-0600

Lseek and write like app_voicemail.c does.
Please note that my environment is Windows(cygwin).
Antway the patch is bellow.

--- connectiontest2.c.orig 2006-12-16 19:39:55.871184500 +0900
+++ connectiontest2.c 2006-12-16 19:41:50.026224500 +0900
@@ -142,6 +142,8 @@
        printf("We failed to open the file for writing!\n");
return(-2);
      }
+      lseek(fd, 1023, SEEK_SET);
+      write(fd, "", 1);
      res = SQLNumResultCols(stmt, &columns);
      printf("columns = %d\n",columns);


By: Tilghman Lesher (tilghman) 2006-12-16 08:56:06.000-0600

hinoue:  in your modified connectiontest2.c, could you verify whether you obtain the column desired if DEBUG is defined (near the top of the file)?
By: Hiroshi Inoue (hinoue) 2006-12-16 15:11:53.000-0600

Hi Corydon,
The Windows driver manager seems to hate NULL pointer input and returns
SQL_ERROR. Though I'm not sure about the driver manager on Debian, please
try to replace the NULL pointer by a non-null pointer.

regards, Hiroshi Inoue
By: Jared Smith (jsmith) 2006-12-18 14:36:08.000-0600

I've uploaded connectiontest3.c, which added the two lines suggested by hinoue (the lseek and write calls), and it appears to be working correctly with DEBUG set to 0.

With DEBUG set to 1, the program runs (and doesn't get a SIGBUS), but writes out a  file filled with 0x00 instead of the correct data.  

It appears that the PostgreSQL driver doesn't support call SQLGetData with a null buffer and zero length just to get the size of the object.  But by adding the call to lseek and write, we seem to be able to get the data out of PostgreSQL without a SIGBUS error.
By: Hiroshi Inoue (hinoue) 2006-12-18 15:36:30.000-0600

As for DEBUG, how about the following patch ?

--- connectiontest3,c,orig 2006-12-19 06:09:24.067376000 +0900
+++ connectiontest3.c 2006-12-19 06:21:18.013980800 +0900
@@ -174,7 +174,7 @@
if (!strcasecmp(coltitle, "data")) {
#if DEBUG
/* Pass in a NULL buffer, so we can get the size of what we'd like to pull out */
- res = SQLGetData(stmt, x, SQL_C_BINARY, NULL, 0, &colsize2);
+ res = SQLGetData(stmt, x, SQL_C_BINARY, (void *) 1, 0, &colsize2);
printf("The object is %d bytes!\n", colsize2);
#endif
sigaction(SIGBUS, &sigbus, NULL);
By: Jared Smith (jsmith) 2006-12-19 09:24:24.000-0600

I can confirm that changing the NULL to a "(void *) 1" does in fact work as well.  I'm uploading connectiontest4.c.  I've also double-checked that the binary that comes out of connectiontest4 is exactly the same as the binary I put in the database... it seems we've finally got this bug licked!

Thanks hinoue for all your help!
By: Caio Begotti (caio1982) 2006-12-19 10:03:13.000-0600

Incredible good news, guys. I'm waiting patiently for a patch or SVN update to test the new ODBC connection in the voicemail app!
By: Tilghman Lesher (tilghman) 2006-12-19 10:47:57.000-0600

Okay, new prospective patch uploaded.
By: Hiroshi Inoue (hinoue) 2006-12-19 19:36:41.000-0600

Hmm does the patch work ?
Though the NULL parameter was replaced by fdm, isn't fdm initialized to NULL ?
By: Leif Madsen (lmadsen) 2006-12-19 21:25:22.000-0600

jsmith did not have luck with using the patch as reported on IRC
By: Hiroshi Inoue (hinoue) 2006-12-19 23:38:17.000-0600

I uploaded a trial patch though I can't even compile it by mysef.
Could anyone try the patch ?
By: Caio Begotti (caio1982) 2006-12-20 10:57:42.000-0600

I could not get it working with new Corydon's patch as well.

Sorry Hiroshi, your patch seems to be lacking something... I don't know but, I could not even store the voicemail using ODBC. I tried to patch the code with Corydon's file then edited it manually to fit your changes but it did not compile at all.
By: Hiroshi Inoue (hinoue) 2006-12-21 18:37:47.000-0600

Doesn't Corydon's new patch(20061220__bug8513.diff.txt) work ?
By: Jared Smith (jsmith) 2006-12-22 11:37:39.000-0600

I've uploaded a new patch (22Dec2006_8513.patch), which mostly works for me.  It works fine, as long as:

o  The format is gsm, and not WAV.  For some reason, Asterisk crashes whenever it plays back the WAV file after it was pulled from the database.  I think this is a bug somewhere else in Asterisk.  For now, just set "format=gsm" in the [general] section of voicemail.conf.

o  The voicemail is smaller than CHUNKSIZE (65536 bytes, as defined near the top of app_voicemail.c).  The chunking code isn't working correctly.  I'll continue to work on this code and see if I can't get the chunking working correctly.
By: Caio Begotti (caio1982) 2006-12-28 09:32:21.000-0600

Jsmith, could you please write down what steps are necessary for us to reproduce this using GSM and get it working partially? So that the WAV problem can be tracked in another ticket and you can consider this one solved after us checking it in 1.4, maybe?

I remember when I tested some procedures you told me on IRC but it didn't work in my environment (heavily modified at that time, although it worked on yours).

If I recall correctly, you asked me to upgrade to PGSQL 8.1 (due the ODBC connector' version), to run some SQL commands to reconfigure my lo type and add new triggers to update and delete the objects and... well, I cannot remember anything more :-)

Thanks one more time for your attention on IRC that night!
By: Jared Smith (jsmith) 2007-01-02 21:41:06.000-0600

I've finally taken some time to write up some *very* detailed instructions on how I've been able to get this working.  I'd really appreciate it if people could test this out on a variety of systems and give me some feedback.

Once we've all got it working up to this point, we'll tackle the problem of recording in other formats besides gsm.
By: Jared Smith (jsmith) 2007-01-04 09:11:03.000-0600

One other quick note -- there was a bug fix that went in a short time ago that may have been causing some of you some grief with ODBC voicemail.  Please update to the latest revision of the 1.2 branch before trying the latest patch, so that we all know we're on the same page.

Also, I don't know why the chunking code was causing me grief before -- it's working fine for me now.  One less thing to have to worry about.
By: Leif Madsen (lmadsen) 2007-01-04 19:24:24.000-0600

Unfortunately I get segfaults on my Dell 1750 with the patch applied. Any else have a change to try this out?

Segfault (backtrace)
http://pastebin.ca/305722
By: Andrew Kohlsmith (akohlsmith) 2007-01-10 15:23:51.000-0600

modified patch (I've uploaded one that works with svn trunk) works for me, although I have a *slew* of other issues with trunk, since trunk seems to save the full path to the voicemailbox to the DB, and the char sql[MAX_PATH] variable isn't long enough.
By: Jared Smith (jsmith) 2007-01-10 15:34:39.000-0600

I agree that saving the entire path in the database is silly, but that probably ought to be addressed as a seperate bug, since it has backward-compatibility implications.  (Of course, we could look to see if the path started with a /, in which case we could handle it the old way).

Can anyone else try this with using the 22Dec patch for 1.2, but making sql bigger than MAX_PATH as akohlsmith described?
By: Andrew Kohlsmith (akohlsmith) 2007-01-10 15:42:55.000-0600

agreed on the new functionality; save that for another bug.

jsmith, remember that I had *no* issues with VM retrieval with the 22Dec patch (ported to svn trunk, which is all the 10Jan patch is).

building out sql[MAX_PATH] to sql[1000] in two places (retrieve_file() and last_message_index()) fixed my problems with storing voicemail.  

Mind you -- those functions are used for retrieval too.  Good idea.  :-)
By: Jared Smith (jsmith) 2007-01-10 18:47:34.000-0600

I've uploaded my latest patch, which adds some NOTICE messages to the CLI, but seems to work better than the last patch on x86 hardware.  Please test and let me know the results.
By: Leif Madsen (lmadsen) 2007-01-10 19:30:13.000-0600

It seems my Asterisk no longer segfaults with this (patch applied to latest 1.2 branch). However, the msg0000.gsm file is returning the large object ID, and not the actual large object. I've run the lo_export function and there really does exist audio there.
By: Leif Madsen (lmadsen) 2007-01-11 09:14:46.000-0600

Just a little tip from me to you... when testing this, don't use your old DB table with the 'bytea' type because it really screws with you.

After dropping my voicemessages table and recreating it with the one in Mr. Smith's excellent documentation, I got this WORKING!!!!!!!!!!!!!!!!

Bug squashed?  We'll find out. Will be doing some more testing today.
By: Andrew Kohlsmith (akohlsmith) 2007-01-11 11:02:31.000-0600

Ok just some documentation in this post:

The 10Jan patch, which is just the 22Dec patch for svn trunk, works just fine.  Blitzrage confirmed that the 22Dec patch works for 1.2.

I have, however, discovered what I believe is a stack overflow bug.  I was getting segfaults in the lowlevel odbc-pgsql library which appear to be caused by plain old running out of stack space.  Changing the char sql[PATH_MAX] to [1000] "fixed" it, in the sense that everything seems to work just fine now, but that just means that there could be other stack issues.  The fact that blitzrage does not see these issues and I do, and I'm not running on modest hardware either, suggests that there are some nasty little issues lurking in the res_odbc codebase in general.

Honestly though -- why are we allocating **MULTIPLE FOUR THOUSAND BYTE ARRAYS** on the stack?  Surely that is NOT what people are thinking when they see PATH_MAX...
By: Denis Galvao (denisgalvao) 2007-01-16 07:51:41.000-0600

Hi all.

I tried the 10Jan patch in 1.4.0 with the detailed_instructions_v1.txt doc.

The problem still there. I got the envelope but when VoiceMailMain will play the message, the call is dropped.

Is there anything else that should be done?
By: Andrew Kohlsmith (akohlsmith) 2007-01-16 07:57:43.000-0600

I do *not* have any issues with 10Jan and svn trunk.  Can you dig in a little and see WHERE the issue is?
By: Leif Madsen (lmadsen) 2007-01-16 09:12:45.000-0600

It seems strange to me that you are still getting the errors after both tzanger and myself have had success with it. Can you give us some information as to what you have setup in your voicemail.conf, the PostgreSQL triggers, functions, and tables you created, and the descriptions of the tables? That output you're seeing might also be useful.

The more information you can attach (in a text file please), the easier it will be to determine where your issue lies. There are a lot of steps there, and I missed something the first couple of times I went through the instructions, so I imagine it is something you missed.

/Leif.
By: Caio Begotti (caio1982) 2007-01-16 09:25:23.000-0600

Seems to me that he tested it with the 1.4.0 release, not trunk. Anyway, I'm gonna test it as well using trunk today and will try to increase the statistic of success.
By: Leif Madsen (lmadsen) 2007-01-16 09:56:04.000-0600

Yah, akohlsmith was testing with trunk as well. I was testing on 1.2-branch-r50150.

Of interesting note, it seems ALL codec formats work, and not just GSM. jsmith had said other formats wouldn't work at this time, but it appears they do for akohlsmith and I.
By: Caio Begotti (caio1982) 2007-01-16 11:48:38.000-0600

The patch '10Jan2007' got applied to trunk (checked out minutes ago) with the following warnings:

Hunk #1 succeeded at 1074 (offset -43 lines).
Hunk #2 succeeded at 1086 (offset -43 lines).

I tried to use wav, wav49 and gsm formats (since people got it working with other formats besides GSM). Both wav and wav49 crashed Asterisk as everyone from this ticket knows. Using GSM it didn't crashed but the audio was played back as empty:

   -- Playing 'vm-from-phonenumber' (language 'pt_BR')
   -- Playing 'digits/5' (language 'pt_BR')
   -- Playing 'digits/0' (language 'pt_BR')
   -- Playing 'digits/5' (language 'pt_BR')
   -- Playing 'digits/0' (language 'pt_BR')
   -- Playing '/var/spool/asterisk/voicemail/default/5050/INBOX/msg0000' (language 'pt_BR')
   -- Playing 'vm-advopts' (language 'pt_BR')
   -- Playing 'vm-repeat' (language 'pt_BR')

Asterisk tries to playback the audio file and I cannot hear anything (yes, I got sound from everything else). I then exported the audio data from the large objects table and it's there. Reproduced loud and fine using the 'play' command.

Maybe those hunks are the problem with the current trunk?
I checked it and seemed okay for me. I'm not sure about it, so I'll try 1.2.

PS: does it matter if my Asterisk binary has all modules embedded? (it always had)
By: Caio Begotti (caio1982) 2007-01-16 12:29:20.000-0600

Same issue with branch 1.2: patch applied (no hunks), voicemessage stored but when playing it back it just pass the audio like it's empty. Maybe it's something related to my environment, I'll double check it, sorry. Anyway, the 1.4 patch has those hunks right now.
By: Leif Madsen (lmadsen) 2007-01-16 12:55:09.000-0600

Hrmmm... caio1982 that is very strange as it is working on my environment here, but if we can figure out where the issue lies on your side, then maybe there is another bug or situation that needs to be addressed in the code?
By: Caio Begotti (caio1982) 2007-01-16 13:13:22.000-0600

Blitzrage, right now I'm testing my own environment from scratch.
I believe this is probably a local issue, I am sorry.
By: Leif Madsen (lmadsen) 2007-01-16 14:18:17.000-0600

caio1982: hey no problem -- it's good to find out in as many situations as it works or does not work. If we can figure out where it does not work, and why not, then we just end up making the solution more robust. Good luck in your testing!
By: Tilghman Lesher (tilghman) 2007-01-16 15:34:12.000-0600

Fixed in 1.2 in revision 51158, merged to 1.4 in 51159, merged to trunk in 51160.