dexter has submitted this change. (
https://gerrit.osmocom.org/c/osmo-mgw/+/32506 )
(
1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted
one.
)Change subject: mgcp_codec: move mgcp_codec_decide down
......................................................................
mgcp_codec: move mgcp_codec_decide down
In a follow up patch we intend to fix mgcp_codec_decide, since the
fixed version of mgcp_codec_decide will use some functions below its
current position let's move it down before fixing it to make reviewing
the changes easier.
Related: OS#5461
Change-Id: I2f2538ff912eae4d80d3b74b766e18c4da94d6b6
---
M src/libosmo-mgcp/mgcp_codec.c
1 file changed, 96 insertions(+), 81 deletions(-)
Approvals:
pespin: Looks good to me, but someone else must approve
msuraev: Looks good to me, but someone else must approve
laforge: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c
index 003c4c4..7fada78 100644
--- a/src/libosmo-mgcp/mgcp_codec.c
+++ b/src/libosmo-mgcp/mgcp_codec.c
@@ -276,87 +276,6 @@
return -EINVAL;
}
-/* Check if the given codec is applicable on the specified endpoint
- * Helper function for mgcp_codec_decide() */
-static bool is_codec_compatible(const struct mgcp_endpoint *endp, const struct
mgcp_rtp_codec *codec)
-{
- /* A codec name must be set, if not, this might mean that the codec
- * (payload type) that was assigned is unknown to us so we must stop
- * here. */
- if (!strlen(codec->subtype_name))
- return false;
-
- /* FIXME: implement meaningful checks to make sure that the given codec
- * is compatible with the given endpoint */
-
- return true;
-}
-
-/*! Decide for one suitable codec
- * \param[in] conn related rtp-connection.
- * \returns 0 on success, -EINVAL on failure. */
-int mgcp_codec_decide(struct mgcp_conn_rtp *conn)
-{
- struct mgcp_rtp_end *rtp;
- unsigned int i;
- struct mgcp_endpoint *endp;
- bool codec_assigned = false;
-
- endp = conn->conn->endp;
- rtp = &conn->end;
-
- /* This function works on the results the SDP/LCO parser has extracted
- * from the MGCP message. The goal is to select a suitable codec for
- * the given connection. When transcoding is available, the first codec
- * from the codec list is taken without further checking. When
- * transcoding is not available, then the choice must be made more
- * carefully. Each codec in the list is checked until one is found that
- * is rated compatible. The rating is done by the helper function
- * is_codec_compatible(), which does the actual checking. */
- for (i = 0; i < rtp->codecs_assigned; i++) {
- /* When no transcoding is available, avoid codecs that would
- * require transcoding. */
- if (endp->trunk->no_audio_transcoding &&
!is_codec_compatible(endp, &rtp->codecs[i])) {
- LOGP(DLMGCP, LOGL_NOTICE, "transcoding not available,
skipping codec: %d/%s\n",
- rtp->codecs[i].payload_type,
rtp->codecs[i].subtype_name);
- continue;
- }
-
- rtp->codec = &rtp->codecs[i];
- codec_assigned = true;
- break;
- }
-
- /* FIXME: To the reviewes: This is problematic. I do not get why we
- * need to reset the packet_duration_ms depending on the codec
- * selection. I thought it were all 20ms? Is this to address some
- * cornercase. (This piece of code was in the code path before,
- * together with the note: "TODO/XXX: Store this per codec and derive
- * it on use" */
- if (codec_assigned) {
- if (rtp->maximum_packet_time >= 0
- && rtp->maximum_packet_time *
rtp->codec->frame_duration_den >
- rtp->codec->frame_duration_num * 1500)
- rtp->packet_duration_ms = 0;
-
- return 0;
- }
-
- return -EINVAL;
-}
-
-/* Check if the codec has a specific AMR mode (octet-aligned or
bandwith-efficient) set. */
-bool mgcp_codec_amr_align_mode_is_indicated(const struct mgcp_rtp_codec *codec)
-{
- if (codec->param_present == false)
- return false;
- if (!codec->param.amr_octet_aligned_present)
- return false;
- if (strcmp(codec->subtype_name, "AMR") != 0)
- return false;
- return true;
-}
-
/* Return true if octet-aligned is set in the given codec. Default to
octet-aligned=0, i.e. bandwidth-efficient mode.
* See RFC4867 "RTP Payload Format for AMR and AMR-WB" sections "8.1. AMR
Media Type Registration" and "8.2. AMR-WB
* Media Type Registration":
@@ -463,6 +382,87 @@
return codec_convertible;
}
+/* Check if the given codec is applicable on the specified endpoint
+ * Helper function for mgcp_codec_decide() */
+static bool is_codec_compatible(const struct mgcp_endpoint *endp, const struct
mgcp_rtp_codec *codec)
+{
+ /* A codec name must be set, if not, this might mean that the codec
+ * (payload type) that was assigned is unknown to us so we must stop
+ * here. */
+ if (!strlen(codec->subtype_name))
+ return false;
+
+ /* FIXME: implement meaningful checks to make sure that the given codec
+ * is compatible with the given endpoint */
+
+ return true;
+}
+
+/*! Decide for one suitable codec
+ * \param[in] conn related rtp-connection.
+ * \returns 0 on success, -EINVAL on failure. */
+int mgcp_codec_decide(struct mgcp_conn_rtp *conn)
+{
+ struct mgcp_rtp_end *rtp;
+ unsigned int i;
+ struct mgcp_endpoint *endp;
+ bool codec_assigned = false;
+
+ endp = conn->conn->endp;
+ rtp = &conn->end;
+
+ /* This function works on the results the SDP/LCO parser has extracted
+ * from the MGCP message. The goal is to select a suitable codec for
+ * the given connection. When transcoding is available, the first codec
+ * from the codec list is taken without further checking. When
+ * transcoding is not available, then the choice must be made more
+ * carefully. Each codec in the list is checked until one is found that
+ * is rated compatible. The rating is done by the helper function
+ * is_codec_compatible(), which does the actual checking. */
+ for (i = 0; i < rtp->codecs_assigned; i++) {
+ /* When no transcoding is available, avoid codecs that would
+ * require transcoding. */
+ if (endp->trunk->no_audio_transcoding &&
!is_codec_compatible(endp, &rtp->codecs[i])) {
+ LOGP(DLMGCP, LOGL_NOTICE, "transcoding not available,
skipping codec: %d/%s\n",
+ rtp->codecs[i].payload_type,
rtp->codecs[i].subtype_name);
+ continue;
+ }
+
+ rtp->codec = &rtp->codecs[i];
+ codec_assigned = true;
+ break;
+ }
+
+ /* FIXME: To the reviewes: This is problematic. I do not get why we
+ * need to reset the packet_duration_ms depending on the codec
+ * selection. I thought it were all 20ms? Is this to address some
+ * cornercase. (This piece of code was in the code path before,
+ * together with the note: "TODO/XXX: Store this per codec and derive
+ * it on use" */
+ if (codec_assigned) {
+ if (rtp->maximum_packet_time >= 0
+ && rtp->maximum_packet_time *
rtp->codec->frame_duration_den >
+ rtp->codec->frame_duration_num * 1500)
+ rtp->packet_duration_ms = 0;
+
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+/* Check if the codec has a specific AMR mode (octet-aligned or
bandwith-efficient) set. */
+bool mgcp_codec_amr_align_mode_is_indicated(const struct mgcp_rtp_codec *codec)
+{
+ if (codec->param_present == false)
+ return false;
+ if (!codec->param.amr_octet_aligned_present)
+ return false;
+ if (strcmp(codec->subtype_name, "AMR") != 0)
+ return false;
+ return true;
+}
+
/* Find the payload type number configured for a specific codec by SDP.
* For example, IuUP gets assigned a payload type number, and the endpoint
needs to translate that to the number
* assigned to "AMR" on the other conn (by a=rtpmap:N).
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/32506
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I2f2538ff912eae4d80d3b74b766e18c4da94d6b6
Gerrit-Change-Number: 32506
Gerrit-PatchSet: 3
Gerrit-Owner: dexter <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: msuraev <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-MessageType: merged
