[Home]

Summary:ASTERISK-21863: [patch] RTP Native Bridge Codec Change Handling - Appears to compare immediately after setting equal
Reporter:David Woolley (davidw)Labels:
Date Opened:2013-06-04 09:54:44Date Closed:2017-12-18 12:00:55.000-0600
Priority:MinorRegression?
Status:Closed/CompleteComponents:Core/RTP
Versions:11.4.0 Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) more_complete_analysys_and_code_extract.txt
Description:ast_bridge_result remote_bridge_loop sets the old codec capabilities equal to the current ones immediately before testing whether they differ on the next iteration of the loop, thus meaning it will fail to act on many changes.
Comments:By: David Woolley (davidw) 2013-06-04 10:10:43.231-0500

The key part of the loop is:

{code}
for ( ; ; ) {
                        glue1->get_codec(c1, cap1);
glue0->get_codec(c0, cap0);
if (........||(!ast_format_cap_identical(cap1, oldcap1))) {
glue0->update_peer(c0, .....,       cap1, 0)
ast_format_cap_copy(oldcap1, cap1);
}
if (.....|| (!ast_format_cap_identical(cap0, oldcap0))) { ....
                        glue1->update_peer(c1, ......, cap0, 0)
ast_format_cap_copy(oldcap0, cap0);
}
Processing with delay
       ast_format_cap_append(oldcap0, cap0);
       ast_format_cap_append(oldcap1, cap1);
}
{code}

I will attach a more complete extract and commentary.

This was detected on an old version, but analysed on the current branch 11 version (r381306).  On the asterisk-dev mailing list, Joshua Colp has indicated qualified agreement that there is a real problem, and suggests removing the late updates of the old capabilities and also noting that the code may need to review the joint capabilities.

I have set this to minor simply because it can't affect many people if it has been in the code so long!
By: David Woolley (davidw) 2013-06-04 10:15:33.959-0500

Slightly edited email containing a more complete annotated version of the relevant parts of the native bridge loop code.
By: David Woolley (davidw) 2013-06-04 10:17:08.009-0500

Joshua Colp's response on the asterisk-dev list:

I *think* you are correct and I believe this stems from the UPDATE_RTP_PEER frame. It is used to indicate explicitly that the remote peer has changed. Removing the code that you mention I think should do it... although I think this really shows another issue which is, the code doesn't handle codecs changing to a non-joint situation so a joint check should also probably be added in.
By: Rusty Newton (rnewton) 2013-06-14 15:29:38.992-0500

Thanks for the report and all of your analysis. Opened.
By: Joshua C. Colp (jcolp) 2017-12-18 12:00:55.708-0600

Bridging support has been rewritten in current supported versions and this issue is no longer applicable to them.