From 17976d1b4e302b3aa7cc510109a78010d7893fe1 Mon Sep 17 00:00:00 2001
From: Richard Mudgett <rmudgett@digium.com>
Date: Wed, 16 Aug 2017 17:50:18 -0500
Subject: [PATCH] bridge_channel.c: Fix FRACK when mapping frames to the
bridge.
* Add protection checks when mapping streams to the bridge. The channel
and bridge may be in the process of updating the stream mapping when a
media frame comes in so we may not be able to map the frame at the time.
* We need to map the streams to the bridge's stream numbers right before
they are written into the bridge. That way we don't have to keep
locking/unlocking the bridge and we won't have any synchronization
problems before the frames actually go into the bridge.
* Protect the deferred queue with the bridge_channel lock.
ASTERISK-27212
Change-Id: Id6860dd61b594b90c8395f6e2c0150219094c21a
---
main/bridge_channel.c | 71 +++++++++++++++++++++++++++++++++++++------
1 file changed, 61 insertions(+), 10 deletions(-)
diff --git a/main/bridge_channel.c b/main/bridge_channel.c
index c465ec16eb..66292bf921 100644
--- a/main/bridge_channel.c
+++ b/main/bridge_channel.c
@@ -626,11 +626,11 @@ void ast_bridge_channel_kick(struct ast_bridge_channel *bridge_channel, int caus
/*!
* \internal
- * \brief Write an \ref ast_frame onto the bridge channel
+ * \brief Write an \ref ast_frame into the bridge
* \since 12.0.0
*
- * \param bridge_channel Which channel to queue the frame onto.
- * \param frame The frame to write onto the bridge_channel
+ * \param bridge_channel Which channel is queueing the frame.
+ * \param frame The frame to write into the bridge
*
* \retval 0 on success.
* \retval -1 on error.
@@ -638,19 +638,73 @@ void ast_bridge_channel_kick(struct ast_bridge_channel *bridge_channel, int caus
static int bridge_channel_write_frame(struct ast_bridge_channel *bridge_channel, struct ast_frame *frame)
{
const struct ast_control_t38_parameters *t38_parameters;
+ int unmapped_stream_num;
int deferred;
ast_assert(frame->frametype != AST_FRAME_BRIDGE_ACTION_SYNC);
ast_bridge_channel_lock_bridge(bridge_channel);
+ /* Map the frame to the bridge. */
+ if (ast_channel_is_multistream(bridge_channel->chan)) {
+ unmapped_stream_num = frame->stream_num;
+ switch (frame->frametype) {
+ case AST_FRAME_VOICE:
+ case AST_FRAME_VIDEO:
+ case AST_FRAME_TEXT:
+ case AST_FRAME_IMAGE:
+ /* Media frames need to be mapped to an appropriate write stream */
+ if (frame->stream_num < 0) {
+ /* Map to default stream */
+ frame->stream_num = -1;
+ break;
+ }
+ ast_bridge_channel_lock(bridge_channel);
+ if (frame->stream_num < (int)AST_VECTOR_SIZE(&bridge_channel->stream_map.to_bridge)) {
+ frame->stream_num = AST_VECTOR_GET(
+ &bridge_channel->stream_map.to_bridge, frame->stream_num);
+ if (0 <= frame->stream_num) {
+ ast_bridge_channel_unlock(bridge_channel);
+ break;
+ }
+ }
+ ast_bridge_channel_unlock(bridge_channel);
+ ast_bridge_unlock(bridge_channel->bridge);
+ /*
+ * Ignore frame because we don't know how to map the frame
+ * or the bridge is not expecting any media from that
+ * stream.
+ */
+ return 0;
+ case AST_FRAME_DTMF_BEGIN:
+ case AST_FRAME_DTMF_END:
+ /*
+ * XXX It makes sense that DTMF could be on any audio stream.
+ * For now we will only put it on the default audio stream.
+ */
+ default:
+ frame->stream_num = -1;
+ break;
+ }
+ } else {
+ unmapped_stream_num = -1;
+ frame->stream_num = -1;
+ }
+
deferred = bridge_channel->bridge->technology->write(bridge_channel->bridge, bridge_channel, frame);
if (deferred) {
struct ast_frame *dup;
dup = ast_frdup(frame);
if (dup) {
+ /*
+ * We have to unmap the deferred frame so it comes back
+ * in like a new frame.
+ */
+ dup->stream_num = unmapped_stream_num;
+ ast_bridge_channel_lock(bridge_channel);
AST_LIST_INSERT_HEAD(&bridge_channel->deferred_queue, dup, frame_list);
+ ast_bridge_channel_unlock(bridge_channel);
}
}
@@ -760,12 +814,14 @@ void bridge_channel_queue_deferred_frames(struct ast_bridge_channel *bridge_chan
{
struct ast_frame *frame;
+ ast_bridge_channel_lock(bridge_channel);
ast_channel_lock(bridge_channel->chan);
while ((frame = AST_LIST_REMOVE_HEAD(&bridge_channel->deferred_queue, frame_list))) {
ast_queue_frame_head(bridge_channel->chan, frame);
ast_frfree(frame);
}
ast_channel_unlock(bridge_channel->chan);
+ ast_bridge_channel_unlock(bridge_channel);
}
/*!
@@ -2458,13 +2514,8 @@ static void bridge_handle_trip(struct ast_bridge_channel *bridge_channel)
return;
}
- if (ast_channel_is_multistream(bridge_channel->chan) &&
- (frame->frametype == AST_FRAME_IMAGE || frame->frametype == AST_FRAME_TEXT ||
- frame->frametype == AST_FRAME_VIDEO || frame->frametype == AST_FRAME_VOICE)) {
- /* Media frames need to be mapped to an appropriate write stream */
- frame->stream_num = AST_VECTOR_GET(
- &bridge_channel->stream_map.to_bridge, frame->stream_num);
- } else {
+ if (!ast_channel_is_multistream(bridge_channel->chan)) {
+ /* This may not be initialized by non-multistream channel drivers */
frame->stream_num = -1;
}
--
GitLab