[Asterisk-code-review] SDP: Ensure SDPs "merge" properly. (asterisk[master])

Mark Michelson asteriskteam at digium.com
Fri Apr 14 09:54:18 CDT 2017


Hello George Joseph, Richard Mudgett, Anonymous Coward #1000019, Joshua Colp,

I'd like you to reexamine a change.  Please visit

    https://gerrit.asterisk.org/5415

to look at the new patch set (#6).

Change subject: SDP: Ensure SDPs "merge" properly.
......................................................................

SDP: Ensure SDPs "merge" properly.

The gist of this work ensures that when a remote SDP is received, it is
merged properly with the local capabilities. The remote SDP is converted
into a stream topology. That topology is then merged with the current
local topology on the SDP state. That new merged topology is then used
to create an SDP. Finally, adjustments are made to RTP instances based
on knowledge gained from the remote SDP.

There are also a battery of tests in this commit that ensure that some
basic SDP merges work as expected.

While this may not sound like a big change, it has the property that it
caused lots of ancillary changes.

* The remote SDP is no longer stored on the SDP state. Biggest reason:
  there's no need for it. The remote SDP is used at the time it is being
  set and nowhere else.

* Some new SDP APIs were added in order to find attributes and convert
  generic SDP attributes into rtpmap structures.

* Writing tests made me realize that retrieving a value from an SDP
  options structure, the SDP options needs to be made const.

* The SDP state machine was essentially gutted by a previous commit.
  Initially, I attempted to reinstate it, but I found that as it had
  been defined, it was not all that useful. What was more useful was
  knowing the role we play in SDP negotiation, so the SDP state machine
  has been transformed into an indicator of role.

* Rather than storing separate local and joint stream state
  capabilities, it makes more sense to keep track of current stream
  state and update it as things change.

Change-Id: I5938c2be3c6f0a003aa88a39a59e0880f8b2df3d
---
M include/asterisk/codec.h
M include/asterisk/sdp.h
M include/asterisk/sdp_options.h
M include/asterisk/sdp_state.h
M main/codec.c
M main/sdp.c
M main/sdp_options.c
M main/sdp_private.h
M main/sdp_state.c
A tests/test_sdp.c
10 files changed, 2,154 insertions(+), 482 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/15/5415/6
-- 
To view, visit https://gerrit.asterisk.org/5415
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5938c2be3c6f0a003aa88a39a59e0880f8b2df3d
Gerrit-PatchSet: 6
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>



More information about the asterisk-code-review mailing list