Attention is currently required from: dexter.

Patch set 10:Code-Review -1

View Change

1 comment:

• File include/osmocom/mgcp/mgcp_common.h:

  • Patch Set #10, 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 change 31214. To unsubscribe, or for help writing mail filters, visit settings.