From 33f300915a09ddd16dfcdac2912d79bcdb361bf7 Mon Sep 17 00:00:00 2001 From: Jacob Erlbeck Date: Tue, 10 Dec 2013 13:09:37 +0100 Subject: mgcp/rtp: Refactor timestamp offset calculation into own function Currently the timestamp offset calculation is done in two different places. This patch moves and unifies both code parts into a separate function. Sponsored-by: On-Waves ehf --- openbsc/src/libmgcp/mgcp_network.c | 98 +++++++++++++++++++++++++------------- 1 file changed, 66 insertions(+), 32 deletions(-) diff --git a/openbsc/src/libmgcp/mgcp_network.c b/openbsc/src/libmgcp/mgcp_network.c index bf205d1c7..8bd8afba8 100644 --- a/openbsc/src/libmgcp/mgcp_network.c +++ b/openbsc/src/libmgcp/mgcp_network.c @@ -200,6 +200,58 @@ static int check_rtp_timestamp(struct mgcp_endpoint *endp, return 1; } +/* Set the timestamp offset according to the packet duration. */ +static int adjust_rtp_timestamp_offset(struct mgcp_endpoint *endp, + struct mgcp_rtp_state *state, + struct mgcp_rtp_end *rtp_end, + struct sockaddr_in *addr, + int16_t delta_seq, uint32_t timestamp) +{ + int32_t tsdelta = state->packet_duration; + int timestamp_offset; + + if (tsdelta == 0) { + tsdelta = state->out_stream.last_tsdelta; + if (tsdelta != 0) { + LOGP(DMGCP, LOGL_NOTICE, + "A fixed packet duration is not available on 0x%x, " + "using last output timestamp delta instead: %d " + "from %s:%d in %d\n", + ENDPOINT_NUMBER(endp), tsdelta, + inet_ntoa(addr->sin_addr), ntohs(addr->sin_port), + endp->conn_mode); + } else { + tsdelta = rtp_end->rate * 20 / 1000; + LOGP(DMGCP, LOGL_NOTICE, + "Fixed packet duration and last timestamp delta " + "are not available on 0x%x, " + "using fixed 20ms instead: %d " + "from %s:%d in %d\n", + ENDPOINT_NUMBER(endp), tsdelta, + inet_ntoa(addr->sin_addr), ntohs(addr->sin_port), + endp->conn_mode); + } + } + + timestamp_offset = + state->out_stream.last_timestamp - timestamp + + delta_seq * tsdelta; + + if (state->timestamp_offset != timestamp_offset) { + state->timestamp_offset = timestamp_offset; + + LOGP(DMGCP, LOGL_NOTICE, + "Timestamp offset change on 0x%x SSRC: %u " + "SeqNo delta: %d, TS offset: %d, " + "from %s:%d in %d\n", + ENDPOINT_NUMBER(endp), state->in_stream.ssrc, + delta_seq, state->timestamp_offset, + inet_ntoa(addr->sin_addr), ntohs(addr->sin_port), + endp->conn_mode); + } + + return timestamp_offset; +} /** * The RFC 3550 Appendix A assumes there are multiple sources but @@ -259,22 +311,14 @@ void mgcp_patch_and_count(struct mgcp_endpoint *endp, struct mgcp_rtp_state *sta state->in_stream.ssrc = ssrc; if (rtp_end->force_constant_ssrc) { - int32_t tsdelta = state->out_stream.last_tsdelta; - if (tsdelta == 0) { - tsdelta = state->packet_duration; - LOGP(DMGCP, LOGL_NOTICE, - "Timestamp delta is not available on 0x%x, " - "using packet duration instead: %d " - "from %s:%d in %d\n", - ENDPOINT_NUMBER(endp), tsdelta, - inet_ntoa(addr->sin_addr), ntohs(addr->sin_port), - endp->conn_mode); - } + const int16_t delta_seq = 1; + state->seq_offset = - (state->out_stream.last_seq + 1) - seq; - state->timestamp_offset = - (state->out_stream.last_timestamp + tsdelta) - - timestamp; + (state->out_stream.last_seq + delta_seq) - seq; + + adjust_rtp_timestamp_offset(endp, state, rtp_end, addr, + delta_seq, timestamp); + state->patch_ssrc = 1; ssrc = state->orig_ssrc; if (rtp_end->force_constant_ssrc != -1) @@ -282,10 +326,10 @@ void mgcp_patch_and_count(struct mgcp_endpoint *endp, struct mgcp_rtp_state *sta LOGP(DMGCP, LOGL_NOTICE, "SSRC patching enabled on 0x%x SSRC: %u " - "offset: %d tsdelta: %d " + "SeqNo offset: %d, TS offset: %d " "from %s:%d in %d\n", ENDPOINT_NUMBER(endp), state->in_stream.ssrc, - state->seq_offset, tsdelta, + state->seq_offset, state->timestamp_offset, inet_ntoa(addr->sin_addr), ntohs(addr->sin_port), endp->conn_mode); } @@ -307,22 +351,12 @@ void mgcp_patch_and_count(struct mgcp_endpoint *endp, struct mgcp_rtp_state *sta if (rtp_end->force_constant_timing && state->out_stream.ssrc == ssrc && state->packet_duration) { - int delta_seq = seq + state->seq_offset - state->out_stream.last_seq; - int timestamp_offset = - state->out_stream.last_timestamp - timestamp + - delta_seq * state->packet_duration; - if (state->timestamp_offset != timestamp_offset) { - state->timestamp_offset = timestamp_offset; + /* Update the timestamp offset */ + const int delta_seq = + seq + state->seq_offset - state->out_stream.last_seq; - LOGP(DMGCP, LOGL_NOTICE, - "Timestamp patching enabled on 0x%x SSRC: %u " - "SeqNo delta: %d, TS offset: %d, " - "from %s:%d in %d\n", - ENDPOINT_NUMBER(endp), state->in_stream.ssrc, - delta_seq, state->timestamp_offset, - inet_ntoa(addr->sin_addr), ntohs(addr->sin_port), - endp->conn_mode); - } + adjust_rtp_timestamp_offset(endp, state, rtp_end, addr, + delta_seq, timestamp); } /* Store the updated SSRC back to the packet */ -- cgit v1.2.3 From ba477d2ba3d6ff667eb38a8b42731a7886781266 Mon Sep 17 00:00:00 2001 From: Jacob Erlbeck Date: Tue, 10 Dec 2013 13:09:37 +0100 Subject: mgcp/test: Output the packet duration after MGCP parsing This also adds additional MDCX tests (based on MDCX4) to test the analysis of different combinations of 'p' and 'ptime' fields. Sponsored-by: On-Waves ehf --- openbsc/tests/mgcp/mgcp_test.c | 69 ++++++++++++++++++++++++++++++++++++++--- openbsc/tests/mgcp/mgcp_test.ok | 21 +++++++++++++ 2 files changed, 85 insertions(+), 5 deletions(-) diff --git a/openbsc/tests/mgcp/mgcp_test.c b/openbsc/tests/mgcp/mgcp_test.c index 7c02d09f2..bea977156 100644 --- a/openbsc/tests/mgcp/mgcp_test.c +++ b/openbsc/tests/mgcp/mgcp_test.c @@ -92,8 +92,9 @@ static void test_strline(void) "c=IN IP4 0.0.0.0\r\n" \ "t=0 0\r\n" \ "m=audio 4441 RTP/AVP 99\r\n" \ - "a=rtpmap:99 AMR/8000\r\n" -#define MDCX4_RET "200 18983216 OK\r\n" \ + "a=rtpmap:99 AMR/8000\r\n" \ + "a=ptime:40\r\n" +#define MDCX4_RET(Ident) "200 " Ident " OK\r\n" \ "I: 1\n" \ "\n" \ "v=0\r\n" \ @@ -103,6 +104,45 @@ static void test_strline(void) "m=audio 0 RTP/AVP 126\r\n" \ "a=rtpmap:126 AMR/8000\r\n" +#define MDCX4_PT1 "MDCX 18983217 1@mgw MGCP 1.0\r\n" \ + "C: 2\r\n" \ + "I: 1\r\n" \ + "L: p:20-40, a:AMR, nt:IN\r\n" \ + "\n" \ + "v=0\r\n" \ + "o=- 1 23 IN IP4 0.0.0.0\r\n" \ + "c=IN IP4 0.0.0.0\r\n" \ + "t=0 0\r\n" \ + "m=audio 4441 RTP/AVP 99\r\n" \ + "a=rtpmap:99 AMR/8000\r\n" \ + "a=ptime:40\r\n" + +#define MDCX4_PT2 "MDCX 18983218 1@mgw MGCP 1.0\r\n" \ + "C: 2\r\n" \ + "I: 1\r\n" \ + "L: p:20-20, a:AMR, nt:IN\r\n" \ + "\n" \ + "v=0\r\n" \ + "o=- 1 23 IN IP4 0.0.0.0\r\n" \ + "c=IN IP4 0.0.0.0\r\n" \ + "t=0 0\r\n" \ + "m=audio 4441 RTP/AVP 99\r\n" \ + "a=rtpmap:99 AMR/8000\r\n" \ + "a=ptime:40\r\n" + +#define MDCX4_PT3 "MDCX 18983219 1@mgw MGCP 1.0\r\n" \ + "C: 2\r\n" \ + "I: 1\r\n" \ + "L: a:AMR, nt:IN\r\n" \ + "\n" \ + "v=0\r\n" \ + "o=- 1 23 IN IP4 0.0.0.0\r\n" \ + "c=IN IP4 0.0.0.0\r\n" \ + "t=0 0\r\n" \ + "m=audio 4441 RTP/AVP 99\r\n" \ + "a=rtpmap:99 AMR/8000\r\n" \ + "a=ptime:40\r\n" + #define SHORT2 "CRCX 1" #define SHORT2_RET "510 000000 FAIL\r\n" #define SHORT3 "CRCX 1 1@mgw" @@ -112,11 +152,13 @@ static void test_strline(void) #define CRCX "CRCX 2 1@mgw MGCP 1.0\r\n" \ "M: sendrecv\r\n" \ "C: 2\r\n" \ + "L: p:20\r\n" \ "\r\n" \ "v=0\r\n" \ "c=IN IP4 123.12.12.123\r\n" \ "m=audio 5904 RTP/AVP 97\r\n" \ - "a=rtpmap:97 GSM-EFR/8000\r\n" + "a=rtpmap:97 GSM-EFR/8000\r\n" \ + "a=ptime:40\r\n" #define CRCX_RET "200 2 OK\r\n" \ "I: 1\n" \ @@ -128,7 +170,6 @@ static void test_strline(void) "m=audio 0 RTP/AVP 126\r\n" \ "a=rtpmap:126 AMR/8000\r\n" - #define CRCX_ZYN "CRCX 2 1@mgw MGCP 1.0\r" \ "M: sendrecv\r" \ "C: 2\r\r" \ @@ -184,7 +225,10 @@ static const struct mgcp_test tests[] = { { "MDCX2", MDCX_UNALLOCATED, MDCX_RET }, { "CRCX", CRCX, CRCX_RET, 97, 126 }, { "MDCX3", MDCX3, MDCX3_RET, PTYPE_NONE, 126 }, - { "MDCX4", MDCX4, MDCX4_RET, 99, 126 }, + { "MDCX4", MDCX4, MDCX4_RET("18983216"), 99, 126 }, + { "MDCX4_PT1", MDCX4_PT1, MDCX4_RET("18983217"), 99, 126 }, + { "MDCX4_PT2", MDCX4_PT2, MDCX4_RET("18983218"), 99, 126 }, + { "MDCX4_PT3", MDCX4_PT3, MDCX4_RET("18983219"), 99, 126 }, { "DLCX", DLCX, DLCX_RET, -1, -1 }, { "CRCX_ZYN", CRCX_ZYN, CRCX_ZYN_RET, 97, 126 }, { "EMPTY", EMPTY, EMPTY_RET }, @@ -245,6 +289,7 @@ static void test_messages(void) for (i = 0; i < cfg->trunk.number_endpoints; i++) { endp = &cfg->trunk.endpoints[i]; endp->net_end.payload_type = PTYPE_NONE; + endp->net_end.packet_duration_ms = -1; } for (i = 0; i < ARRAY_SIZE(tests); i++) { @@ -266,6 +311,20 @@ static void test_messages(void) printf("%s failed '%s'\n", t->name, (char *) msg->data); msgb_free(msg); + if (last_endpoint != -1) { + endp = &cfg->trunk.endpoints[last_endpoint]; + + if (endp->net_end.packet_duration_ms != -1) + printf("Detected packet duration: %d\n", + endp->net_end.packet_duration_ms); + else + printf("Packet duration not set\n"); + printf("Requested packetization period not set\n"); + + endp->net_end.packet_duration_ms = -1; + } + + /* Check detected payload type */ if (t->exp_net_ptype != PTYPE_IGNORE || t->exp_bts_ptype != PTYPE_IGNORE) { diff --git a/openbsc/tests/mgcp/mgcp_test.ok b/openbsc/tests/mgcp/mgcp_test.ok index 3beeb5970..509958aa1 100644 --- a/openbsc/tests/mgcp/mgcp_test.ok +++ b/openbsc/tests/mgcp/mgcp_test.ok @@ -16,10 +16,29 @@ Testing AUEP2 Testing MDCX1 Testing MDCX2 Testing CRCX +Packet duration not set +Requested packetization period not set Testing MDCX3 +Packet duration not set +Requested packetization period not set Testing MDCX4 +Packet duration not set +Requested packetization period not set +Testing MDCX4_PT1 +Packet duration not set +Requested packetization period not set +Testing MDCX4_PT2 +Packet duration not set +Requested packetization period not set +Testing MDCX4_PT3 +Packet duration not set +Requested packetization period not set Testing DLCX +Detected packet duration: 20 +Requested packetization period not set Testing CRCX_ZYN +Packet duration not set +Requested packetization period not set Testing EMPTY Testing SHORT1 Testing SHORT2 @@ -28,6 +47,8 @@ Testing SHORT4 Testing RQNT1 Testing RQNT2 Testing DLCX +Detected packet duration: 20 +Requested packetization period not set Testing CRCX Re-transmitting CRCX Testing RQNT1 -- cgit v1.2.3 From 2c2ca4df382110c4bebdb91d7410838fbf20d493 Mon Sep 17 00:00:00 2001 From: Jacob Erlbeck Date: Mon, 9 Dec 2013 14:32:03 +0100 Subject: mgcp: Put local connection options into a struct Currently the local connection options have been stored as a string. This patch replaces this string by a struct (that still contains a string) along with the parsed fields (only the packetization period at the moment). It also re-adds the calls to set_local_cx_options() to the handle_create_con() and handle_modify_con() functions. Except for the test program this has no side effects, since the LCO values aren't used yet. --- openbsc/include/openbsc/mgcp_internal.h | 8 +++++++- openbsc/src/libmgcp/mgcp_protocol.c | 33 ++++++++++++++++++++++++++++----- openbsc/tests/mgcp/mgcp_test.c | 9 ++++++++- openbsc/tests/mgcp/mgcp_test.ok | 8 ++++---- 4 files changed, 47 insertions(+), 11 deletions(-) diff --git a/openbsc/include/openbsc/mgcp_internal.h b/openbsc/include/openbsc/mgcp_internal.h index d59c5d7cb..20c433a2e 100644 --- a/openbsc/include/openbsc/mgcp_internal.h +++ b/openbsc/include/openbsc/mgcp_internal.h @@ -114,6 +114,12 @@ struct mgcp_rtp_tap { struct sockaddr_in forward; }; +struct mgcp_lco { + char *string; + int pkt_period_min; /* time in ms */ + int pkt_period_max; /* time in ms */ +}; + enum mgcp_type { MGCP_RTP_DEFAULT = 0, MGCP_RTP_TRANSCODED, @@ -123,7 +129,7 @@ struct mgcp_endpoint { int allocated; uint32_t ci; char *callid; - char *local_options; + struct mgcp_lco local_options; int conn_mode; int orig_mode; diff --git a/openbsc/src/libmgcp/mgcp_protocol.c b/openbsc/src/libmgcp/mgcp_protocol.c index ab941645e..c4ff757c3 100644 --- a/openbsc/src/libmgcp/mgcp_protocol.c +++ b/openbsc/src/libmgcp/mgcp_protocol.c @@ -614,6 +614,25 @@ static int parse_sdp_data(struct mgcp_rtp_end *rtp, struct mgcp_parse_data *p) return found_media; } +/* Set the LCO from a string (see RFC 3435). + * The string is stored in the 'string' field. A NULL string is handled excatly + * like an empty string, the 'string' field is never NULL after this function + * has been called. */ +static void set_local_cx_options(void *ctx, struct mgcp_lco *lco, + const char *options) +{ + char *p_opt; + + talloc_free(lco->string); + lco->pkt_period_min = lco->pkt_period_max = 0; + lco->string = talloc_strdup(ctx, options ? options : ""); + + p_opt = strstr(lco->string, "p:"); + if (p_opt && sscanf(p_opt, "p:%d-%d", + &lco->pkt_period_min, &lco->pkt_period_max) == 1) + lco->pkt_period_max = lco->pkt_period_min; +} + void mgcp_rtp_end_config(struct mgcp_endpoint *endp, int expect_ssrc_change, struct mgcp_rtp_end *rtp) { @@ -712,8 +731,8 @@ mgcp_header_done: /* copy some parameters */ endp->callid = talloc_strdup(tcfg->endpoints, callid); - if (local_options) - endp->local_options = talloc_strdup(tcfg->endpoints, local_options); + set_local_cx_options(endp->tcfg->endpoints, &endp->local_options, + local_options); if (parse_conn_mode(mode, &endp->conn_mode) != 0) { error_code = 517; @@ -789,6 +808,7 @@ static struct msgb *handle_modify_con(struct mgcp_parse_data *p) int error_code = 500; int silent = 0; char *line; + const char *local_options = NULL; if (p->found != 0) return create_err_response(NULL, 510, "MDCX", p->trans); @@ -812,7 +832,7 @@ static struct msgb *handle_modify_con(struct mgcp_parse_data *p) break; } case 'L': - /* skip */ + local_options = (const char *) line + 3; break; case 'M': if (parse_conn_mode(line + 3, &endp->conn_mode) != 0) { @@ -838,6 +858,9 @@ static struct msgb *handle_modify_con(struct mgcp_parse_data *p) } } + set_local_cx_options(endp->tcfg->endpoints, &endp->local_options, + local_options); + /* policy CB */ if (p->cfg->policy_cb) { int rc; @@ -1148,8 +1171,8 @@ void mgcp_free_endp(struct mgcp_endpoint *endp) talloc_free(endp->callid); endp->callid = NULL; - talloc_free(endp->local_options); - endp->local_options = NULL; + talloc_free(endp->local_options.string); + endp->local_options.string = NULL; mgcp_rtp_end_reset(&endp->bts_end); mgcp_rtp_end_reset(&endp->net_end); diff --git a/openbsc/tests/mgcp/mgcp_test.c b/openbsc/tests/mgcp/mgcp_test.c index bea977156..8b5e17ddc 100644 --- a/openbsc/tests/mgcp/mgcp_test.c +++ b/openbsc/tests/mgcp/mgcp_test.c @@ -319,7 +319,14 @@ static void test_messages(void) endp->net_end.packet_duration_ms); else printf("Packet duration not set\n"); - printf("Requested packetization period not set\n"); + if (endp->local_options.pkt_period_min || + endp->local_options.pkt_period_max) + printf("Requested packetetization period: " + "%d-%d\n", + endp->local_options.pkt_period_min, + endp->local_options.pkt_period_max); + else + printf("Requested packetization period not set\n"); endp->net_end.packet_duration_ms = -1; } diff --git a/openbsc/tests/mgcp/mgcp_test.ok b/openbsc/tests/mgcp/mgcp_test.ok index 509958aa1..24f9b336c 100644 --- a/openbsc/tests/mgcp/mgcp_test.ok +++ b/openbsc/tests/mgcp/mgcp_test.ok @@ -17,19 +17,19 @@ Testing MDCX1 Testing MDCX2 Testing CRCX Packet duration not set -Requested packetization period not set +Requested packetetization period: 20-20 Testing MDCX3 Packet duration not set Requested packetization period not set Testing MDCX4 Packet duration not set -Requested packetization period not set +Requested packetetization period: 20-20 Testing MDCX4_PT1 Packet duration not set -Requested packetization period not set +Requested packetetization period: 20-40 Testing MDCX4_PT2 Packet duration not set -Requested packetization period not set +Requested packetetization period: 20-20 Testing MDCX4_PT3 Packet duration not set Requested packetization period not set -- cgit v1.2.3 From 24754f0490b0f77fca4110402bbff08603a29360 Mon Sep 17 00:00:00 2001 From: Jacob Erlbeck Date: Tue, 10 Dec 2013 13:09:37 +0100 Subject: mgcp: Parse SDP to get rate and packet duration This patch parses the 'ptime' and 'maxptime' SDP attributes, and the SDP rate information and sets up packet_duration_ms accordingly. If the packet duration is unknown or allows for different values (e.g. because 'ptime' uses a range or 'maxptime' allows for more than one frame) the duration is set to 0. Sponsored-by: On-Waves ehf --- openbsc/src/libmgcp/mgcp_network.c | 9 +++++++ openbsc/src/libmgcp/mgcp_protocol.c | 53 ++++++++++++++++++++++++++++++++----- openbsc/tests/mgcp/mgcp_test.ok | 10 +++---- 3 files changed, 61 insertions(+), 11 deletions(-) diff --git a/openbsc/src/libmgcp/mgcp_network.c b/openbsc/src/libmgcp/mgcp_network.c index 8bd8afba8..40227af01 100644 --- a/openbsc/src/libmgcp/mgcp_network.c +++ b/openbsc/src/libmgcp/mgcp_network.c @@ -300,6 +300,15 @@ void mgcp_patch_and_count(struct mgcp_endpoint *endp, struct mgcp_rtp_state *sta state->seq_offset, state->packet_duration, inet_ntoa(addr->sin_addr), ntohs(addr->sin_port), endp->conn_mode); + if (state->packet_duration == 0 && rtp_end->force_constant_timing) + LOGP(DMGCP, LOGL_ERROR, + "Cannot patch timestamps on 0x%x: " + "RTP packet duration is unknown, SSRC: %u, " + "from %s:%d in %d\n", + ENDPOINT_NUMBER(endp), state->in_stream.ssrc, + inet_ntoa(addr->sin_addr), ntohs(addr->sin_port), + endp->conn_mode); + } else if (state->in_stream.ssrc != ssrc) { LOGP(DMGCP, LOGL_NOTICE, "The SSRC changed on 0x%x: %u -> %u " diff --git a/openbsc/src/libmgcp/mgcp_protocol.c b/openbsc/src/libmgcp/mgcp_protocol.c index c4ff757c3..b8d192043 100644 --- a/openbsc/src/libmgcp/mgcp_protocol.c +++ b/openbsc/src/libmgcp/mgcp_protocol.c @@ -562,25 +562,64 @@ static int parse_sdp_data(struct mgcp_rtp_end *rtp, struct mgcp_parse_data *p) { char *line; int found_media = 0; + int audio_payload = -1; for_each_line(line, p->save) { switch (line[0]) { - case 'a': case 'o': case 's': case 't': case 'v': /* skip these SDP attributes */ break; + case 'a': { + int payload; + int rate; + int channels = 1; + int ptime, ptime2 = 0; + char audio_name[64]; + char audio_codec[64]; + + if (audio_payload == -1) + break; + + if (sscanf(line, "a=rtpmap:%d %64s", + &payload, audio_name) == 2) { + if (payload != audio_payload) + break; + + if (sscanf(audio_name, "%[^/]/%d/%d", + audio_codec, &rate, &channels) < 2) + break; + + rtp->rate = rate; + if (channels != 1) + LOGP(DMGCP, LOGL_NOTICE, + "Channels != 1 in SDP: '%s' on 0x%x\n", + line, ENDPOINT_NUMBER(p->endp)); + } else if (sscanf(line, "a=ptime:%d-%d", + &ptime, &ptime2) >= 1) { + if (ptime2 > 0 && ptime2 != ptime) + rtp->packet_duration_ms = 0; + else + rtp->packet_duration_ms = ptime; + } else if (sscanf(line, "a=maxptime:%d", &ptime2) == 1) { + if (ptime2 * rtp->frame_duration_den > + rtp->frame_duration_num * 1500) + /* more than 1 frame */ + rtp->packet_duration_ms = 0; + } + break; + } case 'm': { int port; - int payload; + audio_payload = -1; if (sscanf(line, "m=audio %d RTP/AVP %d", - &port, &payload) == 2) { + &port, &audio_payload) == 2) { rtp->rtp_port = htons(port); rtp->rtcp_port = htons(port + 1); - rtp->payload_type = payload; + rtp->payload_type = audio_payload; found_media = 1; } break; @@ -608,8 +647,10 @@ static int parse_sdp_data(struct mgcp_rtp_end *rtp, struct mgcp_parse_data *p) if (found_media) LOGP(DMGCP, LOGL_NOTICE, - "Got media info via SDP: port %d, payload %d, addr %s\n", - ntohs(rtp->rtp_port), rtp->payload_type, inet_ntoa(rtp->addr)); + "Got media info via SDP: port %d, payload %d, " + "duration %d, addr %s\n", + ntohs(rtp->rtp_port), rtp->payload_type, + rtp->packet_duration_ms, inet_ntoa(rtp->addr)); return found_media; } diff --git a/openbsc/tests/mgcp/mgcp_test.ok b/openbsc/tests/mgcp/mgcp_test.ok index 24f9b336c..f9dd7cb62 100644 --- a/openbsc/tests/mgcp/mgcp_test.ok +++ b/openbsc/tests/mgcp/mgcp_test.ok @@ -16,22 +16,22 @@ Testing AUEP2 Testing MDCX1 Testing MDCX2 Testing CRCX -Packet duration not set +Detected packet duration: 40 Requested packetetization period: 20-20 Testing MDCX3 Packet duration not set Requested packetization period not set Testing MDCX4 -Packet duration not set +Detected packet duration: 40 Requested packetetization period: 20-20 Testing MDCX4_PT1 -Packet duration not set +Detected packet duration: 40 Requested packetetization period: 20-40 Testing MDCX4_PT2 -Packet duration not set +Detected packet duration: 40 Requested packetetization period: 20-20 Testing MDCX4_PT3 -Packet duration not set +Detected packet duration: 40 Requested packetization period not set Testing DLCX Detected packet duration: 20 -- cgit v1.2.3 From 0a1bc56e5a55393ed5294a0671cd8feab78f454b Mon Sep 17 00:00:00 2001 From: Jacob Erlbeck Date: Tue, 10 Dec 2013 13:09:37 +0100 Subject: mgcp: Optionally send ptime in SDP Currently the SDP 'ptime' media attribute is never set in generated MGCP responses. This patch optionally includes the 'ptime' attribute if packet_duration_ms is != 0. This behaviour can be enabled/disabled by using the VTY command "sdp audio-payload send-ptime" (enabled by default). Sponsored-by: On-Waves ehf --- openbsc/include/openbsc/mgcp.h | 1 + openbsc/src/libmgcp/mgcp_protocol.c | 23 ++++++++++++++++- openbsc/src/libmgcp/mgcp_vty.c | 50 +++++++++++++++++++++++++++++++++++++ openbsc/tests/mgcp/mgcp_test.c | 19 +++++++++++--- 4 files changed, 88 insertions(+), 5 deletions(-) diff --git a/openbsc/include/openbsc/mgcp.h b/openbsc/include/openbsc/mgcp.h index 8ab52ce1a..0d6459024 100644 --- a/openbsc/include/openbsc/mgcp.h +++ b/openbsc/include/openbsc/mgcp.h @@ -114,6 +114,7 @@ struct mgcp_trunk_config { char *audio_fmtp_extra; char *audio_name; int audio_payload; + int audio_send_ptime; int audio_loop; int omit_rtcp; diff --git a/openbsc/src/libmgcp/mgcp_protocol.c b/openbsc/src/libmgcp/mgcp_protocol.c index b8d192043..7d3ad74df 100644 --- a/openbsc/src/libmgcp/mgcp_protocol.c +++ b/openbsc/src/libmgcp/mgcp_protocol.c @@ -230,11 +230,12 @@ static struct msgb *create_response_with_sdp(struct mgcp_endpoint *endp, const char *addr = endp->cfg->local_ip; const char *fmtp_extra = endp->bts_end.fmtp_extra; char sdp_record[4096]; + int len; if (!addr) addr = endp->cfg->source_addr; - snprintf(sdp_record, sizeof(sdp_record) - 1, + len = snprintf(sdp_record, sizeof(sdp_record) - 1, "I: %u\n\n" "v=0\r\n" "o=- %u 23 IN IP4 %s\r\n" @@ -247,7 +248,25 @@ static struct msgb *create_response_with_sdp(struct mgcp_endpoint *endp, endp->net_end.local_port, endp->bts_end.payload_type, endp->bts_end.payload_type, endp->tcfg->audio_name, fmtp_extra ? fmtp_extra : "", fmtp_extra ? "\r\n" : ""); + + if (len < 0 || len >= sizeof(sdp_record)) + goto buffer_too_small; + + if (endp->bts_end.packet_duration_ms > 0 && endp->tcfg->audio_send_ptime) { + int nchars = snprintf(sdp_record + len, sizeof(sdp_record) - len, + "a=ptime:%d\r\n", + endp->bts_end.packet_duration_ms); + if (nchars < 0 || nchars >= sizeof(sdp_record) - len) + goto buffer_too_small; + + len += nchars; + } return create_resp(endp, 200, " OK", msg, trans_id, NULL, sdp_record); + +buffer_too_small: + LOGP(DMGCP, LOGL_ERROR, "SDP buffer too small: %d (needed %d)\n", + sizeof(sdp_record), len); + return NULL; } /* @@ -1109,6 +1128,7 @@ struct mgcp_config *mgcp_config_alloc(void) cfg->trunk.trunk_type = MGCP_TRUNK_VIRTUAL; cfg->trunk.audio_name = talloc_strdup(cfg, "AMR/8000"); cfg->trunk.audio_payload = 126; + cfg->trunk.audio_send_ptime = 1; cfg->trunk.omit_rtcp = 0; INIT_LLIST_HEAD(&cfg->trunks); @@ -1131,6 +1151,7 @@ struct mgcp_trunk_config *mgcp_trunk_alloc(struct mgcp_config *cfg, int nr) trunk->trunk_nr = nr; trunk->audio_name = talloc_strdup(cfg, "AMR/8000"); trunk->audio_payload = 126; + trunk->audio_send_ptime = 1; trunk->number_endpoints = 33; trunk->omit_rtcp = 0; llist_add_tail(&trunk->entry, &cfg->trunks); diff --git a/openbsc/src/libmgcp/mgcp_vty.c b/openbsc/src/libmgcp/mgcp_vty.c index e48b0503e..235b8bd6a 100644 --- a/openbsc/src/libmgcp/mgcp_vty.c +++ b/openbsc/src/libmgcp/mgcp_vty.c @@ -105,6 +105,8 @@ static int config_write_mgcp(struct vty *vty) if (g_cfg->trunk.audio_fmtp_extra) vty_out(vty, " sdp audio fmtp-extra %s%s", g_cfg->trunk.audio_fmtp_extra, VTY_NEWLINE); + vty_out(vty, " %ssdp audio-payload send-ptime%s", + g_cfg->trunk.audio_send_ptime ? "" : "no ", VTY_NEWLINE); vty_out(vty, " loop %u%s", !!g_cfg->trunk.audio_loop, VTY_NEWLINE); vty_out(vty, " number endpoints %u%s", g_cfg->trunk.number_endpoints - 1, VTY_NEWLINE); if (g_cfg->call_agent_addr) @@ -391,6 +393,26 @@ ALIAS_DEPRECATED(cfg_mgcp_sdp_payload_name, cfg_mgcp_sdp_payload_name_cmd_old, "sdp audio payload name NAME", SDP_STR AUDIO_STR AUDIO_STR "Name\n" "Payload name\n") +DEFUN(cfg_mgcp_sdp_payload_send_ptime, + cfg_mgcp_sdp_payload_send_ptime_cmd, + "sdp audio-payload send-ptime", + SDP_STR AUDIO_STR + "Send SDP ptime (packet duration) attribute\n") +{ + g_cfg->trunk.audio_send_ptime = 1; + return CMD_SUCCESS; +} + +DEFUN(cfg_mgcp_no_sdp_payload_send_ptime, + cfg_mgcp_no_sdp_payload_send_ptime_cmd, + "no sdp audio-payload send-ptime", + NO_STR SDP_STR AUDIO_STR + "Send SDP ptime (packet duration) attribute\n") +{ + g_cfg->trunk.audio_send_ptime = 0; + return CMD_SUCCESS; +} + DEFUN(cfg_mgcp_loop, cfg_mgcp_loop_cmd, "loop (0|1)", @@ -568,6 +590,8 @@ static int config_write_trunk(struct vty *vty) trunk->audio_payload, VTY_NEWLINE); vty_out(vty, " sdp audio-payload name %s%s", trunk->audio_name, VTY_NEWLINE); + vty_out(vty, " %ssdp audio-payload send-ptime%s", + trunk->audio_send_ptime ? "" : "no ", VTY_NEWLINE); vty_out(vty, " loop %d%s", trunk->audio_loop, VTY_NEWLINE); if (trunk->omit_rtcp) @@ -649,6 +673,28 @@ DEFUN(cfg_trunk_loop, return CMD_SUCCESS; } +DEFUN(cfg_trunk_sdp_payload_send_ptime, + cfg_trunk_sdp_payload_send_ptime_cmd, + "sdp audio-payload send-ptime", + SDP_STR AUDIO_STR + "Send SDP ptime (packet duration) attribute\n") +{ + struct mgcp_trunk_config *trunk = vty->index; + trunk->audio_send_ptime = 1; + return CMD_SUCCESS; +} + +DEFUN(cfg_trunk_no_sdp_payload_send_ptime, + cfg_trunk_no_sdp_payload_send_ptime_cmd, + "no sdp audio-payload send-ptime", + NO_STR SDP_STR AUDIO_STR + "Send SDP ptime (packet duration) attribute\n") +{ + struct mgcp_trunk_config *trunk = vty->index; + trunk->audio_send_ptime = 0; + return CMD_SUCCESS; +} + DEFUN(cfg_trunk_omit_rtcp, cfg_trunk_omit_rtcp_cmd, "rtcp-omit", @@ -966,6 +1012,8 @@ int mgcp_vty_init(void) install_element(MGCP_NODE, &cfg_mgcp_no_patch_rtp_ts_cmd); install_element(MGCP_NODE, &cfg_mgcp_no_patch_rtp_cmd); install_element(MGCP_NODE, &cfg_mgcp_sdp_fmtp_extra_cmd); + install_element(MGCP_NODE, &cfg_mgcp_sdp_payload_send_ptime_cmd); + install_element(MGCP_NODE, &cfg_mgcp_no_sdp_payload_send_ptime_cmd); install_element(MGCP_NODE, &cfg_mgcp_trunk_cmd); install_node(&trunk_node, config_write_trunk); @@ -983,6 +1031,8 @@ int mgcp_vty_init(void) install_element(TRUNK_NODE, &cfg_trunk_no_patch_rtp_ts_cmd); install_element(TRUNK_NODE, &cfg_trunk_no_patch_rtp_cmd); install_element(TRUNK_NODE, &cfg_trunk_sdp_fmtp_extra_cmd); + install_element(TRUNK_NODE, &cfg_trunk_sdp_payload_send_ptime_cmd); + install_element(TRUNK_NODE, &cfg_trunk_no_sdp_payload_send_ptime_cmd); return 0; } diff --git a/openbsc/tests/mgcp/mgcp_test.c b/openbsc/tests/mgcp/mgcp_test.c index 8b5e17ddc..9777a4df9 100644 --- a/openbsc/tests/mgcp/mgcp_test.c +++ b/openbsc/tests/mgcp/mgcp_test.c @@ -81,7 +81,8 @@ static void test_strline(void) "c=IN IP4 0.0.0.0\r\n" \ "t=0 0\r\n" \ "m=audio 0 RTP/AVP 126\r\n" \ - "a=rtpmap:126 AMR/8000\r\n" + "a=rtpmap:126 AMR/8000\r\n" \ + "a=ptime:20\r\n" #define MDCX4 "MDCX 18983216 1@mgw MGCP 1.0\r\n" \ "C: 2\r\n" \ "I: 1\r\n" \ @@ -102,7 +103,8 @@ static void test_strline(void) "c=IN IP4 0.0.0.0\r\n" \ "t=0 0\r\n" \ "m=audio 0 RTP/AVP 126\r\n" \ - "a=rtpmap:126 AMR/8000\r\n" + "a=rtpmap:126 AMR/8000\r\n" \ + "a=ptime:20\r\n" #define MDCX4_PT1 "MDCX 18983217 1@mgw MGCP 1.0\r\n" \ "C: 2\r\n" \ @@ -168,7 +170,8 @@ static void test_strline(void) "c=IN IP4 0.0.0.0\r\n" \ "t=0 0\r\n" \ "m=audio 0 RTP/AVP 126\r\n" \ - "a=rtpmap:126 AMR/8000\r\n" + "a=rtpmap:126 AMR/8000\r\n" \ + "a=ptime:20\r\n" #define CRCX_ZYN "CRCX 2 1@mgw MGCP 1.0\r" \ "M: sendrecv\r" \ @@ -186,7 +189,8 @@ static void test_strline(void) "c=IN IP4 0.0.0.0\r\n" \ "t=0 0\r\n" \ "m=audio 0 RTP/AVP 126\r\n" \ - "a=rtpmap:126 AMR/8000\r\n" + "a=rtpmap:126 AMR/8000\r\n" \ + "a=ptime:20\r\n" #define DLCX "DLCX 7 1@mgw MGCP 1.0\r\n" \ "C: 2\r\n" @@ -371,6 +375,13 @@ static void test_retransmission(void) mgcp_endpoints_allocate(mgcp_trunk_alloc(cfg, 1)); + /* reset endpoints */ + for (i = 0; i < cfg->trunk.number_endpoints; i++) { + struct mgcp_endpoint *endp; + endp = &cfg->trunk.endpoints[i]; + endp->bts_end.packet_duration_ms = 20; + } + for (i = 0; i < ARRAY_SIZE(retransmit); i++) { const struct mgcp_test *t = &retransmit[i]; struct msgb *inp; -- cgit v1.2.3