dexter has uploaded this change for review. ( 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 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, 28 insertions(+), 13 deletions(-)

git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/91/27391/1

diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c index 38aa0a7..6b8d14a 100644 --- a/src/libosmo-mgcp/mgcp_codec.c +++ b/src/libosmo-mgcp/mgcp_codec.c @@ -378,10 +378,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 4ed18c2..9b4933c 100644 --- a/tests/mgcp/mgcp_test.c +++ b/tests/mgcp/mgcp_test.c @@ -1896,13 +1896,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 }, }, }, @@ -1911,13 +1914,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 }, }, }, @@ -1926,13 +1931,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 @@ -1347,24 +1347,32 @@ #5: test AMR with differing octet-aligned settings - add codecs on conn0: 0: 111 AMR/8000 octet-aligned=1 -> rc=0 + 1: 112 AMR/8000 octet-aligned=0 -> rc=0 - add codecs on conn1: 0: 122 AMR/8000 octet-aligned=0 -> rc=0 - - mgcp_codec_pt_translate(conn0, conn1, 111) -> 122 - - mgcp_codec_pt_translate(conn1, conn0, 122) -> 111 + 1: 121 AMR/8000 octet-aligned=1 -> rc=0 + - mgcp_codec_pt_translate(conn0, conn1, 111) -> 121 + - mgcp_codec_pt_translate(conn1, conn0, 121) -> 111 + - mgcp_codec_pt_translate(conn0, conn1, 112) -> 122 + - mgcp_codec_pt_translate(conn1, conn0, 122) -> 112 #6: test AMR with missing octet-aligned settings (defaults to 0) - add codecs on conn0: 0: 111 AMR/8000 octet-aligned=1 -> rc=0 + 1: 112 AMR/8000 octet-aligned=0 -> rc=0 - add codecs on conn1: 0: 122 AMR/8000 octet-aligned=unset -> rc=0 - - mgcp_codec_pt_translate(conn0, conn1, 111) -> 122 - - mgcp_codec_pt_translate(conn1, conn0, 122) -> 111 + - mgcp_codec_pt_translate(conn0, conn1, 111) -> -22 + - mgcp_codec_pt_translate(conn0, conn1, 112) -> 122 + - mgcp_codec_pt_translate(conn1, conn0, 122) -> 112 #7: test AMR with NULL param (defaults to 0) - add codecs on conn0: 0: 111 AMR/8000 octet-aligned=1 -> rc=0 + 1: 112 AMR/8000 octet-aligned=0 -> rc=0 - add codecs on conn1: 0: 122 AMR/8000 -> rc=0 - - mgcp_codec_pt_translate(conn0, conn1, 111) -> 122 - - mgcp_codec_pt_translate(conn1, conn0, 122) -> 111 + - mgcp_codec_pt_translate(conn0, conn1, 111) -> -22 + - mgcp_codec_pt_translate(conn0, conn1, 112) -> 122 + - mgcp_codec_pt_translate(conn1, conn0, 122) -> 112 #8: match FOO/8000/1 and FOO/8000 as identical, single channel is implicit - add codecs on conn0: 0: 0 PCMU/8000/1 -> rc=0