[Asterisk-code-review] app_voicemail.c: Support multiple file formats for forwarded messages. (...asterisk[13])

[George Joseph]

George Joseph has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/12981 )

Change subject: app_voicemail.c: Support multiple file formats for forwarded messages.
......................................................................


Patch Set 1: Code-Review-1

(4 comments)

Should we not fix the ODBC bug instead?

https://gerrit.asterisk.org/#/c/12981/1/apps/app_voicemail.c 
File apps/app_voicemail.c:

https://gerrit.asterisk.org/#/c/12981/1/apps/app_voicemail.c@5511 
PS1, Line 5511: fopen
Why not use access() instead of trying to open the file?


https://gerrit.asterisk.org/#/c/12981/1/apps/app_voicemail.c@5514 
PS1, Line 5514: 		if (c)
              : 			*c = '\0';
Always use braces with an "if".


https://gerrit.asterisk.org/#/c/12981/1/apps/app_voicemail.c@5518 
PS1, Line 5518: fopen
Same.  Why not use access()?


https://gerrit.asterisk.org/#/c/12981/1/apps/app_voicemail.c@5558 
PS1, Line 5558: 			} else {
              : 				if (!strcasecmp(format, "wav")) {
              : 					if (vmu->volgain < -.001 || vmu->volgain > .001) {
              : 						res = snprintf(sox_gain_cmd, sizeof(sox_gain_cmd), "sox -v %.4f %s.%s -e signed-integer -b 16 %s",
              : 									   vmu->volgain, attach, altformat, fname);
              : 					} else {
              : 						res = snprintf(sox_gain_cmd, sizeof(sox_gain_cmd), "sox %s.%s -e signed-integer -b 16 %s",
              : 									   attach, altformat, fname);
              : 					}
              : 				} else {
              : 					if (vmu->volgain < -.001 || vmu->volgain > .001) {
              : 						res = snprintf(sox_gain_cmd, sizeof(sox_gain_cmd), "sox -v %.4f %s.%s %s",
              : 									   vmu->volgain, attach, altformat, fname);
              : 					} else {
              : 						res = snprintf(sox_gain_cmd, sizeof(sox_gain_cmd), "sox %s.%s %s",
              : 									   attach, altformat, fname);
              : 	
Can you explain why we need this logic?  What was it about the original sox command that no longer works?



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/12981
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: I7321e7f7e7c58adbf41dd4fd7191c887b9b2eafd
Gerrit-Change-Number: 12981
Gerrit-PatchSet: 1
Gerrit-Owner: cmaj <chris at penguinpbx.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Comment-Date: Tue, 08 Oct 2019 11:58:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20191008/6efb9ae2/attachment.html>