mgcp-client API: there should be a separate mgcp_codec_param for each codec in mgcp_msg.codecs
This was previously discussed in ticket #5723, which got deleted due to an error.
There is a problem we face with the mgcp_client API. mgcp_client_fsm in particular.
The problem is that we are not able to assign different payload type numbers for the same codec. So it is not possible to have the following configuration:
codec: AMR, payload-type: 112
codec: AMR, payload-type: 113
However, this would be necessary in case we want to define AMR once in octet-aligned and once in bandwith-efficient configuration, which brings us to the second problem. Since there is only one mgcp_codec_param struct member "param", which only lets us to define exactly one set of codec parameters we cannot even define two different configurations.
Also in some cases it would be also convenient to circumvent the SDP parsing/generation of osmo-mgcp-client and use a user defined SDP string directly.
I have now resumed working on this:
For the problem we have with ptmap I found an elegant fix that won't break the ABI or API. We now can just define multiple occurrences of the same codec in ptmap. In case multiple occurrences are used, then the order must match the order that is used in codecs.
https://gerrit.osmocom.org/c/osmo-mgw/+/34349 mgcp_client_fsm: allow the same codec multiple times in ptmap [NEW]
For the problem with the codec params we still have to add an array. I have done this now. I also re-defined struct mgcp_codec_param as mgcp_codec_param2 since I think a union makes more sense than the _present logic of the old mgcp_codec_param.
https://gerrit.osmocom.org/c/osmo-mgw/+/34350 mgcp_client_fsm: explain member param in struct mgcp_conn_peer better [NEW]
https://gerrit.osmocom.org/c/osmo-mgw/+/34351 mgcp_client_fsm: fix inconsistent API (param_present, param). [NEW]
(I have not forgotten the approach where we can pass in SDP directly. I will add this in a follow up patch)
I just submitted https://gerrit.osmocom.org/c/osmo-mgw/+/34899 to allow multiple variants of one codec, as an alternative to https://gerrit.osmocom.org/c/osmo-mgw/+/34349, which does the same thing but in another direction. Instead of that patch, it is possible to remove complexity to fix the problem.
It still has some problems in the ttcn3 test suite concerning conversion between OA and BE, so marked WIP.
dexter It would be great to hear what you think of these alternative patches of mine.
I think by submitting my patch, I may inadvertently have rebased your two patches that I took up in my branch.
I hope you agree with those ideas.
- % Done changed from 80 to 90
the patch series is now complete from my POV.
I tweaked a fmtp parameter value from "ts101318" to "3gpp-ts-101.318".
I'm not so sure about the dot in the end, though this seems the most accurate choice to me.
dexter In case we already have a different value merged somewhere else, we can of course adjust. But it would be good to get the "3GPP" squeezed in there somehow.
On Tue, Oct 31, 2023 at 10:34:18AM +0000, dexter wrote:
I think it is no problem to use 3gpp-ts-101.318. I also don't think that the dot would cause any trouble. It is not used as a separator or anything.
don't the MGCP and SDP specs contain an EBNF syntax or something like this, which formally defines which
characters are permitted where?
Here is the fmtp definition:
fmtp-value = fmt SP format-specific-params [means "a=fmtp:N <format-specific-params>"]
format-specific-params = byte-string
byte-string = 1*(%x01-09/%x0B-0C/%x0E-FF)
;any byte except NUL, CR, or LF
and the further limitation:
Format-specific parameters, semicolon separated, may be any set of parameters required to be conveyed...
Basically total freedom within one line of text, and ';' reserved for separator.
It does not say anything about "foo=bar" assignments, except for the examples.
On Wed, Nov 01, 2023 at 01:28:47AM +0000, neels wrote:
Here is the fmtp definition:
thanks for checking. It's always good to verify rather than making assumptions (the previous
ticket update sounded to me like that was the case).