[Asterisk-code-review] res sdp translator pjmedia.c: Add TODO notes. (asterisk[master])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/5555 )

Change subject: res_sdp_translator_pjmedia.c: Add TODO notes.
......................................................................


res_sdp_translator_pjmedia.c: Add TODO notes.

Change-Id: If27ca61f79accc882c3376d2e876d2b44aa1347b
---
M res/res_sdp_translator_pjmedia.c
1 file changed, 39 insertions(+), 0 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, but someone else must approve
  Jenkins2: Approved for Submit
  Joshua Colp: Looks good to me, approved



diff --git a/res/res_sdp_translator_pjmedia.c b/res/res_sdp_translator_pjmedia.c
index 3b9c26c..85f246e 100644
--- a/res/res_sdp_translator_pjmedia.c
+++ b/res/res_sdp_translator_pjmedia.c
@@ -37,6 +37,45 @@
 	<support_level>core</support_level>
  ***/
 
+/*
+ * XXX TODO: The memory in the pool is held onto longer than necessary.  It
+ * is kept and grows for the duration of the associated chan_pjsip session.
+ *
+ * The translation API does not need to be so generic.  The users will know
+ * at compile time what the non-Asterisk SDP format they have or need.  They
+ * should simply call the specific translation functions.  However, to make
+ * this a loadable module we need to be able to keep it in memory when a
+ * dependent module is loaded.
+ *
+ * To address both issues I propose this API:
+ *
+ * void ast_sdp_translate_pjmedia_ref(void) - Inc this module's user ref
+ * void ast_sdp_translate_pjmedia_unref(void) - Dec this module's user ref.
+ *    The res_pjsip_session.c:ast_sip_session_alloc() can call the module ref
+ *    and the session's destructor can call the module unref.
+ *
+ * struct ast_sdp *ast_sdp_translate_pjmedia_from(const pjmedia_sdp_session *pjmedia_sdp);
+ *
+ * pjmedia_sdp_session *ast_sdp_translate_pjmedia_to(const struct ast_sdp *sdp, pj_pool_t *pool);
+ *    Passing in a memory pool allows the memory to be obtained from an
+ *    rdata memory pool that will be released when the message processing
+ *    is complete.  This prevents memory from accumulating for the duration
+ *    of a call.
+ *
+ * int ast_sdp_translate_pjmedia_set_remote_sdp(struct ast_sdp_state *sdp_state, const pjmedia_sdp_session *remote);
+ * const pjmedia_sdp_session *ast_sdp_translate_pjmedia_get_local_sdp(struct ast_sdp_state *sdp_state, pj_pool_t *pool);
+ *    These two functions just do the bookkeeping to translate and set or get
+ *    the requested SDP.
+ *
+ *
+ * XXX TODO: This code doesn't handle allocation failures very well.  i.e.,
+ *   It assumes they will never happen.
+ *
+ * XXX TODO: This code uses ast_alloca() inside loops.  Doing so if the number
+ *   of times through the loop is unconstrained will blow the stack.
+ *   See dupa_pj_str() usage.
+ */
+
 static pj_caching_pool sdp_caching_pool;
 
 

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

Gerrit-MessageType: merged
Gerrit-Change-Id: If27ca61f79accc882c3376d2e876d2b44aa1347b
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>