<p><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18913">View Change</a></p><p>5 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18913/1/include/osmocom/mgcp/mgcp_endp.h">File include/osmocom/mgcp/mgcp_endp.h:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18913/1/include/osmocom/mgcp/mgcp_endp.h@44">Patch Set #1, Line 44:</a> <code style="font-family:monospace,monospace">#define OSMO_RTP_MSG_CTX(MSGB) (*(struct osmo_rtp_msg_ctx**)&((MSGB)->dst))</code></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">Accordng to msgb.h:<br>"""<br>     /* Part of which TRX logical channel we were received / transmitted */<br>        /* FIXME: move them into the control buffer */<br>        union {<br>               void *dst; /*!< reference of origin/destination */<br>         struct gsm_bts_trx *trx;<br>      };<br>"""</pre><p style="white-space: pre-wrap; word-wrap: break-word;">So rather use ->cb for this, like we do in libosmo-netif's jibuf.c:<br>#define JIBUF_MSGB_CB(__msgb) ((struct osmo_jibuf_msgb_cb *)&((__msgb)->cb[0]))</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18913/1/src/libosmo-mgcp/mgcp_codec.c">File src/libosmo-mgcp/mgcp_codec.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18913/1/src/libosmo-mgcp/mgcp_codec.c@436">Patch Set #1, Line 436:</a> <code style="font-family:monospace,monospace">const struct mgcp_rtp_codec *mgcp_codec_pt_find_by_subtype_name(struct mgcp_conn_rtp *conn,</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">some documentation here would be welcome.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18913/1/src/libosmo-mgcp/mgcp_network.c">File src/libosmo-mgcp/mgcp_network.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18913/1/src/libosmo-mgcp/mgcp_network.c@1302">Patch Set #1, Line 1302:</a> <code style="font-family:monospace,monospace"> /* Since the msgb remains owned and freed by this function, the msg ctx data struct can just be on the stack and</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">It may be the case now and I'm ok with it, but in the long term I'd argue it makes more sense to transfer ownsership since the msgb is most likely gonna end up in someone else wqueue?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18913/1/src/libosmo-mgcp/mgcp_network.c@1310">Patch Set #1, Line 1310:</a> <code style="font-family:monospace,monospace"> LOG_CONN_RTP(conn_src, LOGL_DEBUG, "msg ctx: %d %p %s\n",</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Do we really want to call osmo_hexdump for ALL rtp packets received? Even if it's not printed...</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18913/1/src/libosmo-mgcp/mgcp_network.c@1330">Patch Set #1, Line 1330:</a> <code style="font-family:monospace,monospace">static int rx_rtp(struct msgb *msg)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Move this function on top of rtp_data_net and then you can drop the forward reference of rx_rtp at the top of the file.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18913">change 18913</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-mgw/+/18913"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-mgw </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I3af40b63bc49f8636d4e7ea2f8f83bb67f6619ee </div>
<div style="display:none"> Gerrit-Change-Number: 18913 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Fri, 19 Jun 2020 10:59:42 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Gerrit-MessageType: comment </div>