This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
neels gerrit-no-reply at lists.osmocom.orgneels has submitted this change and it was merged. ( https://gerrit.osmocom.org/c/osmo-mgw/+/15137 ) Change subject: ptmap: implicitly match '/8000' and '/8000/1' ...................................................................... ptmap: implicitly match '/8000' and '/8000/1' In codecs_same(), do not compare the complete audio_name. The parts of it are already checked individually: - subtype_name ("AMR"), - rate ("8000"; defaults to 8000 if omitted) and - channels ("1"; defaults to 1 if omitted) So by also checking the complete audio_name, we brushed over the match of implicit "/8000" and "/8000/1", which otherwise works out fine. As a result, translating payload type numbers in RTP headers now also works if one conn of an endpoint set an rtpmap with "AMR/8000" and the other conn set "AMR/8000/1". It seems to me that most PBX out there generate ptmaps omitting the "/1", so fixing this should make us more interoperable with third party SDP. See IETF RFC4566 section 6. SDP Attributes: For audio streams, <encoding parameters> indicates the number of audio channels. This parameter is OPTIONAL and may be omitted if the number of channels is one, provided that no additional parameters are needed. Also allowing to omit the "/8000" is a mere side effect of this patch. Omitting the rate does not seem to be specified in an RFC, but is logical for audio codecs defined to require exactly 8000 set as rate (most GSM codecs). Add tests in mgcp_test.c. Change-Id: Iab00bf9a55b1847f85999077114b37e70fb677c2 --- M src/libosmo-mgcp/mgcp_codec.c M tests/mgcp/mgcp_test.c M tests/mgcp/mgcp_test.ok 3 files changed, 108 insertions(+), 2 deletions(-) Approvals: Jenkins Builder: Verified neels: Looks good to me, approved pespin: Looks good to me, but someone else must approve diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c index 5d7f840..7c8f6d5 100644 --- a/src/libosmo-mgcp/mgcp_codec.c +++ b/src/libosmo-mgcp/mgcp_codec.c @@ -379,8 +379,6 @@ return false; if (codec_a->frame_duration_den != codec_b->frame_duration_den) return false; - if (strcmp(codec_a->audio_name, codec_b->audio_name)) - return false; if (strcmp(codec_a->subtype_name, codec_b->subtype_name)) return false; if (!strcmp(codec_a->subtype_name, "AMR")) { diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c index 5ebe475..e5dec2a 100644 --- a/tests/mgcp/mgcp_test.c +++ b/tests/mgcp/mgcp_test.c @@ -1900,6 +1900,70 @@ { .end = true }, }, }, + { + .descr = "match FOO/8000/1 and FOO/8000 as identical, single channel is implicit", + .codecs = { + { + { 0, "PCMU/8000/1", NULL, }, + { 111, "GSM-HR-08/8000/1", NULL, }, + { 112, "AMR/8000/1", &amr_param_octet_aligned_true, }, + }, + { + { 97, "GSM-HR-08/8000", NULL, }, + { 0, "PCMU/8000", NULL, }, + { 96, "AMR/8000", &amr_param_octet_aligned_true, }, + }, + }, + .expect = { + { .payload_type_map = {112, 96}, }, + { .payload_type_map = {0, 0}, }, + { .payload_type_map = {111, 97} }, + { .payload_type_map = {123, -EINVAL} }, + { .end = true }, + }, + }, + { + .descr = "match FOO/8000/1 and FOO as identical, 8k and single channel are implicit", + .codecs = { + { + { 0, "PCMU/8000/1", NULL, }, + { 111, "GSM-HR-08/8000/1", NULL, }, + { 112, "AMR/8000/1", &amr_param_octet_aligned_true, }, + }, + { + { 97, "GSM-HR-08", NULL, }, + { 0, "PCMU", NULL, }, + { 96, "AMR", &amr_param_octet_aligned_true, }, + }, + }, + .expect = { + { .payload_type_map = {112, 96}, }, + { .payload_type_map = {0, 0}, }, + { .payload_type_map = {111, 97} }, + { .payload_type_map = {123, -EINVAL} }, + { .end = true }, + }, + }, + { + .descr = "test whether channel number matching is waterproof", + .codecs = { + { + { 111, "GSM-HR-08/8000", }, + { 112, "GSM-HR-08/8000/2", .expect_rc = -22}, + { 113, "GSM-HR-08/8000/3", .expect_rc = -22}, + }, + { + { 122, "GSM-HR-08/8000/2", .expect_rc = -22}, + { 121, "GSM-HR-08/8000/1", }, + }, + }, + .expect = { + { .payload_type_map = {111, 121}, }, + { .payload_type_map = {112, -EINVAL} }, + { .payload_type_map = {113, -EINVAL} }, + { .end = true }, + }, + }, }; static void test_mgcp_codec_pt_translate(void) diff --git a/tests/mgcp/mgcp_test.ok b/tests/mgcp/mgcp_test.ok index 14d5d73..9c48147 100644 --- a/tests/mgcp/mgcp_test.ok +++ b/tests/mgcp/mgcp_test.ok @@ -1315,6 +1315,50 @@ - 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 + 1: 111 GSM-HR-08/8000/1 -> rc=0 + 2: 112 AMR/8000/1 octet-aligned=1 -> rc=0 + - add codecs on conn1: + 0: 97 GSM-HR-08/8000 -> rc=0 + 1: 0 PCMU/8000 -> rc=0 + 2: 96 AMR/8000 octet-aligned=1 -> rc=0 + - mgcp_codec_pt_translate(conn0, conn1, 112) -> 96 + - mgcp_codec_pt_translate(conn1, conn0, 96) -> 112 + - mgcp_codec_pt_translate(conn0, conn1, 0) -> 0 + - mgcp_codec_pt_translate(conn1, conn0, 0) -> 0 + - mgcp_codec_pt_translate(conn0, conn1, 111) -> 97 + - mgcp_codec_pt_translate(conn1, conn0, 97) -> 111 + - mgcp_codec_pt_translate(conn0, conn1, 123) -> -22 +#9: match FOO/8000/1 and FOO as identical, 8k and single channel are implicit + - add codecs on conn0: + 0: 0 PCMU/8000/1 -> rc=0 + 1: 111 GSM-HR-08/8000/1 -> rc=0 + 2: 112 AMR/8000/1 octet-aligned=1 -> rc=0 + - add codecs on conn1: + 0: 97 GSM-HR-08 -> rc=0 + 1: 0 PCMU -> rc=0 + 2: 96 AMR octet-aligned=1 -> rc=0 + - mgcp_codec_pt_translate(conn0, conn1, 112) -> 96 + - mgcp_codec_pt_translate(conn1, conn0, 96) -> 112 + - mgcp_codec_pt_translate(conn0, conn1, 0) -> 0 + - mgcp_codec_pt_translate(conn1, conn0, 0) -> 0 + - mgcp_codec_pt_translate(conn0, conn1, 111) -> 97 + - mgcp_codec_pt_translate(conn1, conn0, 97) -> 111 + - mgcp_codec_pt_translate(conn0, conn1, 123) -> -22 +#10: test whether channel number matching is waterproof + - add codecs on conn0: + 0: 111 GSM-HR-08/8000 -> rc=0 + 1: 112 GSM-HR-08/8000/2 -> rc=-22 + 2: 113 GSM-HR-08/8000/3 -> rc=-22 + - add codecs on conn1: + 0: 122 GSM-HR-08/8000/2 -> rc=-22 + 1: 121 GSM-HR-08/8000/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) -> -22 + - mgcp_codec_pt_translate(conn0, conn1, 113) -> -22 Testing test_conn_id_matching needle='23AB' found '000023AB' -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/15137 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Iab00bf9a55b1847f85999077114b37e70fb677c2 Gerrit-Change-Number: 15137 Gerrit-PatchSet: 5 Gerrit-Owner: neels <nhofmeyr at sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge at gnumonks.org> Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de> Gerrit-Reviewer: pespin <pespin at sysmocom.de> Gerrit-MessageType: merged -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190829/52905d80/attachment.htm>