[Asterisk-code-review] bridge simple: Improve renegotiation success rate. (asterisk[15])

[Richard Mudgett]

Richard Mudgett has posted comments on this change. ( https://gerrit.asterisk.org/6759 )

Change subject: bridge_simple: Improve renegotiation success rate.
......................................................................


Patch Set 1: Code-Review-1

(3 comments)

https://gerrit.asterisk.org/#/c/6759/1/bridges/bridge_simple.c
File bridges/bridge_simple.c:

https://gerrit.asterisk.org/#/c/6759/1/bridges/bridge_simple.c@141
PS1, Line 141: 	new_topology = ast_stream_topology_clone(requested_topology);
This can fail.


https://gerrit.asterisk.org/#/c/6759/1/bridges/bridge_simple.c@146
PS1, Line 146: 	if (audio_formats) {
Invert the logic to reduce indention and avoid cloning the requested_topology.

if (!audio_formats) {
   ast_channel_request_stream_toology_change(chan, requested_topology, &simple_bridge);
   return;
}

new_topology = ast_stream_topology_clone();
if (!new_topology) {
   Don't bother requesting a change?
   Or just request a change without adding the audio formats?
   return;
}

add the audio_formats as needed.


https://gerrit.asterisk.org/#/c/6759/1/bridges/bridge_simple.c@157
PS1, Line 157: 			/* We go through the formats individually to ensure we don't have
             : 			 * duplicates.
             : 			 */
             : 			for (format_index = 0; format_index < ast_format_cap_count(audio_formats); ++format_index) {
             : 				struct ast_format *format = ast_format_cap_get_format(audio_formats, format_index);
             : 
             : 				if (ast_format_cap_iscompatible_format(ast_stream_get_formats(stream), format) != AST_FORMAT_CMP_NOT_EQUAL) {
             : 					ao2_ref(format, -1);
             : 					continue;
             : 				}
             : 
             : 				ast_format_cap_append(ast_stream_get_formats(stream), format, ast_format_cap_get_framing(audio_formats));
             : 				ao2_ref(format, -1);
             : 			}
Why not use:
ast_format_cap_append_from_cap(ast_stream_get_formats(stream), audio_formats, AST_MEDIA_TYPE_AUDIO);

ast_format_cap_append() checks if the format is already in the cap.



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

Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fc0cc03e8bcfd0be8302f13b9f32d8268977f43
Gerrit-Change-Number: 6759
Gerrit-PatchSet: 1
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Fri, 13 Oct 2017 22:16:08 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171013/67c9addc/attachment-0001.html>