Attention is currently required from: dexter. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/31214 ) Change subject: mgcp_sdp: add fmtp string to define HR GSM RTP format ...................................................................... Patch Set 10: Code-Review-1 (1 comment) File include/osmocom/mgcp/mgcp_common.h: https://gerrit.osmocom.org/c/osmo-mgw/+/31214/comment/1701ba1a_21068860 PS10, Line 71: enum mgcp_gsm_hr_fmt gsm_hr_format; struct mgcp_codec_param has a fundamental problem: it lives in struct mgcp_msg, i.e. there can *only be one* in an entire SDP message in the osmo_mgcp_client API. However, "a=fmtp:<pt-nr>" are scoped *per payload type nr*, i.e. there should be one for every codec, not only one per SDP message. amr_octet_aligned has this problem, meaning that our MGCP client cannot offer two distinct payload types for AMR octet-aligned and AMR bandwidth-efficient. This recently bit me while working on codec resolution in osmo-msc and osmo-bsc. So far I have to decide for OA or not for *all* AMR codecs represented in our mgcp_client SDP. Let's not proliferate this problem. This probably means we first need to fix this scoping problem before adding new fmtp -- because when you make room for per-pt fmtp, might as well move the OA flag there. (If you instead would want one global flag, it would be a new X-Osmo header; but I think per pt fmtp is the right choice here) -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/31214 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Idde8da27fd335dc03b8fbd9e0fedc1491b77e9e4 Gerrit-Change-Number: 31214 Gerrit-PatchSet: 10 Gerrit-Owner: dexter <pmaier@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge@osmocom.org> Gerrit-Reviewer: neels <nhofmeyr@sysmocom.de> Gerrit-Reviewer: pespin <pespin@sysmocom.de> Gerrit-Attention: dexter <pmaier@sysmocom.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 14:02:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment