[Asterisk-code-review] Add SDP translator and PJMEDIA implementation. (asterisk[master])

[Mark Michelson]

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

Change subject: Add SDP translator and PJMEDIA implementation.
......................................................................


Patch Set 4:

(2 comments)

https://gerrit.asterisk.org/#/c/4946/4/include/asterisk/sdp_priv.h
File include/asterisk/sdp_priv.h:

PS4, Line 51: AST_VECTOR(ast_sdp_a_line_vector, struct ast_sdp_a_line);
            : 
            : /*!
            :  * \brief An SDP media stream
            :  *
            :  * This contains both the m line, as well as its
            :  * constituent a lines.
            :  */
            : struct ast_sdp_m_line {
            : 	/*! Media type (e.g. "audio" or "video") */
            : 	char *type;
            : 	/*! Port number in m line */
            : 	uint16_t port;
            : 	/*! Number of ports specified in m line */
            : 	uint16_t port_count;
            : 	/*! RTP profile string (e.g. "RTP/AVP") */
            : 	char *profile;
            : 	/*! RTP payloads */
            : 	AST_VECTOR(, char *) payloads;
            : 	/*! Connection information for this media stream */
            : 	struct ast_sdp_c_line c_line;
            : 	/*! The attributes for this media stream */
            : 	struct ast_sdp_a_line_vector a_lines;
            : };
> Why the 2 different mechanisms for vectors?
Because vectors of a_lines appear both within an SDP as a whole, and within media descriptions. Therefore, I created a type name for a vector of a_lines so I could declare the type here and below. It also means that in sdp_repr.c, I can pass an ast_sdp_a_line_vector type to a function if it makes sense to.

The other places where vectors are used are one-offs, so I didn't bother to declare a type name for them.


https://gerrit.asterisk.org/#/c/4946/4/include/asterisk/sdp_translator.h
File include/asterisk/sdp_translator.h:

PS4, Line 82: /*!
            :  * \brief Translate a native SDP to internal Asterisk SDP
            :  *
            :  * \param translator The translator to use when translating
            :  * \param native_sdp The SDP from the channel driver
            :  * \retval NULL FAIL
            :  * \retval Non-NULL The translated SDP
            :  */
            : struct ast_sdp *ast_sdp_translator_to_sdp(struct ast_sdp_translator *translator, void *native_sdp);
> Do sdp's need to be freed or unreffed?
Doubtful. Whenever a consumer of the SDP API requests an SDP, they'll not get an internal representation of the SDP. They'll get a translated SDP to the native format that they require. They'll be responsible for freeing that native SDP, which they should know how to do on their own.

The SDPs that translators create will only be consumed by SDP internals. Those can use the ast_sdp_free() function in sdp_priv.h to free their SDPs.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2ecd3cbebe76756577be9b133e84d2ee356d46b
Gerrit-PatchSet: 4
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-HasComments: Yes