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 <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: neels <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-Attention: dexter <[email protected]>
Gerrit-Comment-Date: Tue, 28 Feb 2023 14:02:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Reply via email to