[Asterisk-code-review] bridging: Add better support for adding/removing streams. (asterisk[certified/16.8])
George Joseph
asteriskteam at digium.com
Wed Mar 4 09:32:06 CST 2020
George Joseph has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/13850 )
Change subject: bridging: Add better support for adding/removing streams.
......................................................................
bridging: Add better support for adding/removing streams.
This change adds support to bridge_softmix to allow the addition
and removal of additional video source streams. When such a change
occurs each participant is renegotiated as needed to reflect the
update. If another video source is added then each participant
gets another source. If a video source is removed then it is
removed from each participant. This functionality allows you to
have both your webcam and screenshare providing video if you
desire, or even more streams. Mapping has been changed to use
the topology index on the source channel as a unique identifier
for outgoing participant streams, this will never change and
provides an easy way to establish the mapping.
The bridge_simple and bridge_native_rtp modules have also been
updated to renegotiate when the stream topology of a party changes
allowing the same behavior to occur as added to bridge_softmix.
If a screen share is added then the opposite party is renegotiated.
If that screen share is removed then the opposite party is
renegotiated again.
Some additional fixes are also included in here. Stream state is
now conveyed in SDP so sendonly/recvonly/inactive streams can
be requested. Removed streams now also remove previous state
from themselves so consumers don't get confused.
ASTERISK-28733
Change-Id: I93f41fb41b85646bef71408111c17ccea30cb0c5
---
M bridges/bridge_native_rtp.c
M bridges/bridge_simple.c
M bridges/bridge_softmix.c
M include/asterisk/channel.h
M main/channel.c
M main/stream.c
M res/res_pjsip_sdp_rtp.c
M res/res_pjsip_session.c
8 files changed, 571 insertions(+), 150 deletions(-)
Approvals:
George Joseph: Looks good to me, approved; Approved for Submit
diff --git a/bridges/bridge_native_rtp.c b/bridges/bridge_native_rtp.c
index 7fd4ae1..a6addf2 100644
--- a/bridges/bridge_native_rtp.c
+++ b/bridges/bridge_native_rtp.c
@@ -43,6 +43,7 @@
#include "asterisk/bridge_technology.h"
#include "asterisk/frame.h"
#include "asterisk/rtp_engine.h"
+#include "asterisk/stream.h"
/*! \brief Internal structure which contains bridged RTP channel hook data */
struct native_rtp_framehook_data {
@@ -85,6 +86,28 @@
struct rtp_glue_data glue;
};
+/*! \brief Forward declarations */
+static int native_rtp_bridge_join(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel);
+static void native_rtp_bridge_unsuspend(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel);
+static void native_rtp_bridge_leave(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel);
+static void native_rtp_bridge_suspend(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel);
+static int native_rtp_bridge_write(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel, struct ast_frame *frame);
+static int native_rtp_bridge_compatible(struct ast_bridge *bridge);
+static void native_rtp_stream_topology_changed(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel);
+
+static struct ast_bridge_technology native_rtp_bridge = {
+ .name = "native_rtp",
+ .capabilities = AST_BRIDGE_CAPABILITY_NATIVE,
+ .preference = AST_BRIDGE_PREFERENCE_BASE_NATIVE,
+ .join = native_rtp_bridge_join,
+ .unsuspend = native_rtp_bridge_unsuspend,
+ .leave = native_rtp_bridge_leave,
+ .suspend = native_rtp_bridge_suspend,
+ .write = native_rtp_bridge_write,
+ .compatible = native_rtp_bridge_compatible,
+ .stream_topology_changed = native_rtp_stream_topology_changed,
+};
+
static void rtp_glue_data_init(struct rtp_glue_data *glue)
{
glue->cb = NULL;
@@ -831,12 +854,124 @@
data->hook_data = NULL;
}
+static struct ast_stream_topology *native_rtp_request_stream_topology_update(
+ struct ast_stream_topology *existing_topology,
+ struct ast_stream_topology *requested_topology)
+{
+ struct ast_stream *stream;
+ struct ast_format_cap *audio_formats = NULL;
+ struct ast_stream_topology *new_topology;
+ int i;
+
+ new_topology = ast_stream_topology_clone(requested_topology);
+ if (!new_topology) {
+ return NULL;
+ }
+
+ /* We find an existing stream with negotiated audio formats that we can place into
+ * any audio streams in the new topology to ensure that negotiation succeeds. Some
+ * endpoints incorrectly terminate the call if SDP negotiation fails.
+ */
+ for (i = 0; i < ast_stream_topology_get_count(existing_topology); ++i) {
+ stream = ast_stream_topology_get_stream(existing_topology, i);
+
+ if (ast_stream_get_type(stream) != AST_MEDIA_TYPE_AUDIO ||
+ ast_stream_get_state(stream) == AST_STREAM_STATE_REMOVED) {
+ continue;
+ }
+
+ audio_formats = ast_stream_get_formats(stream);
+ break;
+ }
+
+ if (audio_formats) {
+ for (i = 0; i < ast_stream_topology_get_count(new_topology); ++i) {
+ stream = ast_stream_topology_get_stream(new_topology, i);
+
+ if (ast_stream_get_type(stream) != AST_MEDIA_TYPE_AUDIO ||
+ ast_stream_get_state(stream) == AST_STREAM_STATE_REMOVED) {
+ continue;
+ }
+
+ ast_format_cap_append_from_cap(ast_stream_get_formats(stream), audio_formats,
+ AST_MEDIA_TYPE_AUDIO);
+ }
+ }
+
+ for (i = 0; i < ast_stream_topology_get_count(new_topology); ++i) {
+ stream = ast_stream_topology_get_stream(new_topology, i);
+
+ /* For both recvonly and sendonly the stream state reflects our state, that is we
+ * are receiving only and we are sending only. Since we are renegotiating a remote
+ * party we need to swap this to reflect what we will be doing. That is, if we are
+ * receiving from Alice then we want to be sending to Bob, so swap recvonly to
+ * sendonly.
+ */
+ if (ast_stream_get_state(stream) == AST_STREAM_STATE_RECVONLY) {
+ ast_stream_set_state(stream, AST_STREAM_STATE_SENDONLY);
+ } else if (ast_stream_get_state(stream) == AST_STREAM_STATE_SENDONLY) {
+ ast_stream_set_state(stream, AST_STREAM_STATE_RECVONLY);
+ }
+ }
+
+ return new_topology;
+}
+
+static void native_rtp_stream_topology_changed(struct ast_bridge *bridge,
+ struct ast_bridge_channel *bridge_channel)
+{
+ struct ast_channel *c0 = bridge_channel->chan;
+ struct ast_channel *c1 = AST_LIST_FIRST(&bridge->channels)->chan;
+ struct ast_stream_topology *req_top;
+ struct ast_stream_topology *existing_top;
+ struct ast_stream_topology *new_top;
+
+ ast_bridge_channel_stream_map(bridge_channel);
+
+ if (ast_channel_get_stream_topology_change_source(bridge_channel->chan)
+ == &native_rtp_bridge) {
+ return;
+ }
+
+ if (c0 == c1) {
+ c1 = AST_LIST_LAST(&bridge->channels)->chan;
+ }
+
+ if (c0 == c1) {
+ return;
+ }
+
+ /* If a party renegotiates we want to renegotiate their counterpart to a matching
+ * topology.
+ */
+ ast_channel_lock_both(c0, c1);
+ req_top = ast_channel_get_stream_topology(c0);
+ existing_top = ast_channel_get_stream_topology(c1);
+ new_top = native_rtp_request_stream_topology_update(existing_top, req_top);
+ ast_channel_unlock(c0);
+ ast_channel_unlock(c1);
+
+ if (!new_top) {
+ /* Failure. We'll just have to live with the current topology. */
+ return;
+ }
+
+ ast_channel_request_stream_topology_change(c1, new_top, &native_rtp_bridge);
+ ast_stream_topology_free(new_top);
+}
+
/*!
* \internal
* \brief Called by the bridge core 'join' callback for each channel joining he bridge
*/
static int native_rtp_bridge_join(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel)
{
+ struct ast_stream_topology *req_top;
+ struct ast_stream_topology *existing_top;
+ struct ast_stream_topology *new_top;
+ struct ast_channel *c0 = AST_LIST_FIRST(&bridge->channels)->chan;
+ struct ast_channel *c1 = AST_LIST_LAST(&bridge->channels)->chan;
+
ast_debug(2, "Bridge '%s'. Channel '%s' is joining bridge tech\n",
bridge->uniqueid, ast_channel_name(bridge_channel->chan));
@@ -858,6 +993,27 @@
return -1;
}
+ if (c0 != c1) {
+ /* When both channels are joined we want to try to improve the experience by
+ * raising the number of streams so they match.
+ */
+ ast_channel_lock_both(c0, c1);
+ req_top = ast_channel_get_stream_topology(c0);
+ existing_top = ast_channel_get_stream_topology(c1);
+ if (ast_stream_topology_get_count(req_top) < ast_stream_topology_get_count(existing_top)) {
+ SWAP(req_top, existing_top);
+ SWAP(c0, c1);
+ }
+ new_top = native_rtp_request_stream_topology_update(existing_top, req_top);
+ ast_channel_unlock(c0);
+ ast_channel_unlock(c1);
+
+ if (new_top) {
+ ast_channel_request_stream_topology_change(c1, new_top, &native_rtp_bridge);
+ ast_stream_topology_free(new_top);
+ }
+ }
+
native_rtp_bridge_start(bridge, NULL);
return 0;
}
@@ -939,18 +1095,6 @@
return defer;
}
-static struct ast_bridge_technology native_rtp_bridge = {
- .name = "native_rtp",
- .capabilities = AST_BRIDGE_CAPABILITY_NATIVE,
- .preference = AST_BRIDGE_PREFERENCE_BASE_NATIVE,
- .join = native_rtp_bridge_join,
- .unsuspend = native_rtp_bridge_unsuspend,
- .leave = native_rtp_bridge_leave,
- .suspend = native_rtp_bridge_suspend,
- .write = native_rtp_bridge_write,
- .compatible = native_rtp_bridge_compatible,
-};
-
static int unload_module(void)
{
ast_bridge_technology_unregister(&native_rtp_bridge);
diff --git a/bridges/bridge_simple.c b/bridges/bridge_simple.c
index 40f7ddc..545b3ad 100644
--- a/bridges/bridge_simple.c
+++ b/bridges/bridge_simple.c
@@ -46,63 +46,8 @@
static void simple_bridge_stream_topology_changed(struct ast_bridge *bridge,
struct ast_bridge_channel *bridge_channel);
-
-static int simple_bridge_join(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel)
-{
- struct ast_channel *c0 = AST_LIST_FIRST(&bridge->channels)->chan;
- struct ast_channel *c1 = AST_LIST_LAST(&bridge->channels)->chan;
-
- /*
- * If this is the first channel we can't make it compatible...
- * unless we make it compatible with itself. O.o
- */
- if (c0 == c1) {
- return 0;
- }
-
- if (ast_channel_make_compatible(c0, c1)) {
- return -1;
- }
-
- /* Align stream topologies */
- simple_bridge_stream_topology_changed(bridge, NULL);
- return 0;
-}
-
-static int simple_bridge_write(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel, struct ast_frame *frame)
-{
- const struct ast_control_t38_parameters *t38_parameters;
- int defer = 0;
-
- if (!ast_bridge_queue_everyone_else(bridge, bridge_channel, frame)) {
- /* This frame was successfully queued so no need to defer */
- return 0;
- }
-
- /* Depending on the frame defer it so when the next channel joins it receives it */
- switch (frame->frametype) {
- case AST_FRAME_CONTROL:
- switch (frame->subclass.integer) {
- case AST_CONTROL_T38_PARAMETERS:
- t38_parameters = frame->data.ptr;
- switch (t38_parameters->request_response) {
- case AST_T38_REQUEST_NEGOTIATE:
- defer = -1;
- break;
- default:
- break;
- }
- break;
- default:
- break;
- }
- break;
- default:
- break;
- }
-
- return defer;
-}
+static int simple_bridge_join(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel);
+static int simple_bridge_write(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel, struct ast_frame *frame);
static struct ast_bridge_technology simple_bridge = {
.name = "simple_bridge",
@@ -157,52 +102,145 @@
}
}
+ for (i = 0; i < ast_stream_topology_get_count(new_topology); ++i) {
+ stream = ast_stream_topology_get_stream(new_topology, i);
+
+ /* For both recvonly and sendonly the stream state reflects our state, that is we
+ * are receiving only and we are sending only. Since we are renegotiating a remote
+ * party we need to swap this to reflect what we will be doing. That is, if we are
+ * receiving from Alice then we want to be sending to Bob, so swap recvonly to
+ * sendonly.
+ */
+ if (ast_stream_get_state(stream) == AST_STREAM_STATE_RECVONLY) {
+ ast_stream_set_state(stream, AST_STREAM_STATE_SENDONLY);
+ } else if (ast_stream_get_state(stream) == AST_STREAM_STATE_SENDONLY) {
+ ast_stream_set_state(stream, AST_STREAM_STATE_RECVONLY);
+ }
+ }
+
return new_topology;
}
+static int simple_bridge_join(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel)
+{
+ struct ast_stream_topology *req_top;
+ struct ast_stream_topology *existing_top;
+ struct ast_stream_topology *new_top;
+ struct ast_channel *c0 = AST_LIST_FIRST(&bridge->channels)->chan;
+ struct ast_channel *c1 = AST_LIST_LAST(&bridge->channels)->chan;
+
+ /*
+ * If this is the first channel we can't make it compatible...
+ * unless we make it compatible with itself. O.o
+ */
+ if (c0 == c1) {
+ return 0;
+ }
+
+ if (ast_channel_make_compatible(c0, c1)) {
+ return -1;
+ }
+
+ /* When both channels are joined we want to try to improve the experience by
+ * raising the number of streams so they match.
+ */
+ ast_channel_lock_both(c0, c1);
+ req_top = ast_channel_get_stream_topology(c0);
+ existing_top = ast_channel_get_stream_topology(c1);
+ if (ast_stream_topology_get_count(req_top) < ast_stream_topology_get_count(existing_top)) {
+ SWAP(req_top, existing_top);
+ SWAP(c0, c1);
+ }
+ new_top = simple_bridge_request_stream_topology_update(existing_top, req_top);
+ ast_channel_unlock(c0);
+ ast_channel_unlock(c1);
+
+ if (!new_top) {
+ /* Failure. We'll just have to live with the current topology. */
+ return 0;
+ }
+
+ ast_channel_request_stream_topology_change(c1, new_top, &simple_bridge);
+ ast_stream_topology_free(new_top);
+
+ return 0;
+}
+
+static int simple_bridge_write(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel, struct ast_frame *frame)
+{
+ const struct ast_control_t38_parameters *t38_parameters;
+ int defer = 0;
+
+ if (!ast_bridge_queue_everyone_else(bridge, bridge_channel, frame)) {
+ /* This frame was successfully queued so no need to defer */
+ return 0;
+ }
+
+ /* Depending on the frame defer it so when the next channel joins it receives it */
+ switch (frame->frametype) {
+ case AST_FRAME_CONTROL:
+ switch (frame->subclass.integer) {
+ case AST_CONTROL_T38_PARAMETERS:
+ t38_parameters = frame->data.ptr;
+ switch (t38_parameters->request_response) {
+ case AST_T38_REQUEST_NEGOTIATE:
+ defer = -1;
+ break;
+ default:
+ break;
+ }
+ break;
+ default:
+ break;
+ }
+ break;
+ default:
+ break;
+ }
+
+ return defer;
+}
+
static void simple_bridge_stream_topology_changed(struct ast_bridge *bridge,
struct ast_bridge_channel *bridge_channel)
{
- struct ast_channel *req_chan;
- struct ast_channel *existing_chan;
+ struct ast_channel *c0 = bridge_channel->chan;
+ struct ast_channel *c1 = AST_LIST_FIRST(&bridge->channels)->chan;
struct ast_stream_topology *req_top;
struct ast_stream_topology *existing_top;
struct ast_stream_topology *new_top;
- if (bridge_channel) {
- ast_bridge_channel_stream_map(bridge_channel);
+ ast_bridge_channel_stream_map(bridge_channel);
- if (ast_channel_get_stream_topology_change_source(bridge_channel->chan)
- == &simple_bridge) {
- return;
- }
- }
-
- req_chan = AST_LIST_FIRST(&bridge->channels)->chan;
- existing_chan = AST_LIST_LAST(&bridge->channels)->chan;
- if (req_chan == existing_chan) {
- /* Wait until both channels are in the bridge to align topologies. */
+ if (ast_channel_get_stream_topology_change_source(bridge_channel->chan)
+ == &simple_bridge) {
return;
}
- /* Align topologies according to size or first channel to join */
- ast_channel_lock_both(req_chan, existing_chan);
- req_top = ast_channel_get_stream_topology(req_chan);
- existing_top = ast_channel_get_stream_topology(existing_chan);
- if (ast_stream_topology_get_count(req_top) < ast_stream_topology_get_count(existing_top)) {
- SWAP(req_top, existing_top);
- SWAP(req_chan, existing_chan);
+ if (c0 == c1) {
+ c1 = AST_LIST_LAST(&bridge->channels)->chan;
}
+
+ if (c0 == c1) {
+ return;
+ }
+
+ /* If a party renegotiates we want to renegotiate their counterpart to a matching
+ * topology.
+ */
+ ast_channel_lock_both(c0, c1);
+ req_top = ast_channel_get_stream_topology(c0);
+ existing_top = ast_channel_get_stream_topology(c1);
new_top = simple_bridge_request_stream_topology_update(existing_top, req_top);
- ast_channel_unlock(req_chan);
- ast_channel_unlock(existing_chan);
+ ast_channel_unlock(c0);
+ ast_channel_unlock(c1);
if (!new_top) {
/* Failure. We'll just have to live with the current topology. */
return;
}
- ast_channel_request_stream_topology_change(existing_chan, new_top, &simple_bridge);
+ ast_channel_request_stream_topology_change(c1, new_top, &simple_bridge);
ast_stream_topology_free(new_top);
}
diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c
index c24fa7a..e1c6734 100644
--- a/bridges/bridge_softmix.c
+++ b/bridges/bridge_softmix.c
@@ -462,12 +462,12 @@
*
* \param stream The stream to test
* \param source_channel_name The name of a source video channel to match
- * \param source_stream_name The name of the source video stream to match
+ * \param source_channel_stream_position The position of the video on the source channel
* \retval 1 The stream is a video destination stream
* \retval 0 The stream is not a video destination stream
*/
static int is_video_dest(const struct ast_stream *stream, const char *source_channel_name,
- const char *source_stream_name)
+ int source_channel_stream_position)
{
char *dest_video_name;
size_t dest_video_name_len;
@@ -480,17 +480,17 @@
dest_video_name_len = SOFTBRIDGE_VIDEO_DEST_LEN + 1;
if (!ast_strlen_zero(source_channel_name)) {
dest_video_name_len += strlen(source_channel_name) + 1;
- if (!ast_strlen_zero(source_stream_name)) {
- dest_video_name_len += strlen(source_stream_name) + 1;
+ if (source_channel_stream_position != -1) {
+ dest_video_name_len += 11;
}
dest_video_name = ast_alloca(dest_video_name_len);
- if (!ast_strlen_zero(source_stream_name)) {
- /* We are looking for an exact stream name */
- snprintf(dest_video_name, dest_video_name_len, "%s%c%s%c%s",
+ if (source_channel_stream_position != -1) {
+ /* We are looking for an exact stream position */
+ snprintf(dest_video_name, dest_video_name_len, "%s%c%s%c%d",
SOFTBRIDGE_VIDEO_DEST_PREFIX, SOFTBRIDGE_VIDEO_DEST_SEPARATOR,
source_channel_name, SOFTBRIDGE_VIDEO_DEST_SEPARATOR,
- source_stream_name);
+ source_channel_stream_position);
return !strcmp(ast_stream_get_name(stream), dest_video_name);
}
snprintf(dest_video_name, dest_video_name_len, "%s%c%s",
@@ -503,46 +503,62 @@
return !strncmp(ast_stream_get_name(stream), dest_video_name, dest_video_name_len - 1);
}
+static int append_source_stream(struct ast_stream_topology *dest,
+ const char *channel_name, const char *sdp_label,
+ struct ast_stream *stream, int index)
+{
+ char *stream_clone_name = NULL;
+ struct ast_stream *stream_clone;
+
+ /* We use the stream topology index for the stream to uniquely identify and recognize it.
+ * This is guaranteed to remain the same across renegotiation of the source channel and
+ * ensures that the stream name is unique.
+ */
+ if (ast_asprintf(&stream_clone_name, "%s%c%s%c%d", SOFTBRIDGE_VIDEO_DEST_PREFIX,
+ SOFTBRIDGE_VIDEO_DEST_SEPARATOR, channel_name, SOFTBRIDGE_VIDEO_DEST_SEPARATOR,
+ index) < 0) {
+ return -1;
+ }
+
+ stream_clone = ast_stream_clone(stream, stream_clone_name);
+ ast_free(stream_clone_name);
+ if (!stream_clone) {
+ return -1;
+ }
+
+ /* Sends an "a:label" attribute in the SDP for participant event correlation */
+ if (!ast_strlen_zero(sdp_label)) {
+ ast_stream_set_metadata(stream_clone, "SDP:LABEL", sdp_label);
+ }
+
+ /* We will be sending them a stream and not expecting anything in return */
+ ast_stream_set_state(stream_clone, AST_STREAM_STATE_SENDONLY);
+
+ if (ast_stream_topology_append_stream(dest, stream_clone) < 0) {
+ ast_stream_free(stream_clone);
+ return -1;
+ }
+
+ return 0;
+}
+
+
static int append_source_streams(struct ast_stream_topology *dest,
const char *channel_name, const char *sdp_label,
const struct ast_stream_topology *source)
{
int i;
- const char *stream_identify;
for (i = 0; i < ast_stream_topology_get_count(source); ++i) {
struct ast_stream *stream;
- struct ast_stream *stream_clone;
- char *stream_clone_name = NULL;
stream = ast_stream_topology_get_stream(source, i);
+
if (!is_video_source(stream)) {
continue;
}
- stream_identify = ast_stream_get_metadata(stream, "MSID:LABEL");
- if (!stream_identify) {
- stream_identify = ast_stream_get_name(stream);
- }
-
- if (ast_asprintf(&stream_clone_name, "%s_%s_%s", SOFTBRIDGE_VIDEO_DEST_PREFIX,
- channel_name, stream_identify) < 0) {
- return -1;
- }
-
- stream_clone = ast_stream_clone(stream, stream_clone_name);
- ast_free(stream_clone_name);
- if (!stream_clone) {
- return -1;
- }
-
- /* Sends an "a:label" attribute in the SDP for participant event correlation */
- if (!ast_strlen_zero(sdp_label)) {
- ast_stream_set_metadata(stream_clone, "SDP:LABEL", sdp_label);
- }
-
- if (ast_stream_topology_append_stream(dest, stream_clone) < 0) {
- ast_stream_free(stream_clone);
+ if (append_source_stream(dest, channel_name, sdp_label, stream, i)) {
return -1;
}
}
@@ -752,7 +768,7 @@
stream = ast_stream_topology_get_stream(topology, i);
- if (is_video_dest(stream, channel_name, NULL)) {
+ if (is_video_dest(stream, channel_name, -1)) {
ast_stream_set_state(stream, AST_STREAM_STATE_REMOVED);
stream_removed = 1;
}
@@ -2135,13 +2151,13 @@
/*!
* \brief Map a source stream to all of its destination streams.
*
- * \param source_stream_name Name of the source stream
* \param source_channel_name Name of channel where the source stream originates
* \param bridge_stream_position The slot in the bridge where source video will come from
* \param participants The bridge_channels in the bridge
+ * \param source_channel_stream_position The position of the stream on the source channel
*/
-static void map_source_to_destinations(const char *source_stream_name, const char *source_channel_name,
- size_t bridge_stream_position, struct ast_bridge_channels_list *participants)
+static void map_source_to_destinations(const char *source_channel_name,
+ size_t bridge_stream_position, struct ast_bridge_channels_list *participants, int source_channel_stream_position)
{
struct ast_bridge_channel *participant;
@@ -2161,7 +2177,7 @@
struct ast_stream *stream;
stream = ast_stream_topology_get_stream(topology, i);
- if (is_video_dest(stream, source_channel_name, source_stream_name)) {
+ if (is_video_dest(stream, source_channel_name, source_channel_stream_position)) {
struct softmix_channel *sc = participant->tech_pvt;
AST_VECTOR_REPLACE(&participant->stream_map.to_channel, bridge_stream_position, i);
@@ -2228,6 +2244,137 @@
}
}
+static void softmix_bridge_stream_sources_update(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel,
+ struct softmix_channel *sc)
+{
+ int index;
+ struct ast_stream_topology *old_topology = sc->topology;
+ struct ast_stream_topology *new_topology = ast_channel_get_stream_topology(bridge_channel->chan);
+ int removed_streams[MAX(ast_stream_topology_get_count(sc->topology), ast_stream_topology_get_count(new_topology))];
+ size_t removed_streams_count = 0;
+ struct ast_stream_topology *added_streams;
+ struct ast_bridge_channels_list *participants = &bridge->channels;
+ struct ast_bridge_channel *participant;
+
+ added_streams = ast_stream_topology_alloc();
+ if (!added_streams) {
+ return;
+ }
+
+ /* We go through the old topology comparing it to the new topology to determine what streams
+ * changed state. A state transition can result in the stream being considered a new source
+ * (for example it was removed and is now present) or being removed (a stream became inactive).
+ * Added streams are copied into a topology and added to each other participant while for
+ * removed streams we merely store their position and mark them as removed later.
+ */
+ for (index = 0; index < ast_stream_topology_get_count(sc->topology) && index < ast_stream_topology_get_count(new_topology); ++index) {
+ struct ast_stream *old_stream = ast_stream_topology_get_stream(sc->topology, index);
+ struct ast_stream *new_stream = ast_stream_topology_get_stream(new_topology, index);
+
+ /* Ignore all streams that don't carry video and streams that are strictly outgoing destination streams */
+ if ((ast_stream_get_type(old_stream) != AST_MEDIA_TYPE_VIDEO && ast_stream_get_type(new_stream) != AST_MEDIA_TYPE_VIDEO) ||
+ !strncmp(ast_stream_get_name(old_stream), SOFTBRIDGE_VIDEO_DEST_PREFIX,
+ SOFTBRIDGE_VIDEO_DEST_LEN)) {
+ continue;
+ }
+
+ if (ast_stream_get_type(old_stream) == AST_MEDIA_TYPE_VIDEO && ast_stream_get_type(new_stream) != AST_MEDIA_TYPE_VIDEO) {
+ /* If a stream renegotiates from video to non-video then we need to remove it as a source */
+ removed_streams[removed_streams_count++] = index;
+ } else if (ast_stream_get_type(old_stream) != AST_MEDIA_TYPE_VIDEO && ast_stream_get_type(new_stream) == AST_MEDIA_TYPE_VIDEO) {
+ if (ast_stream_get_state(new_stream) != AST_STREAM_STATE_REMOVED) {
+ /* If a stream renegotiates from non-video to video in a non-removed state we need to add it as a source */
+ if (append_source_stream(added_streams, ast_channel_name(bridge_channel->chan),
+ bridge->softmix.send_sdp_label ? ast_channel_uniqueid(bridge_channel->chan) : NULL,
+ new_stream, index)) {
+ goto cleanup;
+ }
+ }
+ } else if (ast_stream_get_state(old_stream) != AST_STREAM_STATE_REMOVED &&
+ ast_stream_get_state(new_stream) != AST_STREAM_STATE_SENDRECV && ast_stream_get_state(new_stream) != AST_STREAM_STATE_RECVONLY) {
+ /* If a stream renegotiates and is removed then we remove it */
+ removed_streams[removed_streams_count++] = index;
+ } else if (ast_stream_get_state(old_stream) == AST_STREAM_STATE_REMOVED &&
+ ast_stream_get_state(new_stream) != AST_STREAM_STATE_INACTIVE && ast_stream_get_state(new_stream) != AST_STREAM_STATE_SENDONLY &&
+ ast_stream_get_state(new_stream) != AST_STREAM_STATE_REMOVED) {
+ /* If a stream renegotiates and is added then we add it */
+ if (append_source_stream(added_streams, ast_channel_name(bridge_channel->chan),
+ bridge->softmix.send_sdp_label ? ast_channel_uniqueid(bridge_channel->chan) : NULL,
+ new_stream, index)) {
+ goto cleanup;
+ }
+ }
+ }
+
+ /* Any newly added streams that did not take the position of a removed stream
+ * will be present at the end of the new topology. Since streams are never
+ * removed from the topology but merely marked as removed we can pick up where we
+ * left off when comparing the old and new topologies.
+ */
+ for (; index < ast_stream_topology_get_count(new_topology); ++index) {
+ struct ast_stream *stream = ast_stream_topology_get_stream(new_topology, index);
+
+ if (!is_video_source(stream)) {
+ continue;
+ }
+
+ if (append_source_stream(added_streams, ast_channel_name(bridge_channel->chan),
+ bridge->softmix.send_sdp_label ? ast_channel_uniqueid(bridge_channel->chan) : NULL,
+ stream, index)) {
+ goto cleanup;
+ }
+ }
+
+ /* We always update the stored topology if we can to reflect what is currently negotiated */
+ sc->topology = ast_stream_topology_clone(new_topology);
+ if (!sc->topology) {
+ sc->topology = old_topology;
+ } else {
+ ast_stream_topology_free(old_topology);
+ }
+
+ /* If there are no removed sources and no added sources we don't need to renegotiate the
+ * other participants.
+ */
+ if (!removed_streams_count && !ast_stream_topology_get_count(added_streams)) {
+ goto cleanup;
+ }
+
+ /* Go through each participant adding in the new streams and removing the old ones */
+ AST_LIST_TRAVERSE(participants, participant, entry) {
+ if (participant == bridge_channel) {
+ continue;
+ }
+
+ sc = participant->tech_pvt;
+
+ /* We add in all the new streams first so that they do not take the place
+ * of any of our removed streams, allowing the remote side to reset the state
+ * for each removed stream. */
+ if (append_all_streams(sc->topology, added_streams)) {
+ goto cleanup;
+ }
+
+ /* Then we go through and remove any ones that were removed */
+ for (index = 0; removed_streams_count && index < ast_stream_topology_get_count(sc->topology); ++index) {
+ struct ast_stream *stream = ast_stream_topology_get_stream(sc->topology, index);
+ int removed_stream;
+
+ for (removed_stream = 0; removed_stream < removed_streams_count; ++removed_stream) {
+ if (is_video_dest(stream, ast_channel_name(bridge_channel->chan), removed_streams[removed_stream])) {
+ ast_stream_set_state(stream, AST_STREAM_STATE_REMOVED);
+ }
+ }
+ }
+
+ ast_channel_request_stream_topology_change(participant->chan, sc->topology, NULL);
+ }
+
+
+cleanup:
+ ast_stream_topology_free(added_streams);
+}
+
/*!
* \brief stream_topology_changed callback
*
@@ -2241,7 +2388,7 @@
static void softmix_bridge_stream_topology_changed(struct ast_bridge *bridge, struct ast_bridge_channel *bridge_channel)
{
struct softmix_bridge_data *softmix_data = bridge->tech_pvt;
- struct softmix_channel *sc;
+ struct softmix_channel *sc = bridge_channel->tech_pvt;
struct ast_bridge_channel *participant;
struct ast_vector_int media_types;
int nths[AST_MEDIA_TYPE_END] = {0};
@@ -2258,6 +2405,10 @@
break;
}
+ ast_channel_lock(bridge_channel->chan);
+ softmix_bridge_stream_sources_update(bridge, bridge_channel, sc);
+ ast_channel_unlock(bridge_channel->chan);
+
AST_VECTOR_INIT(&media_types, AST_MEDIA_TYPE_END);
/* The bridge stream identifiers may change, so reset the mapping for them.
@@ -2307,7 +2458,6 @@
for (i = 0; i < ast_stream_topology_get_count(topology); ++i) {
struct ast_stream *stream = ast_stream_topology_get_stream(topology, i);
- const char *stream_identify;
if (is_video_source(stream)) {
AST_VECTOR_APPEND(&media_types, AST_MEDIA_TYPE_VIDEO);
@@ -2325,12 +2475,8 @@
ast_channel_unlock(participant->chan);
ast_bridge_channel_unlock(participant);
- stream_identify = ast_stream_get_metadata(stream, "MSID:LABEL");
- if (!stream_identify) {
- stream_identify = ast_stream_get_name(stream);
- }
- map_source_to_destinations(stream_identify, ast_channel_name(participant->chan),
- AST_VECTOR_SIZE(&media_types) - 1, &bridge->channels);
+ map_source_to_destinations(ast_channel_name(participant->chan),
+ AST_VECTOR_SIZE(&media_types) - 1, &bridge->channels, i);
ast_bridge_channel_lock(participant);
ast_channel_lock(participant->chan);
} else if (ast_stream_get_type(stream) == AST_MEDIA_TYPE_VIDEO) {
@@ -2495,10 +2641,10 @@
{ "alice_video", "vp8", AST_MEDIA_TYPE_VIDEO, },
};
static const struct stream_parameters alice_dest_stream = {
- "softbridge_dest_PJSIP/Bob-00000001_bob_video", "h264,vp8", AST_MEDIA_TYPE_VIDEO,
+ "softbridge_dest_PJSIP/Bob-00000001_1", "h264,vp8", AST_MEDIA_TYPE_VIDEO,
};
static const struct stream_parameters bob_dest_stream = {
- "softbridge_dest_PJSIP/Alice-00000000_alice_video", "vp8", AST_MEDIA_TYPE_VIDEO,
+ "softbridge_dest_PJSIP/Alice-00000000_1", "vp8", AST_MEDIA_TYPE_VIDEO,
};
struct ast_stream_topology *topology_alice = NULL;
struct ast_stream_topology *topology_bob = NULL;
@@ -2645,7 +2791,7 @@
goto end;
}
- if (is_video_dest(actual, removal_results[i].channel_name, NULL) &&
+ if (is_video_dest(actual, removal_results[i].channel_name, -1) &&
ast_stream_get_state(actual) != AST_STREAM_STATE_REMOVED) {
ast_test_status_update(test, "Removed stream %s does not have a state of removed\n", ast_stream_get_name(actual));
goto end;
diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h
index a0b7728..1013003 100644
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -206,6 +206,12 @@
struct ast_stream_topology;
+/*!
+ * \brief Set as the change source reason when a channel stream topology has
+ * been changed externally as a result of the remote side renegotiating.
+ */
+static const char ast_stream_topology_changed_external[] = "external";
+
/*! \todo Add an explanation of an Asterisk generator
*/
struct ast_generator {
@@ -5018,6 +5024,20 @@
int ast_channel_stream_topology_changed(struct ast_channel *chan, struct ast_stream_topology *topology);
/*!
+ * \brief Provide notice from a channel that the topology has changed on it as a result
+ * of the remote party renegotiating.
+ *
+ * \param chan The channel to provide notice from
+ *
+ * \retval 0 success
+ * \retval -1 failure
+ *
+ * \note This interface is provided for channels to provide notice that a topology change
+ * has occurred as a result of a remote party renegotiating the stream topology.
+ */
+int ast_channel_stream_topology_changed_externally(struct ast_channel *chan);
+
+/*!
* \brief Retrieve the source that initiated the last stream topology change
*
* \param chan The channel
diff --git a/main/channel.c b/main/channel.c
index 4b77390..63ce64e 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -11051,6 +11051,25 @@
return ast_channel_tech(chan)->indicate(chan, AST_CONTROL_STREAM_TOPOLOGY_CHANGED, topology, sizeof(topology));
}
+int ast_channel_stream_topology_changed_externally(struct ast_channel *chan)
+{
+ int res;
+ struct ast_frame f = { AST_FRAME_CONTROL, .subclass.integer = AST_CONTROL_STREAM_TOPOLOGY_CHANGED };
+
+ ast_assert(chan != NULL);
+
+ if (!ast_channel_is_multistream(chan)) {
+ return -1;
+ }
+
+ ast_channel_lock(chan);
+ ast_channel_internal_set_stream_topology_change_source(chan, (void *)&ast_stream_topology_changed_external);
+ res = ast_queue_frame(chan, &f);
+ ast_channel_unlock(chan);
+
+ return res;
+}
+
void ast_channel_set_flag(struct ast_channel *chan, unsigned int flag)
{
ast_channel_lock(chan);
diff --git a/main/stream.c b/main/stream.c
index 47415bf..626fa3a 100644
--- a/main/stream.c
+++ b/main/stream.c
@@ -96,8 +96,9 @@
struct ast_stream *ast_stream_alloc(const char *name, enum ast_media_type type)
{
struct ast_stream *stream;
+ size_t name_len = MAX(strlen(S_OR(name, "")), 7); /* Ensure there is enough room for 'removed' */
- stream = ast_calloc(1, sizeof(*stream) + strlen(S_OR(name, "")) + 1);
+ stream = ast_calloc(1, sizeof(*stream) + name_len + 1);
if (!stream) {
return NULL;
}
@@ -113,16 +114,16 @@
struct ast_stream *ast_stream_clone(const struct ast_stream *stream, const char *name)
{
struct ast_stream *new_stream;
- size_t stream_size;
const char *stream_name;
+ size_t name_len;
if (!stream) {
return NULL;
}
stream_name = name ?: stream->name;
- stream_size = sizeof(*stream) + strlen(stream_name) + 1;
- new_stream = ast_calloc(1, stream_size);
+ name_len = MAX(strlen(stream_name), 7); /* Ensure there is enough room for 'removed' */
+ new_stream = ast_calloc(1, sizeof(*stream) + name_len + 1);
if (!new_stream) {
return NULL;
}
@@ -205,6 +206,19 @@
ast_assert(stream != NULL);
stream->state = state;
+
+ /* When a stream is set to removed that means that any previous data for it
+ * is no longer valid. We therefore change its name to removed and remove
+ * any old metadata associated with it.
+ */
+ if (state == AST_STREAM_STATE_REMOVED) {
+ strcpy(stream->name, "removed");
+ ast_variables_destroy(stream->metadata);
+ stream->metadata = NULL;
+ if (stream->formats) {
+ ast_format_cap_remove_by_type(stream->formats, AST_MEDIA_TYPE_UNKNOWN);
+ }
+ }
}
const char *ast_stream_state2str(enum ast_stream_state state)
diff --git a/res/res_pjsip_sdp_rtp.c b/res/res_pjsip_sdp_rtp.c
index 8d9cece..5906211 100644
--- a/res/res_pjsip_sdp_rtp.c
+++ b/res/res_pjsip_sdp_rtp.c
@@ -1547,6 +1547,8 @@
static const pj_str_t STR_IP6 = { "IP6", 3};
static const pj_str_t STR_SENDRECV = { "sendrecv", 8 };
static const pj_str_t STR_SENDONLY = { "sendonly", 8 };
+ static const pj_str_t STR_RECVONLY = { "recvonly", 8 };
+ static const pj_str_t STR_INACTIVE = { "inactive", 8 };
pjmedia_sdp_media *media;
const char *hostip = NULL;
struct ast_sockaddr addr;
@@ -1813,7 +1815,15 @@
/* Add the sendrecv attribute - we purposely don't keep track because pjmedia-sdp will automatically change our offer for us */
attr = PJ_POOL_ZALLOC_T(pool, pjmedia_sdp_attr);
- attr->name = !session_media->locally_held ? STR_SENDRECV : STR_SENDONLY;
+ if (session_media->locally_held || ast_stream_get_state(stream) == AST_STREAM_STATE_SENDONLY) {
+ attr->name = STR_SENDONLY; /* Send sendonly to initate a local hold */
+ } else if (ast_stream_get_state(stream) == AST_STREAM_STATE_RECVONLY) {
+ attr->name = STR_RECVONLY; /* Stream has requested recvonly */
+ } else if (ast_stream_get_state(stream) == AST_STREAM_STATE_INACTIVE) {
+ attr->name = STR_INACTIVE; /* Stream has requested inactive */
+ } else {
+ attr->name = STR_SENDRECV; /* No hold in either direction */
+ }
media->attr[media->attr_count++] = attr;
/* If we've got rtcp-mux enabled, add it unless we received an offer without it */
diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index 1371909..504d6f1 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -945,7 +945,7 @@
{
int i;
struct ast_stream_topology *topology;
- unsigned int changed = 0;
+ unsigned int changed = 0; /* 0 = unchanged, 1 = new source, 2 = new topology */
if (!session->pending_media_state->topology) {
if (session->active_media_state->topology) {
@@ -1057,6 +1057,14 @@
topology = ast_stream_topology_clone(session->pending_media_state->topology);
if (topology) {
ast_channel_set_stream_topology(session->channel, topology);
+ /* If this is a remotely done renegotiation that has changed the stream topology notify what is
+ * currently handling this channel.
+ */
+ if (pjmedia_sdp_neg_was_answer_remote(session->inv_session->neg) == PJ_FALSE &&
+ session->active_media_state && session->active_media_state->topology &&
+ !ast_stream_topology_equal(session->active_media_state->topology, topology)) {
+ changed = 2;
+ }
}
/* Remove all current file descriptors from the channel */
@@ -1079,10 +1087,12 @@
ast_channel_unlock(session->channel);
- if (changed) {
+ if (changed == 1) {
struct ast_frame f = { AST_FRAME_CONTROL, .subclass.integer = AST_CONTROL_STREAM_TOPOLOGY_SOURCE_CHANGED };
ast_queue_frame(session->channel, &f);
+ } else if (changed == 2) {
+ ast_channel_stream_topology_changed_externally(session->channel);
} else {
ast_queue_frame(session->channel, &ast_null_frame);
}
@@ -1875,6 +1885,7 @@
enum ast_media_type type;
struct ast_sip_session_media *session_media = NULL;
enum ast_sip_session_sdp_stream_defer res;
+ pjmedia_sdp_media *remote_stream = sdp->media[i];
/* We need a null-terminated version of the media string */
ast_copy_pj_str(media, &sdp->media[i]->desc.media, sizeof(media));
@@ -1903,6 +1914,25 @@
return -1;
}
+ /* For backwards compatibility with the core default streams are always sendrecv */
+ if (!ast_sip_session_is_pending_stream_default(session, stream)) {
+ if (pjmedia_sdp_media_find_attr2(remote_stream, "sendonly", NULL)) {
+ /* Stream state reflects our state of a stream, so in the case of
+ * sendonly and recvonly we store the opposite since that is what ours
+ * is.
+ */
+ ast_stream_set_state(stream, AST_STREAM_STATE_RECVONLY);
+ } else if (pjmedia_sdp_media_find_attr2(remote_stream, "recvonly", NULL)) {
+ ast_stream_set_state(stream, AST_STREAM_STATE_SENDONLY);
+ } else if (pjmedia_sdp_media_find_attr2(remote_stream, "inactive", NULL)) {
+ ast_stream_set_state(stream, AST_STREAM_STATE_INACTIVE);
+ } else {
+ ast_stream_set_state(stream, AST_STREAM_STATE_SENDRECV);
+ }
+ } else {
+ ast_stream_set_state(stream, AST_STREAM_STATE_SENDRECV);
+ }
+
if (session_media->handler) {
handler = session_media->handler;
if (handler->defer_incoming_sdp_stream) {
--
To view, visit https://gerrit.asterisk.org/c/asterisk/+/13850
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: certified/16.8
Gerrit-Change-Id: I93f41fb41b85646bef71408111c17ccea30cb0c5
Gerrit-Change-Number: 13850
Gerrit-PatchSet: 1
Gerrit-Owner: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200304/26765524/attachment-0001.html>
More information about the asterisk-code-review
mailing list