Merge pull request #145 from wireapp/fix/sifa/WPB-24049-tune-rx-jitter

Tune stats rx jitter (WPB 24049

Author: sifa-wire

src/ccall/ccall.c

@@ -526,10 +526,7 @@ static void ccall_reconnect(struct ccall *ccall,
 
 	if (notify) {
 
-		struct stats_report stats = {
-			.packets.lost.rx = ICALL_RECONNECTING,
-			.packets.lost.tx = ICALL_RECONNECTING,
-		};
+		struct stats_report stats = { 0 };
 
 		ICALL_CALL_CB(ccall->icall, qualityh,
 			      &ccall->icall, 

src/egcall/egcall.c

@@ -1029,10 +1029,7 @@ static void noconn_handler(void *arg)
 {
 	struct egcall *egcall = arg;
 
-	struct stats_report stats = {
-		.packets.lost.rx = ICALL_NETWORK_PROBLEM,
-		.packets.lost.tx = ICALL_NETWORK_PROBLEM,
-	};
+	struct stats_report stats = { 0 };
 
 	ICALL_CALL_CB(egcall->icall, qualityh,
 		      &egcall->icall,

src/jzon/jzon.c

@@ -162,10 +162,21 @@ int jzon_double(double *dst, struct json_object *obj, const char *key)
 	if (!json_object_object_get_ex(obj, key, &value))
 		return ENOENT;
 	type = json_object_get_type(value);
-	if (type != json_type_double && type != json_type_int)
+
+	switch (type) {
+	case json_type_double:
+		*dst = json_object_get_double(value);
+		break;
+
+	case json_type_int:
+		*dst = (double)(json_object_get_int(value));
+		break;
+
+	default:
 		return EPROTO;
 
-	*dst = json_object_get_double(value);
+	}
+
 	return 0;
 }
 

src/stats/stats.c

@@ -222,6 +222,10 @@ static uint32_t calculate_loss_percentage(uint32_t packets, uint32_t lost) {
 static int read_packet_stats_and_jitter(struct avs_stats *stats, struct stats_obj* stats_obj)
 {
 	struct le *le = NULL;
+	double audio_jitter = 0;
+	int aj_count = 0;
+	double video_jitter = 0;
+	int vj_count = 0;
 
 	if (!stats || !stats_obj) {
 		return EINVAL;
@@ -231,6 +235,8 @@ static int read_packet_stats_and_jitter(struct avs_stats *stats, struct stats_ob
 	  Calculation of packet and loss statistics
 	  1. read json stats into report
 	      webrtc json -> stats.report
+	    1.1 rx jitter calculation is done wrt mean of incoming rtps
+	        that have nonzero received packets.
 	  2. calculate interval percentage for packet loss into tmp variables
 	      loss_tx = calculate_loss_percentage(...)
 	      loss_rx = calculate_loss_percentage(...)
@@ -245,11 +251,17 @@ static int read_packet_stats_and_jitter(struct avs_stats *stats, struct stats_ob
 
 		if (data->kind == STATS_KIND_AUDIO) {
 			stats->report.packets.audio.rx += data->packets_received;
-			stats->report.jitter.audio.rx = max(stats->report.jitter.audio.rx, (1000.0 * data->jitter));
+			if (data->packets_received) {
+				audio_jitter += data->jitter;
+				aj_count++;
+			}
 		}
 		else if (data->kind == STATS_KIND_VIDEO) {
 			stats->report.packets.video.rx += data->packets_received;
-			stats->report.jitter.video.rx = max(stats->report.jitter.video.rx, (1000.0 * data->jitter));
+			if (data->packets_received) {
+				video_jitter += data->jitter;
+				vj_count++;
+			}
 		}
 
 		stats->report.packets.lost.rx += data->packets_lost;
@@ -279,6 +291,10 @@ static int read_packet_stats_and_jitter(struct avs_stats *stats, struct stats_ob
 		stats->report.packets.lost.tx += data->packets_lost;
 	}
 
+	// 1.1 calcualete rx jitter in ms with taking mean
+	stats->report.jitter.audio.rx = aj_count ? 1000 * (audio_jitter / aj_count) : 0;
+	stats->report.jitter.video.rx = vj_count ? 1000 * (video_jitter / vj_count) : 0;
+
 	// 2. calculate interval percentage for packet loss into tmp variables
 	uint32_t loss_tx = calculate_loss_percentage(
 		(stats->report.packets.audio.tx - stats->last_packets.audio.tx +

test/test_jzon.cpp

@@ -153,6 +153,9 @@ TEST(jzon, createf_succeeds)
 	ASSERT_EQ(intv, 42);
 	ASSERT_EQ(0, jzon_double(&doublev, jout, "double"));
 	ASSERT_EQ(doublev, 42.);
+	// Reading an integer as double should also be ok
+	ASSERT_EQ(0, jzon_double(&doublev, jout, "int"));
+	ASSERT_EQ(doublev, 42.);
 
 	ASSERT_EQ(0, jzon_bool(&bval, jout, "bool0"));
 	ASSERT_FALSE(bval);

test/test_stats.cpp

@@ -320,32 +320,37 @@ class StatsJitter: public Base,
 
 		const auto irrelevant_rtp = new RTCInboundRtpStreamStats("irrelevantRtpId", Timestamp::Zero());
 		irrelevant_rtp->kind = "irrelevant";
-		irrelevant_rtp->jitter = 1.0;
+		irrelevant_rtp->jitter = 1;
+		irrelevant_rtp->packets_received = 1;
 		report->AddStats(std::unique_ptr<RTCStats>(irrelevant_rtp));
 
 		const auto audio_rtp = new RTCInboundRtpStreamStats("someAudioRtpId", Timestamp::Zero());
 		audio_rtp->kind = "audio";
 		audio_rtp->jitter = 0.01;
+		audio_rtp->packets_received = 1;
 		report->AddStats(std::unique_ptr<RTCStats>(audio_rtp));
 
 		const auto another_audio_rtp = new RTCInboundRtpStreamStats("anotherRtpId", Timestamp::Zero());
 		another_audio_rtp->kind = "audio";
 		another_audio_rtp->jitter = 0.02;
+		another_audio_rtp->packets_received = 1;
 		report->AddStats(std::unique_ptr<RTCStats>(another_audio_rtp));
 
 		const auto video_rtp = new RTCInboundRtpStreamStats("someVideoRtpId", Timestamp::Zero());
 		video_rtp->kind = "video";
 		video_rtp->jitter = 0.025;
+		video_rtp->packets_received = 1;
 		report->AddStats(std::unique_ptr<RTCStats>(video_rtp));
 
 		const auto another_video_rtp = new RTCInboundRtpStreamStats("anotherVideoRtpId", Timestamp::Zero());
 		another_video_rtp->kind = "video";
 		another_video_rtp->jitter = 0.015;
+		another_video_rtp->packets_received = 1;
 		report->AddStats(std::unique_ptr<RTCStats>(another_video_rtp));
 
 		const auto remote_irrelevant_rtp = new RTCRemoteInboundRtpStreamStats("irrelevantRemoteRtpId", Timestamp::Zero());
 		remote_irrelevant_rtp->kind = "irrelevant";
-		remote_irrelevant_rtp->jitter = 1.0;
+		remote_irrelevant_rtp->jitter = 1;
 		report->AddStats(std::unique_ptr<RTCStats>(remote_irrelevant_rtp));
 
 		const auto remote_audio_rtp = new RTCRemoteInboundRtpStreamStats("someRemoteAudioRtpId", Timestamp::Zero());
@@ -370,14 +375,38 @@ TEST_F(StatsJitter, audio_and_video)
 	stats_get_report(stats, &sr);
 
 	stats_jitter expected_jitter;
-	expected_jitter.audio.rx = 20; // max of [0.01, 0.02] * 1000
+	expected_jitter.audio.rx = 15; // mean of [0.01, 0.02] * 1000
 	expected_jitter.audio.tx = 30;
-	expected_jitter.video.rx = 25; // max of [0.025, 0.015] * 1000
+	expected_jitter.video.rx = 20; // mean of [0.025, 0.015] * 1000
 	expected_jitter.video.tx = 40;
 
 	EXPECT_EQ(sr.jitter, expected_jitter);
 }
 
+TEST_F(StatsJitter, zero_packet_rtp)
+{
+	const auto inbound_rtp = new RTCInboundRtpStreamStats("inboundRtpId", Timestamp::Zero());
+	inbound_rtp->kind = "audio";
+	inbound_rtp->jitter = 0.05;
+	inbound_rtp->packets_received = 1;
+	auto report = RTCStatsReport::Create(Timestamp::Zero());
+	report->AddStats(std::unique_ptr<RTCStats>(inbound_rtp));
+
+	const auto zero_packets_inbound_rtp = new RTCInboundRtpStreamStats("zeroInboundRtpId", Timestamp::Zero());
+	zero_packets_inbound_rtp->kind = "audio";
+	zero_packets_inbound_rtp->jitter = 0;
+	report->AddStats(std::unique_ptr<RTCStats>(zero_packets_inbound_rtp));
+
+	stats_update(stats, report->ToJson().c_str());
+	stats_get_report(stats, &sr);
+
+	stats_jitter expected_jitter;
+
+	const auto expected_audio_jitter = 50;
+
+	EXPECT_EQ(sr.jitter.audio.rx, expected_audio_jitter);
+}
+
 // ------------------------------------- RTT Tests --------------------------------------
 class StatsRtt: public Base,
 				public ::testing::Test {