summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorHolger Hans Peter Freyther <holger@moiji-mobile.com>2014-06-27 19:27:38 +0200
committerHolger Hans Peter Freyther <holger@moiji-mobile.com>2014-07-22 14:42:53 +0200
commitbd4109babcc810c680a76a5bb990a4dbb5264a58 (patch)
tree123e70863cd9e265c1a230de437d2971ae53f77c
parent91eeeae312b3dfc54fc577b437c481811ff60d4a (diff)
mgcp: Document transcoding semantic and follow it
Transcoding from GSM to PCMA can lead to the MGCP MGW sending two PCMA packages with the same sequence number and timestamp. Once with the encoded audio and once completely empty. This is because "state->dst_packet_duration" is 0 in most cases (unless a ptime is forced) and we attempt to encode audio even if there are not enough samples. The encode_audio return will return 0 in that case which is not trated as an error by the mgcp network code. Handle rc == 0 specially and document the semantic.
-rw-r--r--openbsc/include/openbsc/mgcp.h6
-rw-r--r--openbsc/src/libmgcp/mgcp_transcode.c9
-rw-r--r--openbsc/tests/mgcp/mgcp_transcoding_test.c84
-rw-r--r--openbsc/tests/mgcp/mgcp_transcoding_test.ok12
4 files changed, 95 insertions, 16 deletions
diff --git a/openbsc/include/openbsc/mgcp.h b/openbsc/include/openbsc/mgcp.h
index d4d614099..1790f8431 100644
--- a/openbsc/include/openbsc/mgcp.h
+++ b/openbsc/include/openbsc/mgcp.h
@@ -87,6 +87,12 @@ typedef int (*mgcp_policy)(struct mgcp_trunk_config *cfg, int endpoint, int stat
typedef int (*mgcp_reset)(struct mgcp_trunk_config *cfg);
typedef int (*mgcp_rqnt)(struct mgcp_endpoint *endp, char tone);
+/**
+ * Return:
+ * < 0 in case no audio was processed
+ * >= 0 in case audio was processed. The remaining payload
+ * length will be returned.
+ */
typedef int (*mgcp_processing)(struct mgcp_endpoint *endp,
struct mgcp_rtp_end *dst_end,
char *data, int *len, int buf_size);
diff --git a/openbsc/src/libmgcp/mgcp_transcode.c b/openbsc/src/libmgcp/mgcp_transcode.c
index 296020c49..ff20fb891 100644
--- a/openbsc/src/libmgcp/mgcp_transcode.c
+++ b/openbsc/src/libmgcp/mgcp_transcode.c
@@ -488,7 +488,14 @@ int mgcp_transcoding_process_rtp(struct mgcp_endpoint *endp,
nsamples = state->sample_cnt;
rc = encode_audio(state, dst, buf_size, max_samples);
- if (rc <= 0)
+ /*
+ * There were no samples to encode?
+ * TODO: how does this work for comfort noise?
+ */
+ if (rc == 0)
+ return -ENOMSG;
+ /* Any other error during the encoding */
+ if (rc < 0)
return rc;
nsamples -= state->sample_cnt;
diff --git a/openbsc/tests/mgcp/mgcp_transcoding_test.c b/openbsc/tests/mgcp/mgcp_transcoding_test.c
index 404268a68..27d72692d 100644
--- a/openbsc/tests/mgcp/mgcp_transcoding_test.c
+++ b/openbsc/tests/mgcp/mgcp_transcoding_test.c
@@ -120,6 +120,24 @@ struct rtp_packets audio_packets_pcma[] = {
"\xD5\xA5\xA3\xA5\xD5\x25\x23\x25\xD5\xA5\xA3\xA5\xD5\x25\x23\x25"
"\xD5\xA5\xA3\xA5\xD5\x25\x23\x25\xD5\xA5\xA3\xA5\xD5\x25\x23\x25"
},
+ /* RTP: SeqNo=26527, TS=0 */
+ {0.020000, 92,
+ "\x80\x08\x67\x9f\x00\x00\x00\x00\x04\xaa\x67\x9f\xd5\xd5\xd5\xd5"
+ "\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5"
+ "\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5"
+ "\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5\xd5"
+ "\xd5\xd5\xd5\xd5\xd5\xd5\x55\x55\xd5\xd5\x55\x55\xd5\xd5\x55\x55"
+ "\xd5\xd5\xd5\x55\x55\xd5\xd5\xd5\x55\x55\xd5\xd5"
+ },
+ /* RTP: SeqNo=26528, TS=80 */
+ {0.020000, 92,
+ "\x80\x08\x67\xa0\x00\x00\x00\x50\x04\xaa\x67\x9f\x55\xd5\xd5\x55"
+ "\xd5\x55\xd5\xd5\xd5\x55\xd5\x55\xd5\xd5\x55\xd5\x55\xd5\x55\xd5"
+ "\x55\x55\xd5\x55\xd5\xd5\x55\x55\x55\x55\x55\xd5\xd5\x55\xd5\xd5"
+ "\xd5\x55\xd5\xd5\xd5\x55\x54\x55\xd5\xd5\x55\xd5\xd5\xd5\xd5\x55"
+ "\x54\x55\xd5\x55\xd5\x55\x55\x55\x55\x55\xd5\xd5\xd5\xd5\xd5\xd4"
+ "\xd5\x54\x55\xd5\xd4\xd5\x54\xd5\x55\xd5\xd5\xd5"
+ },
};
@@ -221,8 +239,9 @@ static int transcode_test(const char *srcfmt, const char *dstfmt,
cont = mgcp_transcoding_process_rtp(endp, dst_end,
buf, &len, sizeof(buf));
if (cont < 0) {
- printf("processing failed: %s", strerror(-cont));
- abort();
+ printf("Nothing encoded due: %s\n", strerror(-cont));
+ talloc_free(ctx);
+ return -1;
}
if (len < 24) {
@@ -296,6 +315,56 @@ static void test_rtp_seq_state(void)
talloc_free(ctx);
}
+static void test_transcode_result(void)
+{
+ char buf[4096];
+ int len, res;
+ void *ctx;
+ struct mgcp_endpoint *endp;
+ struct mgcp_process_rtp_state *state;
+
+ {
+ /* from GSM to PCMA and same ptime */
+ given_configured_endpoint(160, 0, "gsm", "pcma", &ctx, &endp);
+ state = endp->bts_end.rtp_process_data;
+
+ /* result */
+ len = audio_packets_gsm[0].len;
+ memcpy(buf, audio_packets_gsm[0].data, len);
+ res = mgcp_transcoding_process_rtp(endp, &endp->bts_end, buf, &len, ARRAY_SIZE(buf));
+ OSMO_ASSERT(res == sizeof(struct rtp_hdr));
+ OSMO_ASSERT(state->sample_cnt == 0);
+
+ len = res;
+ res = mgcp_transcoding_process_rtp(endp, &endp->bts_end, buf, &len, ARRAY_SIZE(buf));
+ OSMO_ASSERT(res == -ENOMSG);
+
+ talloc_free(ctx);
+ }
+
+ {
+ /* from PCMA to GSM and wrong different ptime */
+ given_configured_endpoint(80, 160, "pcma", "gsm", &ctx, &endp);
+ state = endp->bts_end.rtp_process_data;
+
+ /* Add the first sample */
+ len = audio_packets_pcma[1].len;
+ memcpy(buf, audio_packets_pcma[1].data, len);
+ res = mgcp_transcoding_process_rtp(endp, &endp->bts_end, buf, &len, ARRAY_SIZE(buf));
+ OSMO_ASSERT(state->sample_cnt == 80);
+ OSMO_ASSERT(res < 0);
+
+ /* Add the second sample and it should be consumable */
+ len = audio_packets_pcma[2].len;
+ memcpy(buf, audio_packets_pcma[2].data, len);
+ res = mgcp_transcoding_process_rtp(endp, &endp->bts_end, buf, &len, ARRAY_SIZE(buf));
+ OSMO_ASSERT(state->sample_cnt == 0);
+ OSMO_ASSERT(res == sizeof(struct rtp_hdr));
+
+ talloc_free(ctx);
+ }
+}
+
static int test_repacking(int in_samples, int out_samples, int no_transcode)
{
char buf[4096] = {0x80, 0};
@@ -378,6 +447,7 @@ static int test_repacking(int in_samples, int out_samples, int no_transcode)
int main(int argc, char **argv)
{
+ int rc;
osmo_init_logging(&log_info);
printf("=== Transcoding Good Cases ===\n");
@@ -413,19 +483,22 @@ int main(int argc, char **argv)
printf("=== Transcoding Bad Cases ===\n");
printf("Invalid size:\n");
- transcode_test("gsm", "pcma",
+ rc = transcode_test("gsm", "pcma",
(uint8_t *)audio_packets_gsm_invalid_size[0].data,
audio_packets_gsm_invalid_size[0].len);
+ OSMO_ASSERT(rc < 0);
printf("Invalid data:\n");
- transcode_test("gsm", "pcma",
+ rc = transcode_test("gsm", "pcma",
(uint8_t *)audio_packets_gsm_invalid_data[0].data,
audio_packets_gsm_invalid_data[0].len);
+ OSMO_ASSERT(rc < 0);
printf("Invalid payload type:\n");
- transcode_test("gsm", "pcma",
+ rc = transcode_test("gsm", "pcma",
(uint8_t *)audio_packets_gsm_invalid_ptype[0].data,
audio_packets_gsm_invalid_ptype[0].len);
+ OSMO_ASSERT(rc == 0);
printf("=== Repacking ===\n");
@@ -440,6 +513,7 @@ int main(int argc, char **argv)
test_repacking(160, 100, 0);
test_repacking(160, 100, 1);
test_rtp_seq_state();
+ test_transcode_result();
return 0;
}
diff --git a/openbsc/tests/mgcp/mgcp_transcoding_test.ok b/openbsc/tests/mgcp/mgcp_transcoding_test.ok
index e06b0e1bc..7c1c8cebd 100644
--- a/openbsc/tests/mgcp/mgcp_transcoding_test.ok
+++ b/openbsc/tests/mgcp/mgcp_transcoding_test.ok
@@ -144,19 +144,11 @@ counted: 0
Invalid size:
== Transcoding test ==
converting gsm -> pcma
-encoded:
- 80 03 00 01 00 00 00 a0 11 22 33 44 d4 7c e3 e9
- 62 50 39 f0 f8 b4 68 ea 6c 0e 81 1b 56 2a d5 bc
- 69 9c d1 f0 66 7a ec 49 7a
-counted: 0
+Nothing encoded due: No message of desired type
Invalid data:
== Transcoding test ==
converting gsm -> pcma
-encoded:
- 80 03 00 01 00 00 00 a0 11 22 33 44 ee ee ee ee
- ee ee ee ee ee ee ee ee ee ee ee ee ee ee ee ee
- ee ee ee ee ee ee ee ee ee ee ee ee ee
-counted: 0
+Nothing encoded due: No message of desired type
Invalid payload type:
== Transcoding test ==
converting gsm -> pcma