[Asterisk-code-review] SDP: Rework SDP offer/answer model and update capabilities m... (asterisk[master])

[Richard Mudgett]

Hello Jenkins2, 

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

    https://gerrit.asterisk.org/5819

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

Change subject: SDP: Rework SDP offer/answer model and update capabilities merges.
......................................................................

SDP: Rework SDP offer/answer model and update capabilities merges.

The SDP offer/answer model requires an answer to an offer before a new SDP
can be processed.  This allows our local SDP creation to be deferred until
we know that we need to create an offer or an answer SDP.  Once the local
SDP is created it won't change until the SDP negotiation is restarted.

An offer SDP in an initial SIP INVITE can receive more than one answer
SDP.  In this case, we need to merge each answer SDP with our original
offer capabilities to get the currently negotiated capabilities.  To
satisfy this requirement means that we cannot update our proposed
capabilities until the negotiations are restarted.

Local topology updates from ast_sdp_state_update_local_topology() are
merged together until the next offer SDP is created.  These accumulated
updates are then merged with the current negotiated capabilities to create
the new proposed capabilities that the offer SDP is built.

Local topology updates are merged in several passes to attempt to be smart
about how streams from the system are matched with the previously
negotiated stream slots.  To allow for T.38 support when merging, type
matching considers audio and image types to be equivalent.  First streams
are matched by stream name and type.  Then streams are matched by stream
type only.  Any remaining unmatched existing streams are declined.  Any
new active streams are either backfilled into pre-merge declined slots or
appended onto the end of the merged topology.  Any excess new streams
above the maximum supported number of streams are simply discarded.

Remote topology negotiation merges depend if the topology is an offer or
answer.  An offer remote topology negotiation dictates the stream slot
ordering and new streams can be added.  A remote offer can do anything to
the previously negotiated streams except reduce the number of stream
slots.  An answer remote topology negotiation is limited to what our offer
requested.  The answer can only decline streams, pick codecs from the
offered list, or indicate the remote's stream hold state.

I had originally kept the RTP instance if the remote offer SDP changed a
stream type between audio and video since they both use RTP.  However, I
later removed this support in favor of simply creating a new RTP instance
since the stream's purpose has to be changing anyway.  Any RTP packets
from the old stream type might cause mischief for the bridged peer.

* Added ast_sdp_state_restart_negotiations() to restart the SDP
offer/answer negotiations.  We will thus know to create a new local SDP
when it is time to create an offer or answer.

* Removed ast_sdp_state_reset().  Save the current topology before
starting T.38.  To recover from T.38 simply update the local topology to
the saved topology and restart the SDP negotiations to get the offer SDP
renegotiating the previous configuration.

* Allow initial topology for ast_sdp_state_alloc() to be NULL so an
initial remote offer SDP can dictate the streams we start with.  We can
always update the local topology later if it turns out we need to offer
SDP first because the remote chose to defer sending us a SDP.

* Made the ast_sdp_state_alloc() initial topology limit to max_streams,
limit to configured codecs, handle declined streams, and discard
unsupported types.

* Convert struct ast_sdp to ao2 object.  Needed to easily save off a
remote SDP to refer to later for various reasons such as generating
declined m= lines in the local SDP.

* Improve converting remote SDP streams to a topology including stream
state.  A stream state of AST_STREAM_STATE_REMOVED indicates the stream is
declined/dead.

* Improve merging streams to take into account the stream state.

* Added query for remote hold state.

* Added maximum streams allowed SDP config option.

* Added ability to create new streams as needed.  New streams are created
with configured default audio, video, or image codecs depending on stream
type.

* Added global locally_held state along with a per stream local hold
state.  Historically, Asterisk only has a global locally held state
because when the we put the remote on hold we do it for all active
streams.

* Added queries for a rejected offer and current SDP negotiation role.
The rejected query allows the using module to know how to respond to a
failed remote SDP set.  Should the using module respond with a 488 Not
Acceptable Here or 500 Internal Error to the offer SDP?

* Moved sdp_state_capabilities.connection_address to ast_sdp_state.  There
seems no reason to keep it in the sdp_state_capabilities struct since it
was only used by the ast_sdp_state.proposed_capabilities instance.

* Callbacks are now available to allow the using module some customization
of negotiated streams and to complete setting up streams for use.  See the
typedef doxygen for each callback for what is allowable and when they are
called.
    * Added topology answerer modify callback.
    * Added topology pre and post apply callbacks.
    * Added topology offerer modify callback.
    * Added topology offerer configure callback.

* Had to rework the unit tests because I changed how SDP topologies are
merged.  Replaced several unit tests with new negotiation tests.

Change-Id: If07fe6d79fbdce33968a9401d41d908385043a06
---
M include/asterisk/sdp.h
M include/asterisk/sdp_options.h
M include/asterisk/sdp_state.h
M include/asterisk/stream.h
M main/sdp.c
M main/sdp_options.c
M main/sdp_private.h
M main/sdp_state.c
M main/stream.c
M res/res_sdp_translator_pjmedia.c
M tests/test_sdp.c
11 files changed, 3,400 insertions(+), 612 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/19/5819/3
-- 
To view, visit https://gerrit.asterisk.org/5819
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If07fe6d79fbdce33968a9401d41d908385043a06
Gerrit-Change-Number: 5819
Gerrit-PatchSet: 3
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170613/9af026e4/attachment-0001.html>