[Asterisk-code-review] bridge softmix: Clear "talking" when a channel is put on hold (asterisk[certified/13.18])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/8617 )

Change subject: bridge_softmix: Clear "talking" when a channel is put on hold
......................................................................

bridge_softmix: Clear "talking" when a channel is put on hold

This patch clears the talking flag from the channel (if already set), and
notifies listeners when that channel is put on hold. Note however, if the
endpoint continues to send audio frames and these are received by the bridge
then that channel will be put back into a "talking" state even though they
are on hold.

ASTERISK-27755 #close

Change-Id: I930e16c4662810f9f02043d69062f88173c5e2ef
---
M bridges/bridge_softmix.c
1 file changed, 37 insertions(+), 11 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve; Approved for Submit
  George Joseph: Looks good to me, approved



diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c
index b572192..dcd3271 100644
--- a/bridges/bridge_softmix.c
+++ b/bridges/bridge_softmix.c
@@ -667,6 +667,30 @@
 
 /*!
  * \internal
+ * \brief Clear talking flag, stop contributing to mixing and notify handlers.
+ * \since 13.21.0, 15.4.0
+ *
+ * \param bridge_channel Which channel's talking to clear
+ *
+ * \return Nothing
+ */
+static void clear_talking(struct ast_bridge_channel *bridge_channel)
+{
+	struct softmix_channel *sc = bridge_channel->tech_pvt;
+
+	if (sc->talking) {
+		ast_mutex_lock(&sc->lock);
+		ast_slinfactory_flush(&sc->factory);
+		sc->talking = 0;
+		ast_mutex_unlock(&sc->lock);
+
+		/* Notify that we are no longer talking. */
+		ast_bridge_channel_notify_talking(bridge_channel, 0);
+	}
+}
+
+/*!
+ * \internal
  * \brief Check for voice status updates.
  * \since 13.20.0
  *
@@ -677,23 +701,14 @@
  */
 static void softmix_bridge_check_voice(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel)
 {
-	struct softmix_channel *sc = bridge_channel->tech_pvt;
-
-	if (sc->talking
-		&& bridge_channel->features->mute) {
+	if (bridge_channel->features->mute) {
 		/*
 		 * We were muted while we were talking.
 		 *
 		 * Immediately stop contributing to mixing
 		 * and report no longer talking.
 		 */
-		ast_mutex_lock(&sc->lock);
-		ast_slinfactory_flush(&sc->factory);
-		sc->talking = 0;
-		ast_mutex_unlock(&sc->lock);
-
-		/* Notify that we are no longer talking. */
-		ast_bridge_channel_notify_talking(bridge_channel, 0);
+		clear_talking(bridge_channel);
 	}
 }
 
@@ -717,6 +732,17 @@
 	 */
 
 	switch (frame->subclass.integer) {
+	case AST_CONTROL_HOLD:
+		/*
+		 * Doing anything for holds in a conference bridge could be considered a bit
+		 * odd. That being said, in most cases one would probably want the talking
+		 * flag cleared when 'hold' is pressed by the remote endpoint, so go ahead
+		 * and do that here. However, that is all we'll do. Meaning if for some reason
+		 * the endpoint continues to send audio frames despite pressing 'hold' talking
+		 * will once again be detected for that channel.
+		 */
+		clear_talking(bridge_channel);
+		break;
 	case AST_CONTROL_VIDUPDATE:
 		ast_bridge_queue_everyone_else(bridge, NULL, frame);
 		break;

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

Gerrit-Project: asterisk
Gerrit-Branch: certified/13.18
Gerrit-MessageType: merged
Gerrit-Change-Id: I930e16c4662810f9f02043d69062f88173c5e2ef
Gerrit-Change-Number: 8617
Gerrit-PatchSet: 1
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180322/78691a9d/attachment-0001.html>