[Asterisk-code-review] res/res rtp asterisk: ensure marker bit is correctly set on ... (asterisk[13])

[Richard Mudgett]

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

Change subject: res/res_rtp_asterisk: ensure marker bit is correctly set on ssrc change
......................................................................


Patch Set 2: Code-Review-1

(1 comment)

https://gerrit.asterisk.org/#/c/8948/2/res/res_rtp_asterisk.c
File res/res_rtp_asterisk.c:

https://gerrit.asterisk.org/#/c/8948/2/res/res_rtp_asterisk.c@5244
PS2, Line 5244: 		bridged->ssrc = ssrc;
Changing the ssrc is going to mess up the RTCP sender report.  In addition on v15 it is likely to mess up the bundled ssrc mapping, the SSRC reported in SDP for bundling, and DTLS.

I think to get a similar effect we need a new flag value (FLAG_REQ_LOCAL_BRIDGE_BIT) to also be set by ast_rtp_local_bridge().  We treat this flag similar to the FLAG_NEED_MARKER_BIT only in bridge_p2p_rtp_write().

#define FLAG_NEED_MARKER_BIT            (1 << 3)
...
#define FLAG_REQ_LOCAL_BRIDGE_BIT            (1 << 5)
...

bridge_p2p_rtp_write()
{
...
	/* If the marker bit has been explicitly set turn it on */
	if (ast_test_flag(bridged, FLAG_NEED_MARKER_BIT)) {
		mark = 1;
		ast_clear_flag(bridged, FLAG_NEED_MARKER_BIT);
	}
	/* Set the marker bit for the first local bridged packet which has the first bridged peer's SSRC. */
	if (ast_test_flag(bridged, FLAG_REQ_LOCAL_BRIDGE_BIT)) {
		mark = 1;
		ast_clear_flag(bridged, FLAG_REQ_LOCAL_BRIDGE_BIT);
	}
...
}


ast_rtp_local_bridge()
{
...
	ao2_lock(instance0);
	ast_set_flag(rtp, FLAG_NEED_MARKER_BIT | FLAG_REQ_LOCAL_BRIDGE_BIT);
	ao2_unlock(instance0);

}



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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: comment
Gerrit-Change-Id: I01858bd0235f1e5e629e20de71b422b16f55759b
Gerrit-Change-Number: 8948
Gerrit-PatchSet: 2
Gerrit-Owner: Torrey Searle <tsearle at gmail.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Fri, 18 May 2018 18:08:11 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180518/90e67fac/attachment.html>