From 785ed90bba8ee8482a7da037fef7adf350bd79be Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Tue, 12 Oct 2021 14:50:04 -0400 Subject: [PATCH] TT#14008 eliminate duplicate packets_lost counters This solves inconsistent stat output Change-Id: Ic4c010fbceb83d1f8d5fffecacb3c65a436bfeae --- daemon/call.c | 4 ++-- daemon/codec.c | 4 ++-- daemon/media_socket.c | 2 +- daemon/mqtt.c | 4 ++-- daemon/rtcp.c | 6 +++--- include/ssrc.h | 7 ++++--- kernel-module/xt_RTPENGINE.c | 9 ++++++--- 7 files changed, 20 insertions(+), 16 deletions(-) diff --git a/daemon/call.c b/daemon/call.c index a58c32caa..7647d06f2 100644 --- a/daemon/call.c +++ b/daemon/call.c @@ -3237,7 +3237,7 @@ void call_destroy(struct call *c) { ilog(LOG_INFO, "--- SSRC %s%" PRIx32 "%s", FMT_M(se->h.ssrc)); ilog(LOG_INFO, "------ Average MOS %" PRIu64 ".%" PRIu64 ", " "lowest MOS %" PRIu64 ".%" PRIu64 " (at %u:%02u), " - "highest MOS %" PRIu64 ".%" PRIu64 " (at %u:%02u) lost:%d", + "highest MOS %" PRIu64 ".%" PRIu64 " (at %u:%02u) lost:%u", se->average_mos.mos / mos_samples / 10, se->average_mos.mos / mos_samples % 10, se->lowest_mos->mos / 10, @@ -3248,7 +3248,7 @@ void call_destroy(struct call *c) { se->highest_mos->mos % 10, (unsigned int) (timeval_diff(&se->highest_mos->reported, &c->created) / 1000000) / 60, (unsigned int) (timeval_diff(&se->highest_mos->reported, &c->created) / 1000000) % 60, - se->packets_lost); + (unsigned int) se->packets_lost); next_k: k = g_list_delete_link(k, k); } diff --git a/daemon/codec.c b/daemon/codec.c index 3695adedc..ab75d6d57 100644 --- a/daemon/codec.c +++ b/daemon/codec.c @@ -1428,7 +1428,7 @@ static int __handler_func_sequencer(struct media_packet *mp, struct transcode_pa else ilogs(transcoding, LOG_DEBUG, "Ignoring duplicate RTP packet"); __transcode_packet_free(packet); - atomic64_inc(&ssrc_in->duplicates); + ssrc_in_p->duplicates++; goto out; } @@ -1475,7 +1475,7 @@ static int __handler_func_sequencer(struct media_packet *mp, struct transcode_pa goto next; } - atomic64_set(&ssrc_in->packets_lost, ssrc_in_p->sequencer.lost_count); + ssrc_in_p->packets_lost = ssrc_in_p->sequencer.lost_count; atomic64_set(&ssrc_in->last_seq, ssrc_in_p->sequencer.ext_seq); ilogs(transcoding, LOG_DEBUG, "Processing RTP packet: seq %u, TS %lu", diff --git a/daemon/media_socket.c b/daemon/media_socket.c index 89cea61ea..d13e87b98 100644 --- a/daemon/media_socket.c +++ b/daemon/media_socket.c @@ -1408,7 +1408,7 @@ static void __stream_update_stats(struct packet_stream *ps, int have_in_lock) { atomic64_add(&ssrc_ctx->packets, stats.basic_stats.packets); atomic64_add(&ssrc_ctx->octets, stats.basic_stats.bytes); - atomic64_add(&ssrc_ctx->packets_lost, stats.total_lost); + parent->packets_lost += stats.total_lost; // XXX should be atomic? atomic64_set(&ssrc_ctx->last_seq, stats.ext_seq); atomic64_set(&ssrc_ctx->last_ts, stats.timestamp); parent->jitter = stats.jitter; diff --git a/daemon/mqtt.c b/daemon/mqtt.c index 867bd84f3..3f3cc50e4 100644 --- a/daemon/mqtt.c +++ b/daemon/mqtt.c @@ -189,8 +189,8 @@ static void mqtt_ssrc_stats(struct ssrc_ctx *ssrc, JsonBuilder *json, struct cal uint64_t packets, octets, packets_lost, duplicates; packets = atomic64_get(&ssrc->packets); octets = atomic64_get(&ssrc->octets); - packets_lost = atomic64_get(&ssrc->packets_lost); - duplicates = atomic64_get(&ssrc->duplicates); + packets_lost = sc->packets_lost; + duplicates = sc->duplicates; // process per-second stats uint64_t cur_ts = ssrc_timeval_to_ts(&rtpe_now); diff --git a/daemon/rtcp.c b/daemon/rtcp.c index d17369e1c..4a431eaf1 100644 --- a/daemon/rtcp.c +++ b/daemon/rtcp.c @@ -1332,8 +1332,8 @@ static void transcode_rr(struct rtcp_process_ctx *ctx, struct report_block *rr) if (!packets) goto out; - unsigned int lost = atomic64_get(&input_ctx->packets_lost); - unsigned int dupes = atomic64_get(&input_ctx->duplicates); + unsigned int lost = input_ctx->parent->packets_lost; + unsigned int dupes = input_ctx->parent->duplicates; unsigned int tot_lost = lost - dupes; // can be negative/rollover ilogs(rtcp, LOG_DEBUG, "Substituting RTCP RR SSRC from %s%x%s to %x: %u packets, %u lost, %u duplicates", @@ -1467,7 +1467,7 @@ static GString *rtcp_sender_report(struct ssrc_sender_report *ssr, uint32_t jitter = se->jitter; mutex_unlock(&se->h.lock); - uint64_t lost = atomic64_get(&s->packets_lost); + uint64_t lost = se->packets_lost; uint64_t tot = atomic64_get(&s->packets); *rr = (struct report_block) { diff --git a/include/ssrc.h b/include/ssrc.h index b6f8b478f..b06771bbe 100644 --- a/include/ssrc.h +++ b/include/ssrc.h @@ -59,8 +59,6 @@ struct ssrc_ctx { // RTCP stats atomic64 packets, octets, - packets_lost, - duplicates, last_seq, // XXX dup with srtp_index? last_ts; @@ -109,10 +107,13 @@ struct ssrc_entry_call { *highest_mos, average_mos; // contains a running tally of all stats blocks uint16_t no_mos_count; // how many time we where not able to compute MOS due to missing RTT - uint32_t packets_lost; // RTCP cumulative number of packets lost unsigned int last_rtt; // last calculated raw rtt without rtt from opposide side unsigned int last_rtt_xr; // last rtt for both legs retreived from RTCP-XR BT-7 + // input only + uint32_t packets_lost; // RTCP cumulative number of packets lost + uint32_t duplicates; + // for transcoding // input only packet_sequencer_t sequencer; diff --git a/kernel-module/xt_RTPENGINE.c b/kernel-module/xt_RTPENGINE.c index 4fcacc7c4..25b89b46c 100644 --- a/kernel-module/xt_RTPENGINE.c +++ b/kernel-module/xt_RTPENGINE.c @@ -4257,6 +4257,7 @@ static void rtp_stats(struct rtpengine_target *g, struct rtp_parsed *rtp, s64 ar uint32_t clockrate; uint32_t transit; int32_t d; + uint32_t new_seq; uint16_t seq = ntohs(rtp->header->seq_num); uint32_t ts = ntohl(rtp->header->timestamp); @@ -4270,6 +4271,7 @@ static void rtp_stats(struct rtpengine_target *g, struct rtp_parsed *rtp, s64 ar // track sequence numbers and lost frames last_seq = s->ext_seq; + new_seq = last_seq; // old seq or seq reset? old_seq_trunc = last_seq & 0xffff; @@ -4278,18 +4280,19 @@ static void rtp_stats(struct rtpengine_target *g, struct rtp_parsed *rtp, s64 ar ; else if (seq_diff > 0x100) { // reset seq and loss tracker + new_seq = seq; s->ext_seq = seq; s->lost_bits = -1; } else { // seq wrap? - uint32_t new_seq = (last_seq & 0xffff0000) | seq; + new_seq = (last_seq & 0xffff0000) | seq; while (new_seq < last_seq) { new_seq += 0x10000; if ((new_seq & 0xffff0000) == 0) // ext seq wrapped break; } - seq_diff = new_seq - s->ext_seq; + seq_diff = new_seq - last_seq; s->ext_seq = new_seq; // shift loss tracker bit field and count losses @@ -4310,7 +4313,7 @@ static void rtp_stats(struct rtpengine_target *g, struct rtp_parsed *rtp, s64 ar } // track this frame as being seen - seq_diff = (s->ext_seq & 0xffff) - seq; + seq_diff = (new_seq & 0xffff) - seq; if (seq_diff < (sizeof(s->lost_bits) * 8)) s->lost_bits |= (1 << seq_diff);