[Asterisk-code-review] bridge_channel: Don't queue unmapped frames. (asterisk[16])

[George Joseph]

George Joseph has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/14503 )

Change subject: bridge_channel: Don't queue unmapped frames.
......................................................................


Patch Set 2: Code-Review-1

(2 comments)

https://gerrit.asterisk.org/c/asterisk/+/14503/2/main/bridge_channel.c 
File main/bridge_channel.c:

https://gerrit.asterisk.org/c/asterisk/+/14503/2/main/bridge_channel.c@1046 
PS2, Line 1046: 	if ((fr->frametype == AST_FRAME_VOICE || fr->frametype == AST_FRAME_VIDEO ||
              : 		fr->frametype == AST_FRAME_TEXT || fr->frametype == AST_FRAME_IMAGE ||
              : 		fr->frametype == AST_FRAME_RTCP) && fr->stream_num > -1) {
              : 		int num = -1;
              : 
              : 		ast_bridge_channel_lock(bridge_channel);
              : 		if (fr->stream_num < (int)AST_VECTOR_SIZE(&bridge_channel->stream_map.to_channel)) {
              : 			num = AST_VECTOR_GET(&bridge_channel->stream_map.to_channel, fr->stream_num);
              : 		}
              : 		ast_bridge_channel_unlock(bridge_channel);
              : 
              : 		if (num == -1) {
              : 			/* We don't have a mapped stream so just discard this frame. */
              : 			return 0;
              : 		}
              : 	}
How about moving this below the lock on line 1868 that way we don't have to do lock/unlock, lock/unlock.


https://gerrit.asterisk.org/c/asterisk/+/14503/2/main/bridge_channel.c@1063 
PS2, Line 1063: 	dup = ast_frdup(fr);
              : 	if (!dup) {
              : 		return -1;
              : 	}
              : 
Could move this down to just before the INSERT_TAIL as an additional optimization since we don't use it until then.  Saves having to allocate then free it in case we hit the "leaving" or "text" conditions.



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

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I17189b9b1dec45fc7e4490e8081d444a25a00bda
Gerrit-Change-Number: 14503
Gerrit-PatchSet: 2
Gerrit-Owner: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Comment-Date: Fri, 05 Jun 2020 13:41:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200605/17edb6ab/attachment-0001.html>