> we noticed that when the response to a native bridge re-invite changed 
> the codecs, Asterisk didn't seem to react, even though the resulting 
> codecs were incompatible with the other side.

> On looking through the code (rtp.c / rtp_engine.c), it looks like the 
> basic RTP native  bridge loop has a structure like this
......
> To me this seems me to make it very difficult for a codec change to 
> actually be detected, as when you unroll the loop, the unconditional set precedes the test for changes!

Let's be a bit more concrete.  This is an extract from the branches/11 version, as of Thursday, highlighting the issue.  AAAA marks a line that reads the codec for testing for changes, BBBB marks a line that updates the old values of the codecs after acting on the changes, but CCCC seems to mark a line that can be executed without acting on the changes, and which logically precedes an AAAA line in loop execution order:

static enum ast_bridge_result remote_bridge_loop(struct ast_channel *c0, .....
	for (;;) {
....
		if (glue1->get_codec) {                                                <<<<<<<<<<<<<< AAAA
			ast_format_cap_remove_all(cap1);
			glue1->get_codec(c1, cap1);
		}
....
		if (glue0->get_codec) {                                                 <<<<<<<<<<<<<< AAAA
			ast_format_cap_remove_all(cap0);
			glue0->get_codec(c0, cap0);
		}
		if (........||(!ast_format_cap_identical(cap1, oldcap1))) {
			if (glue0->update_peer(c0, .....,       cap1, 0)) {
				......
			}
.....
			ast_format_cap_copy(oldcap1, cap1);    <<<<<<<<<<<<<< BBBB
		}
		if (.....|| (!ast_format_cap_identical(cap0, oldcap0))) { ....
			if (glue1->update_peer(c1, ......, cap0, 0)) {
				.....
			}
....
			ast_format_cap_copy(oldcap0, cap0);    <<<<<<<<<<<<<< BBBB
		}
....
		if (!(who = ast_waitfor_n(cs, 2, &ms))) {
			.....
		}
		fr = ast_read(who);
		if (!fr || ((fr->frametype == AST_FRAME_DTMF_BEGIN || fr->frametype == AST_FRAME_DTMF_END) && ....
			break;
		} else if ((fr->frametype == AST_FRAME_CONTROL) && !(flags & AST_BRIDGE_IGNORE_SIGS)) {
			if ((fr->subclass.integer == AST_CONTROL_HOLD) ||
			    (fr->subclass.integer == AST_CONTROL_UNHOLD) ||
			    (fr->subclass.integer == AST_CONTROL_VIDUPDATE) ||
			    (fr->subclass.integer == AST_CONTROL_SRCUPDATE) ||
			    (fr->subclass.integer == AST_CONTROL_T38_PARAMETERS) ||
				(fr->subclass.integer == AST_CONTROL_UPDATE_RTP_PEER)) {
				if (fr->subclass.integer == AST_CONTROL_HOLD) {
                                                                                 ...... update_peer.......     <<<<< DDDD
				} else if (fr->subclass.integer == AST_CONTROL_UNHOLD ||
                                                                                 ...... update_peer.......     <<<<< DDDD
				}
....
				/* Update codec information */
				if (glue0->get_codec && ast_channel_tech_pvt(c0)) {
					ast_format_cap_remove_all(cap0);
					ast_format_cap_remove_all(oldcap0);
					glue0->get_codec(c0, cap0);                                  <<<<<<< EEEE
					ast_format_cap_append(oldcap0, cap0);         <<<<<<< CCCC

				}
				if (glue1->get_codec && ast_channel_tech_pvt(c1)) {
					ast_format_cap_remove_all(cap1);
					ast_format_cap_remove_all(oldcap1);
					glue1->get_codec(c1, cap1);                                  <<<<<<< EEEE
					ast_format_cap_append(oldcap1, cap1);         <<<<<<< CCCC
				}
.....
			} else if (....
			} else { .....
			}
		} else {
....
	}

Note that whilst the lines marked DDDD do an update, they are only conditionally executed and they are before the read of the codecs at EEEE