[Asterisk-code-review] res musiconhold.c: Restart MOH if previous hold just reached... (asterisk[16])

[George Joseph]

George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/10056 )

Change subject: res_musiconhold.c: Restart MOH if previous hold just reached end-of-file
......................................................................

res_musiconhold.c: Restart MOH if previous hold just reached end-of-file

On MOH activation, moh_files_readframe() is called while the current
stream attached to the channel is NULL and it calls ast_moh_files_next()
immediately.  However, it won't call ast_moh_files_next() again if sample
reading fails.  The failure may occur because res_musiconhold retains the
last sample reading position in the channel data and MOH during the
previous hold/retrieve just reached EOF.  Obviously, a bit of bad luck is
required here.

* Restructured moh_files_readframe() to try a second time to start MOH if
there was no stream setup and the saved position was at EOF.  Also added
comments describing what is going on for each step.

ASTERISK-28029

Change-Id: I1508cf2c094f8feca22d6f76deaa9fdfa9944860
---
M res/res_musiconhold.c
1 file changed, 20 insertions(+), 3 deletions(-)

Approvals:
  Michael L. Young: Looks good to me, but someone else must approve
  Matthew Fredrickson: Looks good to me, approved
  George Joseph: Approved for Submit



diff --git a/res/res_musiconhold.c b/res/res_musiconhold.c
index 137f83b..5c739ab 100644
--- a/res/res_musiconhold.c
+++ b/res/res_musiconhold.c
@@ -402,11 +402,28 @@
 
 static struct ast_frame *moh_files_readframe(struct ast_channel *chan)
 {
-	struct ast_frame *f = NULL;
+	struct ast_frame *f;
 
-	if (!(ast_channel_stream(chan) && (f = ast_readframe(ast_channel_stream(chan))))) {
-		if (!ast_moh_files_next(chan))
+	f = ast_readframe(ast_channel_stream(chan));
+	if (!f) {
+		/* Either there was no file stream setup or we reached EOF. */
+		if (!ast_moh_files_next(chan)) {
+			/*
+			 * Either we resetup the previously saved file stream position
+			 * or we started a new file stream.
+			 */
 			f = ast_readframe(ast_channel_stream(chan));
+			if (!f) {
+				/*
+				 * We can get here if we were very unlucky because the
+				 * resetup file stream was saved at EOF when MOH was
+				 * previously stopped.
+				 */
+				if (!ast_moh_files_next(chan)) {
+					f = ast_readframe(ast_channel_stream(chan));
+				}
+			}
+		}
 	}
 
 	return f;

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

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-MessageType: merged
Gerrit-Change-Id: I1508cf2c094f8feca22d6f76deaa9fdfa9944860
Gerrit-Change-Number: 10056
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Frederic LE FOLL <frederic.lefoll at c-s.fr>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>
Gerrit-Reviewer: Michael L. Young <elgueromexicano at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180914/9c4a2253/attachment.html>