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/.
pespin gerrit-no-reply at lists.osmocom.orgpespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/24872 ) Change subject: Take into account Marker bit when patching RTP stream ...................................................................... Take into account Marker bit when patching RTP stream On a deployed osmo-mgw with RTP traffic coming from a thirdparty RTP source, it was usual to see log messages like following one from time to time: "The input timestamp has an alignment error of 159 on SSRC" Doing a quick traffic analysis showed that the above mentioned RTP source was generating traffic from time to time containing RTP packets with the Marker (M) bit. Those messages were logged because the verification & patching funcions in osmo-mgw were not Marker-bit aware. Hence, this patch implements support for Marker bit when handling RTP packets. The Marker bit is usually used as a start of a talkspurt, and has to be considered a syncrhonization point, where timestamp and relation to real time don't need to match with last received RTP packet in the stream. Related: SYS#5498 Change-Id: I1fb449eda49e82607649122b9b9d983a9e5983fa --- M include/osmocom/mgcp/mgcp_network.h M src/libosmo-mgcp/mgcp_network.c M tests/mgcp/mgcp_test.c M tests/mgcp/mgcp_test.ok 4 files changed, 90 insertions(+), 20 deletions(-) Approvals: dexter: Looks good to me, approved Jenkins Builder: Verified diff --git a/include/osmocom/mgcp/mgcp_network.h b/include/osmocom/mgcp/mgcp_network.h index 75b342d..d6d90dd 100644 --- a/include/osmocom/mgcp/mgcp_network.h +++ b/include/osmocom/mgcp/mgcp_network.h @@ -176,4 +176,4 @@ /* internal RTP Annex A counting */ void mgcp_rtp_annex_count(const struct mgcp_endpoint *endp, struct mgcp_rtp_state *state, const uint16_t seq, const int32_t transit, - const uint32_t ssrc); + const uint32_t ssrc, const bool marker_bit); diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c index d18c7cb..82dc6ec 100644 --- a/src/libosmo-mgcp/mgcp_network.c +++ b/src/libosmo-mgcp/mgcp_network.c @@ -285,13 +285,21 @@ struct mgcp_rtp_state *state, const struct mgcp_rtp_end *rtp_end, const struct osmo_sockaddr *addr, - int16_t delta_seq, uint32_t in_timestamp) + int16_t delta_seq, uint32_t in_timestamp, + bool marker_bit) { int32_t tsdelta = state->packet_duration; int timestamp_offset; uint32_t out_timestamp; char ipbuf[INET6_ADDRSTRLEN]; + if (marker_bit) { + /* If RTP pkt contains marker bit, the timestamps are not longer + * in sync, so we can erase timestamp offset patching. */ + state->patch.timestamp_offset = 0; + return 0; + } + if (tsdelta == 0) { tsdelta = state->out_stream.last_tsdelta; if (tsdelta != 0) { @@ -335,13 +343,19 @@ struct mgcp_rtp_state *state, const struct mgcp_rtp_end *rtp_end, const struct osmo_sockaddr *addr, - uint32_t timestamp) + uint32_t timestamp, bool marker_bit) { char ipbuf[INET6_ADDRSTRLEN]; int ts_error = 0; int ts_check = 0; int ptime = state->packet_duration; + if (marker_bit) { + /* If RTP pkt contains marker bit, the timestamps are not longer + * in sync, so no alignment is needed. */ + return 0; + } + /* Align according to: T + Toffs - Tlast = k * Tptime */ ts_error = ts_alignment_error(&state->out_stream, ptime, @@ -414,12 +428,13 @@ void mgcp_rtp_annex_count(const struct mgcp_endpoint *endp, struct mgcp_rtp_state *state, const uint16_t seq, - const int32_t transit, const uint32_t ssrc) + const int32_t transit, const uint32_t ssrc, + const bool marker_bit) { int32_t d; /* initialize or re-initialize */ - if (!state->stats.initialized || state->stats.ssrc != ssrc) { + if (!state->stats.initialized || state->stats.ssrc != ssrc || marker_bit) { state->stats.initialized = 1; state->stats.base_seq = seq; state->stats.max_seq = seq - 1; @@ -503,6 +518,7 @@ int32_t transit; uint16_t seq; uint32_t timestamp, ssrc; + bool marker_bit; struct rtp_hdr *rtp_hdr; int payload = rtp_end->codec->payload_type; unsigned int len = msgb_length(msg); @@ -515,9 +531,10 @@ timestamp = ntohl(rtp_hdr->timestamp); arrival_time = get_current_ts(rtp_end->codec->rate); ssrc = ntohl(rtp_hdr->ssrc); + marker_bit = !!rtp_hdr->marker; transit = arrival_time - timestamp; - mgcp_rtp_annex_count(endp, state, seq, transit, ssrc); + mgcp_rtp_annex_count(endp, state, seq, transit, ssrc, marker_bit); if (!state->initialized) { state->initialized = 1; @@ -571,7 +588,7 @@ state->packet_duration; adjust_rtp_timestamp_offset(endp, state, rtp_end, addr, - delta_seq, timestamp); + delta_seq, timestamp, marker_bit); state->patch.patch_ssrc = true; ssrc = state->patch.orig_ssrc; @@ -589,10 +606,14 @@ state->in_stream.last_tsdelta = 0; } else { - /* Compute current per-packet timestamp delta */ - check_rtp_timestamp(endp, state, &state->in_stream, rtp_end, - addr, seq, timestamp, "input", - &state->in_stream.last_tsdelta); + if (!marker_bit) { + /* Compute current per-packet timestamp delta */ + check_rtp_timestamp(endp, state, &state->in_stream, rtp_end, + addr, seq, timestamp, "input", + &state->in_stream.last_tsdelta); + } else { + state->in_stream.last_tsdelta = 0; + } if (state->patch.patch_ssrc) ssrc = state->patch.orig_ssrc; @@ -607,7 +628,7 @@ state->out_stream.ssrc == ssrc && state->packet_duration) /* Align the timestamp offset */ align_rtp_timestamp_offset(endp, state, rtp_end, addr, - timestamp); + timestamp, marker_bit); /* Store the updated SSRC back to the packet */ if (state->patch.patch_ssrc) @@ -622,10 +643,14 @@ rtp_hdr->timestamp = htonl(timestamp); /* Check again, whether the timestamps are still valid */ - if (state->out_stream.ssrc == ssrc) - check_rtp_timestamp(endp, state, &state->out_stream, rtp_end, - addr, seq, timestamp, "output", - &state->out_stream.last_tsdelta); + if (!marker_bit) { + if (state->out_stream.ssrc == ssrc) + check_rtp_timestamp(endp, state, &state->out_stream, rtp_end, + addr, seq, timestamp, "output", + &state->out_stream.last_tsdelta); + } else { + state->out_stream.last_tsdelta = 0; + } /* Save output values */ state->out_stream.last_seq = seq; diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c index 6978b1d..7397f5c 100644 --- a/tests/mgcp/mgcp_test.c +++ b/tests/mgcp/mgcp_test.c @@ -1282,6 +1282,15 @@ /* RTP: SeqNo=1002, TS=160320 */ {2.040000, 20, "\x80\x62\x03\xEA\x00\x02\x72\x40\x50\x60\x70\x80" "\x01\x23\x45\x67\x89\xAB\xCD\xEF"}, + /* RTP: SeqNo=1003, TS=180320, Marker */ + {2.060000, 20, "\x80\xE2\x03\xEB\x00\x02\xC0\x60\x50\x60\x70\x80" + "\x01\x23\x45\x67\x89\xAB\xCD\xEF"}, + /* RTP: SeqNo=1004, TS=180480 */ + {2.080000, 20, "\x80\x62\x03\xEC\x00\x02\xC1\x00\x50\x60\x70\x80" + "\x01\x23\x45\x67\x89\xAB\xCD\xEF"}, + /* RTP: SeqNo=1005, TS=180480, 10ms too late */ + {2.110000, 20, "\x80\x62\x03\xED\x00\x02\xC1\xA0\x50\x60\x70\x80" + "\x01\x23\x45\x67\x89\xAB\xCD\xEF"}, }; static void test_packet_error_detection(int patch_ssrc, int patch_ts) @@ -1566,24 +1575,24 @@ OSMO_ASSERT(conn->state.stats.initialized == 0); - mgcp_rtp_annex_count(endp, &conn->state, 0, 0, 2342); + mgcp_rtp_annex_count(endp, &conn->state, 0, 0, 2342, false); OSMO_ASSERT(conn->state.stats.initialized == 1); OSMO_ASSERT(conn->state.stats.cycles == 0); OSMO_ASSERT(conn->state.stats.max_seq == 0); - mgcp_rtp_annex_count(endp, &conn->state, 1, 0, 2342); + mgcp_rtp_annex_count(endp, &conn->state, 1, 0, 2342, false); OSMO_ASSERT(conn->state.stats.initialized == 1); OSMO_ASSERT(conn->state.stats.cycles == 0); OSMO_ASSERT(conn->state.stats.max_seq == 1); /* now jump.. */ - mgcp_rtp_annex_count(endp, &conn->state, UINT16_MAX, 0, 2342); + mgcp_rtp_annex_count(endp, &conn->state, UINT16_MAX, 0, 2342, false); OSMO_ASSERT(conn->state.stats.initialized == 1); OSMO_ASSERT(conn->state.stats.cycles == 0); OSMO_ASSERT(conn->state.stats.max_seq == UINT16_MAX); /* and wrap */ - mgcp_rtp_annex_count(endp, &conn->state, 0, 0, 2342); + mgcp_rtp_annex_count(endp, &conn->state, 0, 0, 2342, false); OSMO_ASSERT(conn->state.stats.initialized == 1); OSMO_ASSERT(conn->state.stats.cycles == UINT16_MAX + 1); OSMO_ASSERT(conn->state.stats.max_seq == 0); diff --git a/tests/mgcp/mgcp_test.ok b/tests/mgcp/mgcp_test.ok index 575fd83..94fada3 100644 --- a/tests/mgcp/mgcp_test.ok +++ b/tests/mgcp/mgcp_test.ok @@ -792,6 +792,15 @@ In TS: 160320, dTS: 160, Seq: 1002 Out TS change: 160, dTS: 160, Seq change: 1, TS Err change: in +0, out +0 Stats: Jitter = 0, Transit = -144000 +In TS: 180320, dTS: 0, Seq: 1003 +Out TS change: 20000, dTS: 0, Seq change: 1, TS Err change: in +0, out +0 +Stats: Jitter = 0, Transit = -163840 +In TS: 180480, dTS: 160, Seq: 1004 +Out TS change: 160, dTS: 160, Seq change: 1, TS Err change: in +0, out +0 +Stats: Jitter = 0, Transit = -163840 +In TS: 180640, dTS: 160, Seq: 1005 +Out TS change: 160, dTS: 160, Seq change: 1, TS Err change: in +0, out +0 +Stats: Jitter = 5, Transit = -163760 Testing packet error detection. Output SSRC changed to 11223344 In TS: 0, dTS: 0, Seq: 0 @@ -886,6 +895,15 @@ In TS: 160320, dTS: 160, Seq: 1002 Out TS change: 160, dTS: 160, Seq change: 1, TS Err change: in +0, out +0 Stats: Jitter = 0, Transit = -144000 +In TS: 180320, dTS: 0, Seq: 1003 +Out TS change: 20000, dTS: 0, Seq change: 1, TS Err change: in +0, out +0 +Stats: Jitter = 0, Transit = -163840 +In TS: 180480, dTS: 160, Seq: 1004 +Out TS change: 160, dTS: 160, Seq change: 1, TS Err change: in +0, out +0 +Stats: Jitter = 0, Transit = -163840 +In TS: 180640, dTS: 160, Seq: 1005 +Out TS change: 160, dTS: 160, Seq change: 1, TS Err change: in +0, out +0 +Stats: Jitter = 5, Transit = -163760 Testing packet error detection, patch timestamps. Output SSRC changed to 11223344 In TS: 0, dTS: 0, Seq: 0 @@ -980,6 +998,15 @@ In TS: 160320, dTS: 160, Seq: 1002 Out TS change: 160, dTS: 160, Seq change: 1, TS Err change: in +0, out +0 Stats: Jitter = 0, Transit = -144000 +In TS: 180320, dTS: 0, Seq: 1003 +Out TS change: 20000, dTS: 0, Seq change: 1, TS Err change: in +0, out +0 +Stats: Jitter = 0, Transit = -163840 +In TS: 180480, dTS: 160, Seq: 1004 +Out TS change: 160, dTS: 160, Seq change: 1, TS Err change: in +0, out +0 +Stats: Jitter = 0, Transit = -163840 +In TS: 180640, dTS: 160, Seq: 1005 +Out TS change: 160, dTS: 160, Seq change: 1, TS Err change: in +0, out +0 +Stats: Jitter = 5, Transit = -163760 Testing packet error detection, patch SSRC, patch timestamps. Output SSRC changed to 11223344 In TS: 0, dTS: 0, Seq: 0 @@ -1072,6 +1099,15 @@ In TS: 160320, dTS: 160, Seq: 1002 Out TS change: 160, dTS: 160, Seq change: 1, TS Err change: in +0, out +0 Stats: Jitter = 0, Transit = -144000 +In TS: 180320, dTS: 0, Seq: 1003 +Out TS change: 20000, dTS: 0, Seq change: 1, TS Err change: in +0, out +0 +Stats: Jitter = 0, Transit = -163840 +In TS: 180480, dTS: 160, Seq: 1004 +Out TS change: 160, dTS: 160, Seq change: 1, TS Err change: in +0, out +0 +Stats: Jitter = 0, Transit = -163840 +In TS: 180640, dTS: 160, Seq: 1005 +Out TS change: 160, dTS: 160, Seq change: 1, TS Err change: in +0, out +0 +Stats: Jitter = 5, Transit = -163760 Testing multiple payload types creating message from statically defined input: ---------8<--------- -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/24872 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I1fb449eda49e82607649122b9b9d983a9e5983fa Gerrit-Change-Number: 24872 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin at sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier at sysmocom.de> Gerrit-Reviewer: laforge <laforge at osmocom.org> 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/20210708/29acfb2f/attachment.htm>