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

3 files changed, 28 insertions, 13 deletions

diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c
index 1a8d26a61..c287b7eb6 100644
--- a/src/libosmo-mgcp/mgcp_codec.c
+++ b/src/libosmo-mgcp/mgcp_codec.c
@@ -390,10 +390,10 @@ static bool codecs_same(struct mgcp_rtp_codec *codec_a, struct mgcp_rtp_codec *c
 		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 dfaba7198..444e07aa3 100644
--- a/tests/mgcp/mgcp_test.c
+++ b/tests/mgcp/mgcp_test.c
@@ -1897,13 +1897,16 @@ static const struct testcase_mgcp_codec_pt_translate test_mgcp_codec_pt_translat
 		.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 @@ static const struct testcase_mgcp_codec_pt_translate test_mgcp_codec_pt_translat
 		.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 @@ static const struct testcase_mgcp_codec_pt_translate test_mgcp_codec_pt_translat
 		.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 5b8d558b6..94fada35c 100644
--- a/tests/mgcp/mgcp_test.ok
+++ b/tests/mgcp/mgcp_test.ok
@@ -1347,24 +1347,32 @@ Testing mgcp_codec_pt_translate()
 #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