<p>dexter has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18645">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">mgcp_trunk: remove audio_name and audio_payloa<br><br>get rid of deprecated trunk parameters which seem to be leftovers<br>from the old osmo-bsc_mgcp implementation. This is in particular<br>audio_name and audio_payload in struct mgcp_trunk_config which<br>allowed the user to "hardcode" an andio name and payload type<br>via VTY configuration<br><br>The removal of the struct members above also require a change to<br>mgcp_codec.c. The code that is is never actively used and even<br>causes wrong behavior when activated (set the no-transcoding<br>flag in VTY). Since the code is removed also the unit tests<br>also require to be changed to match the new behavior.<br><br>Change-Id: Ia050ec3cd34b410dfe089c41b977ae3d5aed7354<br>Related: OS#2659<br>---<br>M include/osmocom/mgcp/mgcp_trunk.h<br>M src/libosmo-mgcp/mgcp_codec.c<br>M src/libosmo-mgcp/mgcp_vty.c<br>M tests/mgcp/mgcp_test.c<br>4 files changed, 9 insertions(+), 53 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/45/18645/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/mgcp/mgcp_trunk.h b/include/osmocom/mgcp/mgcp_trunk.h</span><br><span>index 71d89ac..386e540 100644</span><br><span>--- a/include/osmocom/mgcp/mgcp_trunk.h</span><br><span>+++ b/include/osmocom/mgcp/mgcp_trunk.h</span><br><span>@@ -9,8 +9,6 @@</span><br><span> int trunk_type;</span><br><span> </span><br><span> char *audio_fmtp_extra;</span><br><span style="color: hsl(0, 100%, 40%);">- char *audio_name;</span><br><span style="color: hsl(0, 100%, 40%);">- int audio_payload;</span><br><span> int audio_send_ptime;</span><br><span> int audio_send_name;</span><br><span> int audio_loop;</span><br><span>diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c</span><br><span>index c251317..9ac5fbb 100644</span><br><span>--- a/src/libosmo-mgcp/mgcp_codec.c</span><br><span>+++ b/src/libosmo-mgcp/mgcp_codec.c</span><br><span>@@ -280,36 +280,16 @@</span><br><span> * Helper function for mgcp_codec_decide() */</span><br><span> static bool is_codec_compatible(const struct mgcp_endpoint *endp, const struct mgcp_rtp_codec *codec)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- char codec_name[64];</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> /* A codec name must be set, if not, this might mean that the codec</span><br><span> * (payload type) that was assigned is unknown to us so we must stop</span><br><span> * here. */</span><br><span> if (!codec->subtype_name)</span><br><span> return false;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* We now extract the codec_name (letters before the /, e.g. "GSM"</span><br><span style="color: hsl(0, 100%, 40%);">- * from the audio name that is stored in the trunk configuration.</span><br><span style="color: hsl(0, 100%, 40%);">- * We do not compare to the full audio_name because we expect that</span><br><span style="color: hsl(0, 100%, 40%);">- * "GSM", "GSM/8000" and "GSM/8000/1" are all compatible when the</span><br><span style="color: hsl(0, 100%, 40%);">- * audio name of the codec is set to "GSM" */</span><br><span style="color: hsl(0, 100%, 40%);">- if (sscanf(endp->trunk->audio_name, "%63[^/]/%*d/%*d", codec_name) < 1)</span><br><span style="color: hsl(0, 100%, 40%);">- return false;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* FIXME: implement meaningful checks to make sure that the given codec</span><br><span style="color: hsl(120, 100%, 40%);">+ * is compatible with the given endpoint */</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* Finally we check if the subtype_name we have generated from the</span><br><span style="color: hsl(0, 100%, 40%);">- * audio_name in the trunc struct patches the codec_name of the</span><br><span style="color: hsl(0, 100%, 40%);">- * given codec */</span><br><span style="color: hsl(0, 100%, 40%);">- if (strcasecmp(codec_name, codec->subtype_name) == 0)</span><br><span style="color: hsl(0, 100%, 40%);">- return true;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /* FIXME: It is questinable that the method to pick a compatible</span><br><span style="color: hsl(0, 100%, 40%);">- * codec can work properly. Since this useses trunk->audio_name, as</span><br><span style="color: hsl(0, 100%, 40%);">- * a reference, which is set to "AMR/8000" permanently.</span><br><span style="color: hsl(0, 100%, 40%);">- * trunk->audio_name must be updated by the first connection that</span><br><span style="color: hsl(0, 100%, 40%);">- * has been made on an endpoint, so that the second connection</span><br><span style="color: hsl(0, 100%, 40%);">- * can make a meaningful decision here */</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- return false;</span><br><span style="color: hsl(120, 100%, 40%);">+ return true;</span><br><span> }</span><br><span> </span><br><span> /*! Decide for one suitable codec</span><br><span>diff --git a/src/libosmo-mgcp/mgcp_vty.c b/src/libosmo-mgcp/mgcp_vty.c</span><br><span>index d332d75..f10f326 100644</span><br><span>--- a/src/libosmo-mgcp/mgcp_vty.c</span><br><span>+++ b/src/libosmo-mgcp/mgcp_vty.c</span><br><span>@@ -114,12 +114,6 @@</span><br><span> VTY_NEWLINE);</span><br><span> } else</span><br><span> vty_out(vty, " no rtp-patch%s", VTY_NEWLINE);</span><br><span style="color: hsl(0, 100%, 40%);">- if (trunk->audio_payload != -1)</span><br><span style="color: hsl(0, 100%, 40%);">- vty_out(vty, " sdp audio-payload number %d%s",</span><br><span style="color: hsl(0, 100%, 40%);">- trunk->audio_payload, VTY_NEWLINE);</span><br><span style="color: hsl(0, 100%, 40%);">- if (trunk->audio_name)</span><br><span style="color: hsl(0, 100%, 40%);">- vty_out(vty, " sdp audio-payload name %s%s",</span><br><span style="color: hsl(0, 100%, 40%);">- trunk->audio_name, VTY_NEWLINE);</span><br><span> if (trunk->audio_fmtp_extra)</span><br><span> vty_out(vty, " sdp audio fmtp-extra %s%s",</span><br><span> trunk->audio_fmtp_extra, VTY_NEWLINE);</span><br><span>@@ -600,13 +594,11 @@</span><br><span> </span><br><span> #define SDP_STR "SDP File related options\n"</span><br><span> #define AUDIO_STR "Audio payload options\n"</span><br><span style="color: hsl(0, 100%, 40%);">-DEFUN(cfg_mgcp_sdp_payload_number,</span><br><span style="color: hsl(120, 100%, 40%);">+DEFUN_DEPRECATED(cfg_mgcp_sdp_payload_number,</span><br><span> cfg_mgcp_sdp_payload_number_cmd,</span><br><span> "sdp audio-payload number <0-255>",</span><br><span> SDP_STR AUDIO_STR "Number\n" "Payload number\n")</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- unsigned int payload = atoi(argv[0]);</span><br><span style="color: hsl(0, 100%, 40%);">- g_cfg->virt_trunk->audio_payload = payload;</span><br><span> return CMD_SUCCESS;</span><br><span> }</span><br><span> </span><br><span>@@ -615,12 +607,11 @@</span><br><span> "sdp audio payload number <0-255>",</span><br><span> SDP_STR AUDIO_STR AUDIO_STR "Number\n" "Payload number\n")</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-DEFUN(cfg_mgcp_sdp_payload_name,</span><br><span style="color: hsl(120, 100%, 40%);">+DEFUN_DEPRECATED(cfg_mgcp_sdp_payload_name,</span><br><span> cfg_mgcp_sdp_payload_name_cmd,</span><br><span> "sdp audio-payload name NAME",</span><br><span> SDP_STR AUDIO_STR "Name\n" "Payload name\n")</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- osmo_talloc_replace_string(g_cfg, &g_cfg->virt_trunk->audio_name, argv[0]);</span><br><span> return CMD_SUCCESS;</span><br><span> }</span><br><span> </span><br><span>@@ -845,10 +836,6 @@</span><br><span> </span><br><span> llist_for_each_entry(trunk, &g_cfg->trunks, entry) {</span><br><span> vty_out(vty, " trunk %d%s", trunk->trunk_nr, VTY_NEWLINE);</span><br><span style="color: hsl(0, 100%, 40%);">- vty_out(vty, " sdp audio-payload number %d%s",</span><br><span style="color: hsl(0, 100%, 40%);">- trunk->audio_payload, VTY_NEWLINE);</span><br><span style="color: hsl(0, 100%, 40%);">- vty_out(vty, " sdp audio-payload name %s%s",</span><br><span style="color: hsl(0, 100%, 40%);">- trunk->audio_name, VTY_NEWLINE);</span><br><span> vty_out(vty, " %ssdp audio-payload send-ptime%s",</span><br><span> trunk->audio_send_ptime ? "" : "no ", VTY_NEWLINE);</span><br><span> vty_out(vty, " %ssdp audio-payload send-name%s",</span><br><span>@@ -909,15 +896,11 @@</span><br><span> return CMD_SUCCESS;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-DEFUN(cfg_trunk_payload_number,</span><br><span style="color: hsl(120, 100%, 40%);">+DEFUN_DEPRECATED(cfg_trunk_payload_number,</span><br><span> cfg_trunk_payload_number_cmd,</span><br><span> "sdp audio-payload number <0-255>",</span><br><span> SDP_STR AUDIO_STR "Number\n" "Payload Number\n")</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct mgcp_trunk *trunk = vty->index;</span><br><span style="color: hsl(0, 100%, 40%);">- unsigned int payload = atoi(argv[0]);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- trunk->audio_payload = payload;</span><br><span> return CMD_SUCCESS;</span><br><span> }</span><br><span> </span><br><span>@@ -925,14 +908,11 @@</span><br><span> "sdp audio payload number <0-255>",</span><br><span> SDP_STR AUDIO_STR AUDIO_STR "Number\n" "Payload Number\n")</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-DEFUN(cfg_trunk_payload_name,</span><br><span style="color: hsl(120, 100%, 40%);">+DEFUN_DEPRECATED(cfg_trunk_payload_name,</span><br><span> cfg_trunk_payload_name_cmd,</span><br><span> "sdp audio-payload name NAME",</span><br><span> SDP_STR AUDIO_STR "Payload\n" "Payload Name\n")</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct mgcp_trunk *trunk = vty->index;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- osmo_talloc_replace_string(g_cfg, &trunk->audio_name, argv[0]);</span><br><span> return CMD_SUCCESS;</span><br><span> }</span><br><span> </span><br><span>diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c</span><br><span>index ed0fda0..d0da18b 100644</span><br><span>--- a/tests/mgcp/mgcp_test.c</span><br><span>+++ b/tests/mgcp/mgcp_test.c</span><br><span>@@ -1458,11 +1458,9 @@</span><br><span> OSMO_ASSERT(conn);</span><br><span> OSMO_ASSERT(conn->end.codec->payload_type == 18);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* Allocate 5@mgw at select GSM.. */</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Allocate 5@mgw and let osmo-mgw pick a codec from the list */</span><br><span> last_endpoint = -1;</span><br><span> inp = create_msg(CRCX_MULT_GSM_EXACT, NULL);</span><br><span style="color: hsl(0, 100%, 40%);">- talloc_free(cfg->virt_trunk->audio_name);</span><br><span style="color: hsl(0, 100%, 40%);">- cfg->virt_trunk->audio_name = "GSM/8000";</span><br><span> cfg->virt_trunk->no_audio_transcoding = 1;</span><br><span> resp = mgcp_handle_message(cfg, inp);</span><br><span> OSMO_ASSERT(get_conn_id_from_response(resp->data, conn_id,</span><br><span>@@ -1474,7 +1472,7 @@</span><br><span> endp = cfg->virt_trunk->endpoints[last_endpoint];</span><br><span> conn = mgcp_conn_get_rtp(endp, conn_id);</span><br><span> OSMO_ASSERT(conn);</span><br><span style="color: hsl(0, 100%, 40%);">- OSMO_ASSERT(conn->end.codec->payload_type == 3);</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_ASSERT(conn->end.codec->payload_type == 0);</span><br><span> </span><br><span> inp = create_msg(MDCX_NAT_DUMMY, conn_id);</span><br><span> last_endpoint = -1;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18645">change 18645</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/+/18645"/><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: Ia050ec3cd34b410dfe089c41b977ae3d5aed7354 </div>
<div style="display:none"> Gerrit-Change-Number: 18645 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>