[Home]

Summary:ASTERISK-11794: Changes in #0012115 break Voicemail multi-language capability
Reporter:kuj (kuj)Labels:
Date Opened:2008-04-07 17:08:09Date Closed:2008-04-10 12:22:41
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 12379-absolutepath.diff
Description:Voicemail will not play user-recorded alternate-language greetings any longer with the changes introduced to function fileexists_core (file.c) in the fixes for ASTERISK-1204115. Fileexists_core now sports a check for absolute pathname upfront and will return those absolute pathnames without adding a language specific (sub-) directory. Consequently, VM greetings (greet, busy and unavail) cannot switch from the default language to a secondary language.

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

I've been able to switch to different-language VM greetings based on callerid or simply looking which line/peer a call originated. Setting the channel language variable has worked in the past to switch to alternate language for both user-recorded as well as system prompts. User-recorded alternate-language greetings for VM stopped working with the referenced changes, as play_greeting() (in app_voicemail) calls ast_streamfile with an absolute filename (/var/spool/asterisk/voicemail/...). With ast_streamfile not considering language-specific directories any more because of the absolute pathname test, the result is a mixed-language presentation to the caller:
- the default language recorded greeting (or buy or unavail) message is played
- app_voicemail will then ast_streamfile "vm-intro" by requesting a non-absolute filename "vm-intro" and language setting, resulting in alternate language playback.

This undesirable behavior is fixed by removing the 4-line test in fileexists_core for absolute paths, but I'm certain that will break things in the original bug ASTERISK-1204115.
Comments:By: kuj (kuj) 2008-04-07 19:24:43

It may not be obvious from the above, but I am not using languageprefix=yes in my config. Thus, I never ran into the MoH file issues as referenced in ASTERISK-1204115.
By: kuj (kuj) 2008-04-08 13:44:48

Looking at the code again and playing with the languageprefix=yes option, I do not think that the test for an absolute pathname that was added to fileexists_core() is essential to fix the issues in ASTERISK-1204115. Rather, it looks like the intent of the absolute path test was to optimize the function to not "waste cycles" when the combination of absolute path and languageprefix=yes resulted in a new path that wasn't "well defined".

With languageprefix=yes, an absolute path and the test for absolute path removed from fileexists_core(), the function will still find the valid filenames. It just wastes some cycles on also looking for "not well defined" files. I.e. for a MoH music title, it looks for

  de/var/lib/asterisk/moh/title
  /var/lib/asterisk/moh/title

and will successfully play the latter. I call the first filename "not well defined" as we don't know which base directory it is relative to. So the real issue is the implementation of languageprefix=yes in conjunction with absolute paths. My suggestion would be to

- re-enable language specific content by removing the absolute path test from fileexists_core(), and
- modify fileexists_test() so it tests for an absolute language-specific filename/path, but only when the filename argument is absolute to begin with.

In other words, with my MoH example from above, modify fileexists_test so it checks "/de/var/lib/asterisk/moh/title" instead of "de/var/lib/.....". That would for the first time create "defined behavior" for prefix mode and absolute paths. fileexists_test would look somewhat like:

       if (ast_language_is_prefix) { /* new layout */
               if (lang) {
                  if (is_absolute_path(filename)){
                       snprintf(buf, buflen, "/%s/%s", lang, filename);
                  } else {
                       snprintf(buf, buflen, "%s/%s", lang, filename);
                  }
               } else {
                       snprintf(buf, buflen, "%s", filename);
               }
       } else { /* old layout */ ...
By: Jason Parker (jparker) 2008-04-08 14:25:46

I think this patch follows the original intent a little better.

If the filename is an absolute path, ignore the ast_language_is_prefix setting.

/var/my/blah > /var/my/en/blah, /var/my/blah
my/blah (with lang prefix) > en/my/blah, my/blah
my/blah (without lang prefix) > my/en/blah, my/blah
By: kuj (kuj) 2008-04-08 15:34:51

qwell, that makes sense. As I didn't follow the discussions around the prefix settings, I was not sure what the desired behavior was. Your patch will fix all the broken features, plus fall back to prior behavior (lang-specific dir at the end of the path, right before the filename) even when absolute paths are given and prefix=yes. Good to go from my side. Thanks!
By: Digium Subversion (svnbot) 2008-04-10 12:21:10

Repository: asterisk
Revision: 114035

U   branches/1.4/main/file.c

------------------------------------------------------------------------
r114035 | qwell | 2008-04-10 12:21:09 -0500 (Thu, 10 Apr 2008) | 10 lines

Only try to prefix language if we are not using an absolute path (suffix it otherwise).

en/var/lib/asterisk/sounds/blah.gsm is a very silly path.

(closes issue ASTERISK-11794)
Reported by: kuj
Patches:
     12379-absolutepath.diff uploaded by qwell (license 4)
Tested by: kuj, qwell

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

http://svn.digium.com/view/asterisk?view=rev&revision=114035
By: Digium Subversion (svnbot) 2008-04-10 12:22:19

Repository: asterisk
Revision: 114036

_U  trunk/
U   trunk/main/file.c

------------------------------------------------------------------------
r114036 | qwell | 2008-04-10 12:22:19 -0500 (Thu, 10 Apr 2008) | 18 lines

Merged revisions 114035 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r114035 | qwell | 2008-04-10 12:26:10 -0500 (Thu, 10 Apr 2008) | 10 lines

Only try to prefix language if we are not using an absolute path (suffix it otherwise).

en/var/lib/asterisk/sounds/blah.gsm is a very silly path.

(closes issue ASTERISK-11794)
Reported by: kuj
Patches:
     12379-absolutepath.diff uploaded by qwell (license 4)
Tested by: kuj, qwell

........

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

http://svn.digium.com/view/asterisk?view=rev&revision=114036
By: Digium Subversion (svnbot) 2008-04-10 12:22:41

Repository: asterisk
Revision: 114037

_U  branches/1.6.0/
U   branches/1.6.0/main/file.c

------------------------------------------------------------------------
r114037 | qwell | 2008-04-10 12:22:41 -0500 (Thu, 10 Apr 2008) | 26 lines

Merged revisions 114036 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
r114036 | qwell | 2008-04-10 12:27:16 -0500 (Thu, 10 Apr 2008) | 18 lines

Merged revisions 114035 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r114035 | qwell | 2008-04-10 12:26:10 -0500 (Thu, 10 Apr 2008) | 10 lines

Only try to prefix language if we are not using an absolute path (suffix it otherwise).

en/var/lib/asterisk/sounds/blah.gsm is a very silly path.

(closes issue ASTERISK-11794)
Reported by: kuj
Patches:
     12379-absolutepath.diff uploaded by qwell (license 4)
Tested by: kuj, qwell

........

................

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

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