[Asterisk-code-review] app amd: Infinite loop on silent calls (asterisk[16])

[Richard Mudgett]

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

Change subject: app_amd: Infinite loop on silent calls
......................................................................


Patch Set 1: Code-Review-1

(3 comments)

This didn't address Josh's concerns in the original review.

In fact you created an entirely new review which is wrong.  Every time you change the Change-Id in your commit message and post it to Gerrit that creates a new unrelated review request rather than updating the previous review.  Do not change the Change-Id when you make alterations to your patch.

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

https://gerrit.asterisk.org/#/c/10736/1//COMMIT_MSG@7
PS1, Line 7: app_amd: Infinite loop on silent calls
           : 
           : Change-Id: I24726bd29d7e467fc721ca265363417234b22855
Add the JIRA issue ID as directed by the wiki page Josh indicated on the other review.


https://gerrit.asterisk.org/#/c/10736/1/apps/app_amd.c
File apps/app_amd.c:

https://gerrit.asterisk.org/#/c/10736/1/apps/app_amd.c@283
PS1, Line 283: 		if (res > 0) {
             : 			ms = 2 * maxWaitTimeForFrame - res;
             : 		} else {
             : 			ms = 2 * maxWaitTimeForFrame;
             :  		}
This could just be:
ms = 2 * maxWaitTimeForFrame - res;

The while loop test only allows res >= 0.

However, the JIRA issue has no information on the scenario you are trying to fix.


https://gerrit.asterisk.org/#/c/10736/1/apps/app_amd.c@298
PS1, Line 298: 		if (f->frametype == AST_FRAME_VOICE || f->frametype == AST_FRAME_CNG || f->frametype == AST_FRAME_NULL) {
This was not addressed by Josh's previous comment.  What is the scenario that causes the loop you are trying to fix?



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

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-MessageType: comment
Gerrit-Change-Id: I24726bd29d7e467fc721ca265363417234b22855
Gerrit-Change-Number: 10736
Gerrit-PatchSet: 1
Gerrit-Owner: Abhay Gupta <abhay at avissol.com>
Gerrit-Reviewer: Jenkins2 (1000185)
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Wed, 05 Dec 2018 16:32:44 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20181205/f306e976/attachment.html>