Handle DTMF and hold wrapup when a channel leaves the bridging system.

DTMF start/end and hold/unhold events have state because a DTMF begin
event and hold event must be ended by something.

The following cases need to be handled when a channel is moved around in
the system.

* When a channel leaves a bridge it may owe a DTMF end event to the
bridge.

* When a channel leaves a bridge it may owe an UNHOLD event to the bridge.
(This case is explicitly ignored because things like transfers need
explicit control over this.)

* When a channel leaves the bridging system it may need to simulate a DTMF
end event to the channel.

* When a channel leaves the bridging system it may need to simulate an
UNHOLD event to the channel.

The patch also fixes the following:
* Fixes playing a file and restarting MOH using the latest MOH class used.

(closes issue ASTERISK-22043)
Reported by: Matt Jordan

Review: https://reviewboard.asterisk.org/r/2791/

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@397577 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/bridges/bridge_builtin_interval_features.c b/bridges/bridge_builtin_interval_features.c
index e804729bfa7d425bfb26498e06e2a44ac4ba76b3..bb07e5058f8b5b24d7551f37f255c5a0c3c38170 100644 (file)
--- a/bridges/bridge_builtin_interval_features.c
+++ b/bridges/bridge_builtin_interval_features.c
@@ -103,12 +103,14 @@ static void limits_interval_playback(struct ast_bridge_channel *bridge_channel,
        /*
         * It may be necessary to resume music on hold after we finish
         * playing the announcment.
-        *
-        * XXX We have no idea what MOH class was in use before playing
-        * the file.
         */
        if (ast_test_flag(ast_channel_flags(bridge_channel->chan), AST_FLAG_MOH)) {
-               ast_moh_start(bridge_channel->chan, NULL, NULL);
+               const char *latest_musicclass;
+
+               ast_channel_lock(bridge_channel->chan);
+               latest_musicclass = ast_strdupa(ast_channel_latest_musicclass(bridge_channel->chan));
+               ast_channel_unlock(bridge_channel->chan);
+               ast_moh_start(bridge_channel->chan, latest_musicclass, NULL);
        }
 }
 

diff --git a/include/asterisk/bridge_channel.h b/include/asterisk/bridge_channel.h
index ae5131cef21bfce4d576d4064c08db4bbd84e9c4..149c8e0d198d3e73de3f301b6dd38e71f07e7cd8 100644 (file)
--- a/include/asterisk/bridge_channel.h
+++ b/include/asterisk/bridge_channel.h
@@ -155,6 +155,13 @@ struct ast_bridge_channel {
         * \note Needs to be atomically settable.
         */
        enum bridge_channel_thread_state activity;
+       /*! Owed events to the bridge. */
+       struct {
+               /*! Time started sending the current digit. (Invalid if owed.dtmf_digit is zero.) */
+               struct timeval dtmf_tv;
+               /*! Digit currently sending into the bridge. (zero if not sending) */
+               char dtmf_digit;
+       } owed;
 };
 
 /*!

diff --git a/include/asterisk/bridge_channel_internal.h b/include/asterisk/bridge_channel_internal.h
index 1108c88d7ae04477e482cd7701816bd0ba385f00..09a447d616f1614eed3130d395e1343b7d2fa238 100644 (file)
--- a/include/asterisk/bridge_channel_internal.h
+++ b/include/asterisk/bridge_channel_internal.h
@@ -82,6 +82,20 @@ enum bridge_channel_action_type {
  */
 struct ast_bridge_channel *bridge_channel_internal_alloc(struct ast_bridge *bridge);
 
+/*!
+ * \internal
+ * \brief Clear owed events by the channel to the original bridge.
+ * \since 12.0.0
+ *
+ * \param orig_bridge Original bridge the channel was in before leaving.
+ * \param bridge_channel Channel that owes events to the original bridge.
+ *
+ * \note On entry, the orig_bridge is already locked.
+ *
+ * \return Nothing
+ */
+void bridge_channel_settle_owed_events(struct ast_bridge *orig_bridge, struct ast_bridge_channel *bridge_channel);
+
 /*!
  * \internal
  * \brief Push the bridge channel into its specified bridge.

diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h
index 0175d678c4e8cb387cf30d87f770e9931ea6c26f..24dc53f6bb602239e0d22dda580cbf8cc254e492 100644 (file)
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -3792,6 +3792,7 @@ void ast_channel_name_set(struct ast_channel *chan, const char *name);
 DECLARE_STRINGFIELD_SETTERS_FOR(name);
 DECLARE_STRINGFIELD_SETTERS_FOR(language);
 DECLARE_STRINGFIELD_SETTERS_FOR(musicclass);
+DECLARE_STRINGFIELD_SETTERS_FOR(latest_musicclass);
 DECLARE_STRINGFIELD_SETTERS_FOR(accountcode);
 DECLARE_STRINGFIELD_SETTERS_FOR(peeraccount);
 DECLARE_STRINGFIELD_SETTERS_FOR(userfield);
@@ -3805,6 +3806,7 @@ DECLARE_STRINGFIELD_SETTERS_FOR(dialcontext);
 const char *ast_channel_name(const struct ast_channel *chan);
 const char *ast_channel_language(const struct ast_channel *chan);
 const char *ast_channel_musicclass(const struct ast_channel *chan);
+const char *ast_channel_latest_musicclass(const struct ast_channel *chan);
 const char *ast_channel_accountcode(const struct ast_channel *chan);
 const char *ast_channel_peeraccount(const struct ast_channel *chan);
 const char *ast_channel_userfield(const struct ast_channel *chan);
@@ -3857,6 +3859,8 @@ int ast_channel_timingfd(const struct ast_channel *chan);
 void ast_channel_timingfd_set(struct ast_channel *chan, int value);
 int ast_channel_visible_indication(const struct ast_channel *chan);
 void ast_channel_visible_indication_set(struct ast_channel *chan, int value);
+int ast_channel_hold_state(const struct ast_channel *chan);
+void ast_channel_hold_state_set(struct ast_channel *chan, int value);
 int ast_channel_vstreamid(const struct ast_channel *chan);
 void ast_channel_vstreamid_set(struct ast_channel *chan, int value);
 unsigned short ast_channel_transfercapability(const struct ast_channel *chan);

diff --git a/main/bridge.c b/main/bridge.c
index 3456be26e7aa12d6b33235db2bfb5a1bb6e6cd6f..ddaab0ca06555da9126765f39080974d47f23aae 100644 (file)
--- a/main/bridge.c
+++ b/main/bridge.c
@@ -2034,12 +2034,16 @@ int bridge_do_move(struct ast_bridge *dst_bridge, struct ast_bridge_channel *bri
                        if (bridge_channel_internal_push(bridge_channel)) {
                                ast_bridge_channel_leave_bridge(bridge_channel,
                                        BRIDGE_CHANNEL_STATE_END_NO_DISSOLVE, bridge_channel->bridge->cause);
+                               bridge_channel_settle_owed_events(orig_bridge, bridge_channel);
                        }
                } else {
                        ast_bridge_channel_leave_bridge(bridge_channel,
                                BRIDGE_CHANNEL_STATE_END_NO_DISSOLVE, bridge_channel->bridge->cause);
+                       bridge_channel_settle_owed_events(orig_bridge, bridge_channel);
                }
                res = -1;
+       } else {
+               bridge_channel_settle_owed_events(orig_bridge, bridge_channel);
        }
 
        bridge_reconfigured(dst_bridge, !optimized);

diff --git a/main/bridge_channel.c b/main/bridge_channel.c
index 385be6d3eb3aa66aaa541c4b3a35dd7ae3e1cd28..7e26f6598ea88b1a01ca769678060069d56a1e6c 100644 (file)
--- a/main/bridge_channel.c
+++ b/main/bridge_channel.c
@@ -346,6 +346,24 @@ static int bridge_channel_write_frame(struct ast_bridge_channel *bridge_channel,
  * simple_bridge/native_bridge are likely the only techs that will do this.
  */
        bridge_channel->bridge->technology->write(bridge_channel->bridge, bridge_channel, frame);
+
+       /* Remember any owed events to the bridge. */
+       switch (frame->frametype) {
+       case AST_FRAME_DTMF_BEGIN:
+               bridge_channel->owed.dtmf_tv = ast_tvnow();
+               bridge_channel->owed.dtmf_digit = frame->subclass.integer;
+               break;
+       case AST_FRAME_DTMF_END:
+               bridge_channel->owed.dtmf_digit = '\0';
+               break;
+       case AST_FRAME_CONTROL:
+               /*
+                * We explicitly will not remember HOLD/UNHOLD frames because
+                * things like attended transfers will handle them.
+                */
+       default:
+               break;
+       }
        ast_bridge_unlock(bridge_channel->bridge);
 
        /*
@@ -355,6 +373,27 @@ static int bridge_channel_write_frame(struct ast_bridge_channel *bridge_channel,
        return 0;
 }
 
+void bridge_channel_settle_owed_events(struct ast_bridge *orig_bridge, struct ast_bridge_channel *bridge_channel)
+{
+       if (bridge_channel->owed.dtmf_digit) {
+               struct ast_frame frame = {
+                       .frametype = AST_FRAME_DTMF_END,
+                       .subclass.integer = bridge_channel->owed.dtmf_digit,
+                       .src = "Bridge channel owed DTMF",
+               };
+
+               frame.len = ast_tvdiff_ms(ast_tvnow(), bridge_channel->owed.dtmf_tv);
+               if (frame.len < option_dtmfminduration) {
+                       frame.len = option_dtmfminduration;
+               }
+               ast_log(LOG_DTMF, "DTMF end '%c' simulated to bridge %s because %s left.  Duration %ld ms.\n",
+                       bridge_channel->owed.dtmf_digit, orig_bridge->uniqueid,
+                       ast_channel_name(bridge_channel->chan), frame.len);
+               bridge_channel->owed.dtmf_digit = '\0';
+               orig_bridge->technology->write(orig_bridge, NULL, &frame);
+       }
+}
+
 /*!
  * \internal
  * \brief Suspend a channel from a bridge.
@@ -719,14 +758,14 @@ void ast_bridge_channel_playfile(struct ast_bridge_channel *bridge_channel, ast_
        /*
         * It may be necessary to resume music on hold after we finish
         * playing the announcment.
-        *
-        * XXX We have no idea what MOH class was in use before playing
-        * the file. This method also fails to restore ringing indications.
-        * the proposed solution is to create a resume_entertainment callback
-        * for the bridge technology and execute it here.
         */
        if (ast_test_flag(ast_channel_flags(bridge_channel->chan), AST_FLAG_MOH)) {
-               ast_moh_start(bridge_channel->chan, NULL, NULL);
+               const char *latest_musicclass;
+
+               ast_channel_lock(bridge_channel->chan);
+               latest_musicclass = ast_strdupa(ast_channel_latest_musicclass(bridge_channel->chan));
+               ast_channel_unlock(bridge_channel->chan);
+               ast_moh_start(bridge_channel->chan, latest_musicclass, NULL);
        }
 }
 
@@ -1420,8 +1459,6 @@ void bridge_channel_internal_pull(struct ast_bridge_channel *bridge_channel)
                bridge->v_table->name,
                bridge->uniqueid);
 
-/* BUGBUG This is where incoming HOLD/UNHOLD memory should write UNHOLD into bridge. (if not local optimizing) */
-/* BUGBUG This is where incoming DTMF begin/end memory should write DTMF end into bridge. (if not local optimizing) */
        if (!bridge_channel->just_joined) {
                /* Tell the bridge technology we are leaving so they tear us down */
                ast_debug(1, "Bridge %s: %p(%s) is leaving %s technology\n",
@@ -1559,17 +1596,6 @@ static void bridge_channel_handle_control(struct ast_bridge_channel *bridge_chan
                        ast_indicate_data(chan, fr->subclass.integer, fr->data.ptr, fr->datalen);
                }
                break;
-       case AST_CONTROL_HOLD:
-       case AST_CONTROL_UNHOLD:
-/*
- * BUGBUG bridge_channels should remember sending/receiving an outstanding HOLD to/from the bridge
- *
- * When the sending channel is pulled from the bridge it needs to write into the bridge an UNHOLD before being pulled.
- * When the receiving channel is pulled from the bridge it needs to generate its own UNHOLD.
- * Something similar needs to be done for DTMF begin/end.
- */
-               ast_indicate_data(chan, fr->subclass.integer, fr->data.ptr, fr->datalen);
-               break;
        case AST_CONTROL_OPTION:
                /*
                 * Forward option Requests, but only ones we know are safe These
@@ -1720,7 +1746,6 @@ static void bridge_handle_trip(struct ast_bridge_channel *bridge_channel)
                        ast_bridge_channel_kick(bridge_channel, 0);
                        ast_frfree(frame);
                        return;
-/* BUGBUG This is where incoming HOLD/UNHOLD memory should register.  Write UNHOLD into bridge when this channel is pulled. */
                default:
                        break;
                }
@@ -1735,7 +1760,6 @@ static void bridge_handle_trip(struct ast_bridge_channel *bridge_channel)
                        ast_frfree(frame);
                        return;
                }
-/* BUGBUG This is where incoming DTMF begin/end memory should register.  Write DTMF end into bridge when this channel is pulled. */
                break;
        default:
                break;
@@ -1887,6 +1911,7 @@ static void bridge_channel_event_join_leave(struct ast_bridge_channel *bridge_ch
 int bridge_channel_internal_join(struct ast_bridge_channel *bridge_channel)
 {
        int res = 0;
+
        ast_format_copy(&bridge_channel->read_format, ast_channel_readformat(bridge_channel->chan));
        ast_format_copy(&bridge_channel->write_format, ast_channel_writeformat(bridge_channel->chan));
 
@@ -1952,15 +1977,18 @@ int bridge_channel_internal_join(struct ast_bridge_channel *bridge_channel)
        }
 
        bridge_channel_internal_pull(bridge_channel);
+       bridge_channel_settle_owed_events(bridge_channel->bridge, bridge_channel);
        bridge_reconfigured(bridge_channel->bridge, 1);
 
        ast_bridge_unlock(bridge_channel->bridge);
 
-       /* Indicate a source change since this channel is leaving the bridge system. */
-       ast_indicate(bridge_channel->chan, AST_CONTROL_SRCCHANGE);
+       /* Complete any active hold before exiting the bridge. */
+       if (ast_channel_hold_state(bridge_channel->chan) == AST_CONTROL_HOLD) {
+               ast_debug(1, "Channel %s simulating UNHOLD for bridge end.\n",
+                       ast_channel_name(bridge_channel->chan));
+               ast_indicate(bridge_channel->chan, AST_CONTROL_UNHOLD);
+       }
 
-/* BUGBUG Revisit in regards to moving channels between bridges and local channel optimization. */
-/* BUGBUG This is where outgoing HOLD/UNHOLD memory should write UNHOLD to channel. */
        /* Complete any partial DTMF digit before exiting the bridge. */
        if (ast_channel_sending_dtmf_digit(bridge_channel->chan)) {
                ast_channel_end_dtmf(bridge_channel->chan,
@@ -1968,6 +1996,9 @@ int bridge_channel_internal_join(struct ast_bridge_channel *bridge_channel)
                        ast_channel_sending_dtmf_tv(bridge_channel->chan), "bridge end");
        }
 
+       /* Indicate a source change since this channel is leaving the bridge system. */
+       ast_indicate(bridge_channel->chan, AST_CONTROL_SRCCHANGE);
+
        /*
         * Wait for any dual redirect to complete.
         *

diff --git a/main/channel.c b/main/channel.c
index 1abed5e9412b44484c60832d7d5981d8e9014b73..f085d31e8fcc412e14b5e82402257028239c0a5c 100644 (file)
--- a/main/channel.c
+++ b/main/channel.c
@@ -941,6 +941,7 @@ __ast_channel_alloc_ap(int needqueue, int state, const char *cid_num, const char
 
        /* Initial state */
        ast_channel_state_set(tmp, state);
+       ast_channel_hold_state_set(tmp, AST_CONTROL_UNHOLD);
 
        ast_channel_streamid_set(tmp, -1);
 
@@ -1072,6 +1073,8 @@ struct ast_channel *ast_dummy_channel_alloc(void)
        ast_channel_epfd_set(tmp, -1);
 #endif
 
+       ast_channel_hold_state_set(tmp, AST_CONTROL_UNHOLD);
+
        ast_channel_internal_setup_topics(tmp);
 
        headp = ast_channel_varshead(tmp);
@@ -4453,7 +4456,6 @@ int ast_indicate_data(struct ast_channel *chan, int _condition,
                                ast_channel_connected(chan));
                }
                break;
-
        case AST_CONTROL_REDIRECTING:
                {
                        struct ast_party_redirecting redirecting;
@@ -4466,7 +4468,10 @@ int ast_indicate_data(struct ast_channel *chan, int _condition,
                        ast_party_redirecting_free(&redirecting);
                }
                break;
-
+       case AST_CONTROL_HOLD:
+       case AST_CONTROL_UNHOLD:
+               ast_channel_hold_state_set(chan, condition);
+               break;
        default:
                break;
        }
@@ -6376,6 +6381,7 @@ void ast_do_masquerade(struct ast_channel *original)
        int origstate;
        int visible_indication;
        int clone_was_zombie = 0;/*!< TRUE if the clonechan was a zombie before the masquerade. */
+       int clone_hold_state;
        struct ast_frame *current;
        const struct ast_channel_tech *t;
        void *t_pvt;
@@ -6478,6 +6484,8 @@ void ast_do_masquerade(struct ast_channel *original)
        free_translation(clonechan);
        free_translation(original);
 
+       clone_hold_state = ast_channel_hold_state(clonechan);
+
        /* Save the current DTMF digit being sent if any. */
        clone_sending_dtmf_digit = ast_channel_sending_dtmf_digit(clonechan);
        clone_sending_dtmf_tv = ast_channel_sending_dtmf_tv(clonechan);
@@ -6712,6 +6720,11 @@ void ast_do_masquerade(struct ast_channel *original)
 
        ast_bridge_notify_masquerade(original);
 
+       if (clone_hold_state == AST_CONTROL_HOLD) {
+               ast_debug(1, "Channel %s simulating UNHOLD for masquerade.\n",
+                       ast_channel_name(original));
+               ast_indicate(original, AST_CONTROL_UNHOLD);
+       }
        if (clone_sending_dtmf_digit) {
                /*
                 * The clonechan was sending a DTMF digit that was not completed
@@ -6731,7 +6744,23 @@ void ast_do_masquerade(struct ast_channel *original)
         * (RINGING, CONGESTION, etc.)
         */
        if (visible_indication) {
-               ast_indicate(original, visible_indication);
+               if (visible_indication == AST_CONTROL_HOLD) {
+                       const char *latest_musicclass;
+                       int len;
+
+                       ast_channel_lock(original);
+                       latest_musicclass = ast_strdupa(ast_channel_latest_musicclass(original));
+                       ast_channel_unlock(original);
+                       if (ast_strlen_zero(latest_musicclass)) {
+                               latest_musicclass = NULL;
+                               len = 0;
+                       } else {
+                               len = strlen(latest_musicclass) + 1;
+                       }
+                       ast_indicate_data(original, visible_indication, latest_musicclass, len);
+               } else {
+                       ast_indicate(original, visible_indication);
+               }
        }
 
        ast_channel_lock(original);
@@ -10390,6 +10419,9 @@ void ast_channel_end_dtmf(struct ast_channel *chan, char digit, struct timeval s
        }
 
        duration = ast_tvdiff_ms(ast_tvnow(), start);
+       if (duration < option_dtmfminduration) {
+               duration = option_dtmfminduration;
+       }
        ast_senddigit_end(chan, digit, duration);
        ast_log(LOG_DTMF, "DTMF end '%c' simulated on %s due to %s, duration %ld ms\n",
                digit, ast_channel_name(chan), why, duration);

diff --git a/main/channel_internal_api.c b/main/channel_internal_api.c
index 122b8cfdf91c12af41cd48bd04cab5b8560535d1..956816d764592db054b228b0cf81ef8449cb2e97 100644 (file)
--- a/main/channel_internal_api.c
+++ b/main/channel_internal_api.c
@@ -96,6 +96,7 @@ struct ast_channel {
                AST_STRING_FIELD(name);         /*!< ASCII unique channel name */
                AST_STRING_FIELD(language);     /*!< Language requested for voice prompts */
                AST_STRING_FIELD(musicclass);   /*!< Default music class */
+               AST_STRING_FIELD(latest_musicclass);   /*!< Latest active music class */
                AST_STRING_FIELD(accountcode);  /*!< Account code for billing */
                AST_STRING_FIELD(peeraccount);  /*!< Peer account code for billing */
                AST_STRING_FIELD(userfield);    /*!< Userfield for CEL billing */
@@ -190,6 +191,7 @@ struct ast_channel {
        int epfd;
 #endif
        int visible_indication;                         /*!< Indication currently playing on the channel */
+       int hold_state;                                                 /*!< Current Hold/Unhold state */
 
        unsigned short transfercapability;              /*!< ISDN Transfer Capability - AST_FLAG_DIGITAL is not enough */
 
@@ -444,6 +446,7 @@ void ast_channel_##field##_build(struct ast_channel *chan, const char *fmt, ...)
 DEFINE_STRINGFIELD_SETTERS_FOR(name, 0, 1);
 DEFINE_STRINGFIELD_SETTERS_FOR(language, 1, 0);
 DEFINE_STRINGFIELD_SETTERS_FOR(musicclass, 0, 0);
+DEFINE_STRINGFIELD_SETTERS_FOR(latest_musicclass, 0, 0);
 DEFINE_STRINGFIELD_SETTERS_FOR(accountcode, 1, 0);
 DEFINE_STRINGFIELD_SETTERS_FOR(peeraccount, 1, 0);
 DEFINE_STRINGFIELD_SETTERS_FOR(userfield, 0, 0);
@@ -462,6 +465,7 @@ DEFINE_STRINGFIELD_SETTERS_FOR(dialcontext, 0, 0);
 DEFINE_STRINGFIELD_GETTER_FOR(name);
 DEFINE_STRINGFIELD_GETTER_FOR(language);
 DEFINE_STRINGFIELD_GETTER_FOR(musicclass);
+DEFINE_STRINGFIELD_GETTER_FOR(latest_musicclass);
 DEFINE_STRINGFIELD_GETTER_FOR(accountcode);
 DEFINE_STRINGFIELD_GETTER_FOR(peeraccount);
 DEFINE_STRINGFIELD_GETTER_FOR(userfield);
@@ -645,6 +649,14 @@ void ast_channel_visible_indication_set(struct ast_channel *chan, int value)
 {
        chan->visible_indication = value;
 }
+int ast_channel_hold_state(const struct ast_channel *chan)
+{
+       return chan->hold_state;
+}
+void ast_channel_hold_state_set(struct ast_channel *chan, int value)
+{
+       chan->hold_state = value;
+}
 int ast_channel_vstreamid(const struct ast_channel *chan)
 {
        return chan->vstreamid;

diff --git a/res/res_musiconhold.c b/res/res_musiconhold.c
index df6c7d787b266217f0de8be05f29802c43abf707..70c985eb9c2d71cf92aa8bc4e0c71a8e4c8c701a 100644 (file)
--- a/res/res_musiconhold.c
+++ b/res/res_musiconhold.c
@@ -1570,6 +1570,7 @@ static int local_ast_moh_start(struct ast_channel *chan, const char *mclass, con
                }
        }
 
+       ast_channel_latest_musicclass_set(chan, mohclass->name);
        ast_set_flag(ast_channel_flags(chan), AST_FLAG_MOH);
 
        if (mohclass->total_files) {