[Asterisk-code-review] RFC sdp: Initial SDP creation (asterisk[master])

[Mark Michelson]

Mark Michelson has posted comments on this change. ( https://gerrit.asterisk.org/5119 )

Change subject: RFC sdp: Initial SDP creation
......................................................................


Patch Set 5:

I don't have any more nitpicks or small findings. The code looks really good.

The only thing I'm still not 100% sure on is the decision to give ownership of RTP instances to a stream topology. My main concerns are:
1) In a typical case, there are multiple stream topologies in play at any given time. On the channel, there is the channel stream topology. On the SDP state, there are the local and joint stream topologies. Choosing which of these topologies gets ownership of RTP instances seems arbitrary at best. And if you allow all topologies to share ownership of the RTP instances, this seems like a disaster waiting to happen when a topology changes.
2) I'm just not seeing the advantage that this has over having an array/vector of RTP instances on the SDP state. If RTP instances were going to be retrieved from a stream topology outside the context of the SDP state code, then I could see the advantage. But instead RTP instances are retrieved via an API call. By having a central area of ownership of the RTP instances, it makes it easier to manage.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iefbd877af7f5a4d3c74deead1bff8802661b0d48
Gerrit-PatchSet: 5
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: George Joseph <gjoseph 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-HasComments: No