laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/27391 ) Change subject: Revert "mgcp_codec: do not differentiate between oa and bwe when comparing codec" ...................................................................... Revert "mgcp_codec: do not differentiate between oa and bwe when comparing codec" This reverts commit e0058b7207e022b698aea10f96cc7c0b1209058a. The reason for this revert is that the solution in the reverted patch does not cover a situation where the other side announces both payload formats at the same time. It could be that the end facing to a transit network announces both formats under two different payload types. In this case no conversion would be necessary. Depending on the input format the output would be send to the transit network under the payload type that matches and no conversion would happen at all. This revert re-intruduces the problem that was fixed in the patch before. Therefore it must be merged together with the follow up patch (Ifbd201a2749009a4644a29bd77e1d0fc0c124a9d) that contains the proper fix. Change-Id: I0b2854ef2397f38606fab3425be586a3d0ca27d1 Related: OS#5461 --- M src/libosmo-mgcp/mgcp_codec.c M tests/mgcp/mgcp_test.c M tests/mgcp/mgcp_test.ok 3 files changed, 39 insertions(+), 7 deletions(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, but someone else must approve laforge: Looks good to me, approved diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c index 1a8d26a..c287b7e 100644 --- a/src/libosmo-mgcp/mgcp_codec.c +++ b/src/libosmo-mgcp/mgcp_codec.c @@ -390,10 +390,10 @@ return false; if (strcmp(codec_a->subtype_name, codec_b->subtype_name)) return false; - - /* Note: AMR allows to set the RTP payload format to octet-aligned or bandwith-efficient (octet-aligned=0) - * via SDP. This difference concerns payload format only, but not the actual codec. It is not a difference - * within the meaning of this function. */ + if (!strcmp(codec_a->subtype_name, "AMR")) { + if (mgcp_codec_amr_is_octet_aligned(codec_a) != mgcp_codec_amr_is_octet_aligned(codec_b)) + return false; + } return true; } diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c index dfaba71..444e07a 100644 --- a/tests/mgcp/mgcp_test.c +++ b/tests/mgcp/mgcp_test.c @@ -1897,13 +1897,16 @@ .codecs = { { { 111, "AMR/8000", &amr_param_octet_aligned_true, }, + { 112, "AMR/8000", &amr_param_octet_aligned_false, }, }, { { 122, "AMR/8000", &amr_param_octet_aligned_false, }, + { 121, "AMR/8000", &amr_param_octet_aligned_true, }, }, }, .expect = { - { .payload_type_map = {111, 122}, }, + { .payload_type_map = {111, 121}, }, + { .payload_type_map = {112, 122} }, { .end = true }, }, }, @@ -1912,13 +1915,15 @@ .codecs = { { { 111, "AMR/8000", &amr_param_octet_aligned_true, }, + { 112, "AMR/8000", &amr_param_octet_aligned_false, }, }, { { 122, "AMR/8000", &amr_param_octet_aligned_unset, }, }, }, .expect = { - { .payload_type_map = {111, 122}, }, + { .payload_type_map = {111, -EINVAL}, }, + { .payload_type_map = {112, 122} }, { .end = true }, }, }, @@ -1927,13 +1932,15 @@ .codecs = { { { 111, "AMR/8000", &amr_param_octet_aligned_true, }, + { 112, "AMR/8000", &amr_param_octet_aligned_false, }, }, { { 122, "AMR/8000", NULL, }, }, }, .expect = { - { .payload_type_map = {111, 122}, }, + { .payload_type_map = {111, -EINVAL}, }, + { .payload_type_map = {112, 122} }, { .end = true }, }, }, diff --git a/tests/mgcp/mgcp_test.ok b/tests/mgcp/mgcp_test.ok index 5b8d558..94fada3 100644 --- a/tests/mgcp/mgcp_test.ok +++ b/tests/mgcp/mgcp_test.ok Binary files differ -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/27391 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I0b2854ef2397f38606fab3425be586a3d0ca27d1 Gerrit-Change-Number: 27391 Gerrit-PatchSet: 3 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged