[Asterisk-code-review] app mp3: remove 10 seconds of silence after mp3 playback (asterisk[master])

[Richard Mudgett]

Richard Mudgett has posted comments on this change. ( https://gerrit.asterisk.org/9179 )

Change subject: app_mp3: remove 10 seconds of silence after mp3 playback
......................................................................


Patch Set 1: Code-Review-1

(5 comments)

https://gerrit.asterisk.org/#/c/9179/1//COMMIT_MSG
Commit Message:

https://gerrit.asterisk.org/#/c/9179/1//COMMIT_MSG@14
PS1, Line 14: checks if mpg123 is still running before poll again.
            : 
            : Change-Id: Ib7df8462e3e380cb328011890ad9270d9e9b4620
You are missing the ASTERISK issue identifier here in the commit message.
https://wiki.asterisk.org/wiki/display/AST/Commit+Messages


https://gerrit.asterisk.org/#/c/9179/1/apps/app_mp3.c
File apps/app_mp3.c:

https://gerrit.asterisk.org/#/c/9179/1/apps/app_mp3.c@147
PS1, Line 147: 	for(i=0; i<timeout; i++) {
Guidelines: Spaces around binary operators
for (i = 0; i < timeout; ++i) {
}


https://gerrit.asterisk.org/#/c/9179/1/apps/app_mp3.c@152
PS1, Line 152: 			// is mpg123 still running?
Guidelines: No C++ style comments.


https://gerrit.asterisk.org/#/c/9179/1/apps/app_mp3.c@164
PS1, Line 164: 		ast_log(LOG_NOTICE, "Poll timed out/errored out with %d\n", res);
This message is no longer right.  Should just be:
"Poll timed out.\n"


https://gerrit.asterisk.org/#/c/9179/1/apps/app_mp3.c@240
PS1, Line 240: 				res = timed_read(fds[0], myf.frdata, sizeof(myf.frdata), timeout, mpg123pid);
Creating mpg123pid is completely unnecessary.  There is already a variable called pid with the child pid.



-- 
To view, visit https://gerrit.asterisk.org/9179
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib7df8462e3e380cb328011890ad9270d9e9b4620
Gerrit-Change-Number: 9179
Gerrit-PatchSet: 1
Gerrit-Owner: Sam Wierema <sam at messagebird.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Tue, 12 Jun 2018 21:50:31 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180612/42e534ff/attachment.html>