diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c index c287b7eb6..ccb5d7714 100644 --- a/src/libosmo-mgcp/mgcp_codec.c +++ b/src/libosmo-mgcp/mgcp_codec.c @@ -376,10 +376,15 @@ bool mgcp_codec_amr_is_octet_aligned(const struct mgcp_rtp_codec *codec) return codec->param.amr_octet_aligned; } -/* Compare two codecs, all parameters must match up, except for the payload type - * number. */ +/* Compare two codecs, all parameters must match up */ static bool codecs_same(struct mgcp_rtp_codec *codec_a, struct mgcp_rtp_codec *codec_b) { + /* All codec properties must match up, except the payload type number. Even though standardisd payload numbers + * exist for certain situations, the call agent may still assign them freely. Hence we must not insist on equal + * payload type numbers. Also the audio_name is not checked since it is already parsed into subtype_name, rate, + * and channels, which are checked. */ + if (strcmp(codec_a->subtype_name, codec_b->subtype_name)) + return false; if (codec_a->rate != codec_b->rate) return false; if (codec_a->channels != codec_b->channels) @@ -388,9 +393,11 @@ static bool codecs_same(struct mgcp_rtp_codec *codec_a, struct mgcp_rtp_codec *c return false; if (codec_a->frame_duration_den != codec_b->frame_duration_den) return false; - if (strcmp(codec_a->subtype_name, codec_b->subtype_name)) - return false; - if (!strcmp(codec_a->subtype_name, "AMR")) { + + /* AMR payload may be formatted in two different payload formats, it is still the same codec but since the + * formatting of the payload is different, conversation is required, so we must treat it as a different + * codec here. */ + if (strcmp(codec_a->subtype_name, "AMR") == 0) { if (mgcp_codec_amr_is_octet_aligned(codec_a) != mgcp_codec_amr_is_octet_aligned(codec_b)) return false; } @@ -398,6 +405,26 @@ static bool codecs_same(struct mgcp_rtp_codec *codec_a, struct mgcp_rtp_codec *c return true; } +/* Compare two codecs, all parameters must match up, except parameters related to payload formatting (not checked). */ +static bool codecs_convertible(struct mgcp_rtp_codec *codec_a, struct mgcp_rtp_codec *codec_b) +{ + /* OsmoMGW currently has no ability to transcode from one codec to another. However OsmoMGW is still able to + * translate between different payload formats as long as the encoded voice data itself does not change. + * Therefore we must insist on equal codecs but still allow different payload formatting. */ + if (strcmp(codec_a->subtype_name, codec_b->subtype_name)) + return false; + if (codec_a->rate != codec_b->rate) + return false; + if (codec_a->channels != codec_b->channels) + return false; + if (codec_a->frame_duration_num != codec_b->frame_duration_num) + return false; + if (codec_a->frame_duration_den != codec_b->frame_duration_den) + return false; + + return true; +} + /*! Translate a given payload type number that belongs to the packet of a * source connection to the equivalent payload type number that matches the * configuration of a destination connection. @@ -429,8 +456,8 @@ int mgcp_codec_pt_translate(struct mgcp_conn_rtp *conn_src, struct mgcp_conn_rtp if (!codec_src) return -EINVAL; - /* Use the codec information from the source and try to find the - * equivalent of it on the destination side */ + /* Use the codec information from the source and try to find the equivalent of it on the destination side. In + * the first run we will look for an exact match. */ codecs_assigned = rtp_dst->codecs_assigned; OSMO_ASSERT(codecs_assigned <= MGCP_MAX_CODECS); for (i = 0; i < codecs_assigned; i++) { @@ -439,6 +466,18 @@ int mgcp_codec_pt_translate(struct mgcp_conn_rtp *conn_src, struct mgcp_conn_rtp break; } } + + /* In case we weren't able to find an exact match, we will try to find a match that is the same codec, but the + * payload format may be different. This alternative will require a frame format conversion (i.e. AMR bwe->oe) */ + if (!codec_dst) { + for (i = 0; i < codecs_assigned; i++) { + if (codecs_convertible(codec_src, &rtp_dst->codecs[i])) { + codec_dst = &rtp_dst->codecs[i]; + break; + } + } + } + if (!codec_dst) return -EINVAL; diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c index 444e07aa3..8f081a9ce 100644 --- a/tests/mgcp/mgcp_test.c +++ b/tests/mgcp/mgcp_test.c @@ -1851,6 +1851,7 @@ static const struct testcase_mgcp_codec_pt_translate test_mgcp_codec_pt_translat { .payload_type_map = {96, 97}, }, { .payload_type_map = {97, 96}, }, { .payload_type_map = {0, 0}, }, + { .payload_type_map = {123, -EINVAL} }, { .end = true }, }, }, @@ -1907,31 +1908,63 @@ static const struct testcase_mgcp_codec_pt_translate test_mgcp_codec_pt_translat .expect = { { .payload_type_map = {111, 121}, }, { .payload_type_map = {112, 122} }, + { .payload_type_map = {123, -EINVAL} }, { .end = true }, }, }, { - .descr = "test AMR with missing octet-aligned settings (defaults to 0)", + .descr = "test AMR with missing octet-aligned settings (oa <-> unset)", .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, -EINVAL}, }, - { .payload_type_map = {112, 122} }, + { .payload_type_map = {111, 122}, }, + { .payload_type_map = {55, -EINVAL}, }, { .end = true }, }, }, { - .descr = "test AMR with NULL param (defaults to 0)", + .descr = "test AMR with missing octet-aligned settings (bwe <-> unset)", + .codecs = { + { + { 111, "AMR/8000", &amr_param_octet_aligned_false, }, + }, + { + { 122, "AMR/8000", &amr_param_octet_aligned_unset, }, + }, + }, + .expect = { + { .payload_type_map = {111, 122}, }, + { .payload_type_map = {55, -EINVAL}, }, + { .end = true }, + }, + }, + { + .descr = "test AMR with NULL param (oa <-> null)", + .codecs = { + { + { 112, "AMR/8000", &amr_param_octet_aligned_true, }, + }, + { + { 122, "AMR/8000", NULL, }, + }, + }, + .expect = { + /* Note: Both 111, anbd 112 will translate to 122. The translation from 112 */ + { .payload_type_map = {112, 122} }, + { .payload_type_map = {55, -EINVAL}, }, + { .end = true }, + }, + }, + { + .descr = "test AMR with NULL param (bwe <-> null)", .codecs = { { - { 111, "AMR/8000", &amr_param_octet_aligned_true, }, { 112, "AMR/8000", &amr_param_octet_aligned_false, }, }, { @@ -1939,8 +1972,9 @@ static const struct testcase_mgcp_codec_pt_translate test_mgcp_codec_pt_translat }, }, .expect = { - { .payload_type_map = {111, -EINVAL}, }, + /* Note: Both 111, anbd 112 will translate to 122. The translation from 112 */ { .payload_type_map = {112, 122} }, + { .payload_type_map = {55, -EINVAL}, }, { .end = true }, }, }, diff --git a/tests/mgcp/mgcp_test.ok b/tests/mgcp/mgcp_test.ok index 94fada35c..8b3b3d12a 100644 --- a/tests/mgcp/mgcp_test.ok +++ b/tests/mgcp/mgcp_test.ok @@ -1324,6 +1324,7 @@ Testing mgcp_codec_pt_translate() - mgcp_codec_pt_translate(conn1, conn0, 96) -> 97 - mgcp_codec_pt_translate(conn0, conn1, 0) -> 0 - mgcp_codec_pt_translate(conn1, conn0, 0) -> 0 + - mgcp_codec_pt_translate(conn0, conn1, 123) -> -22 #3: conn0 has no codecs - add codecs on conn0: (none) @@ -1355,25 +1356,40 @@ Testing mgcp_codec_pt_translate() - 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) + - mgcp_codec_pt_translate(conn0, conn1, 123) -> -22 +#6: test AMR with missing octet-aligned settings (oa <-> unset) - 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) -> -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) + - mgcp_codec_pt_translate(conn0, conn1, 111) -> 122 + - mgcp_codec_pt_translate(conn1, conn0, 122) -> 111 + - mgcp_codec_pt_translate(conn0, conn1, 55) -> -22 +#7: test AMR with missing octet-aligned settings (bwe <-> unset) - add codecs on conn0: - 0: 111 AMR/8000 octet-aligned=1 -> rc=0 - 1: 112 AMR/8000 octet-aligned=0 -> rc=0 + 0: 111 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, 55) -> -22 +#8: test AMR with NULL param (oa <-> null) + - add codecs on conn0: + 0: 112 AMR/8000 octet-aligned=1 -> rc=0 - add codecs on conn1: 0: 122 AMR/8000 -> rc=0 - - 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 + - mgcp_codec_pt_translate(conn0, conn1, 55) -> -22 +#9: test AMR with NULL param (bwe <-> null) + - add codecs on conn0: + 0: 112 AMR/8000 octet-aligned=0 -> rc=0 + - add codecs on conn1: + 0: 122 AMR/8000 -> rc=0 + - mgcp_codec_pt_translate(conn0, conn1, 112) -> 122 + - mgcp_codec_pt_translate(conn1, conn0, 122) -> 112 + - mgcp_codec_pt_translate(conn0, conn1, 55) -> -22 +#10: 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 @@ -1389,7 +1405,7 @@ Testing mgcp_codec_pt_translate() - 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 +#11: 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 @@ -1405,7 +1421,7 @@ Testing mgcp_codec_pt_translate() - 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 +#12: 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