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(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Feb 2023 14:02:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment