From be82f937f4540b6d0370c7f6fe2ae4040c3efdd2 Mon Sep 17 00:00:00 2001
From: Simon Morlat <simon.morlat@linphone.org>
Date: Mon, 24 Jul 2023 16:53:03 +0200
Subject: [PATCH] Remove rtcp_rewind()/rtcp_next() that are error-prone, and
replace by a new RtcpParserContext object. This fixes an issue with RTCP
packets received via TURN not decoded properly.
---
coreapi/quality_reporting.c | 32 +++++++++++++++------------
src/c-wrapper/api/c-call-stats.cpp | 26 ++++++++++------------
src/conference/session/ms2-stream.cpp | 12 +++++-----
tester/tester.c | 17 ++++++++------
4 files changed, 47 insertions(+), 40 deletions(-)
diff --git a/coreapi/quality_reporting.c b/coreapi/quality_reporting.c
index 7d9d3651ab..44b85ed0a8 100644
--- a/coreapi/quality_reporting.c
+++ b/coreapi/quality_reporting.c
@@ -677,35 +677,38 @@ void linphone_reporting_on_rtcp_update(LinphoneCall *call, SalStreamType stats_t
metrics = &report->local_metrics;
block = _linphone_call_stats_get_sent_rtcp(stats);
}
+ RtcpParserContext rtcpctx;
+ const mblk_t *rtcpMessage = rtcp_parser_context_init(&rtcpctx, block);
+
do {
- if (rtcp_is_XR(block) && (rtcp_XR_get_block_type(block) == RTCP_XR_VOIP_METRICS)) {
+ if (rtcp_is_XR(rtcpMessage) && (rtcp_XR_get_block_type(rtcpMessage) == RTCP_XR_VOIP_METRICS)) {
- uint8_t config = rtcp_XR_voip_metrics_get_rx_config(block);
+ uint8_t config = rtcp_XR_voip_metrics_get_rx_config(rtcpMessage);
metrics->rtcp_xr_count++;
// for local mos rating, we'll use the quality indicator directly
// because rtcp XR might not be enabled
if (_linphone_call_stats_get_updated(stats) == LINPHONE_CALL_STATS_RECEIVED_RTCP_UPDATE) {
- metrics->quality_estimates.moslq = (rtcp_XR_voip_metrics_get_mos_lq(block) == 127)
+ metrics->quality_estimates.moslq = (rtcp_XR_voip_metrics_get_mos_lq(rtcpMessage) == 127)
? 127
- : rtcp_XR_voip_metrics_get_mos_lq(block) / 10.f;
- metrics->quality_estimates.moscq = (rtcp_XR_voip_metrics_get_mos_cq(block) == 127)
+ : rtcp_XR_voip_metrics_get_mos_lq(rtcpMessage) / 10.f;
+ metrics->quality_estimates.moscq = (rtcp_XR_voip_metrics_get_mos_cq(rtcpMessage) == 127)
? 127
- : rtcp_XR_voip_metrics_get_mos_cq(block) / 10.f;
+ : rtcp_XR_voip_metrics_get_mos_cq(rtcpMessage) / 10.f;
}
- metrics->jitter_buffer.nominal += rtcp_XR_voip_metrics_get_jb_nominal(block);
- metrics->jitter_buffer.max += rtcp_XR_voip_metrics_get_jb_maximum(block);
- metrics->jitter_buffer.abs_max = rtcp_XR_voip_metrics_get_jb_abs_max(block);
+ metrics->jitter_buffer.nominal += rtcp_XR_voip_metrics_get_jb_nominal(rtcpMessage);
+ metrics->jitter_buffer.max += rtcp_XR_voip_metrics_get_jb_maximum(rtcpMessage);
+ metrics->jitter_buffer.abs_max = rtcp_XR_voip_metrics_get_jb_abs_max(rtcpMessage);
metrics->jitter_buffer.adaptive = (config >> 4) & 0x3;
- metrics->packet_loss.network_packet_loss_rate = rtcp_XR_voip_metrics_get_loss_rate(block);
- metrics->packet_loss.jitter_buffer_discard_rate = rtcp_XR_voip_metrics_get_discard_rate(block);
+ metrics->packet_loss.network_packet_loss_rate = rtcp_XR_voip_metrics_get_loss_rate(rtcpMessage);
+ metrics->packet_loss.jitter_buffer_discard_rate = rtcp_XR_voip_metrics_get_discard_rate(rtcpMessage);
metrics->session_description.packet_loss_concealment = (config >> 6) & 0x3;
- metrics->delay.round_trip_delay += rtcp_XR_voip_metrics_get_round_trip_delay(block);
- } else if (rtcp_is_SR(block)) {
+ metrics->delay.round_trip_delay += rtcp_XR_voip_metrics_get_round_trip_delay(rtcpMessage);
+ } else if (rtcp_is_SR(rtcpMessage)) {
MediaStream *ms = (stats_type == 0)
? Call::toCpp(call)->getMediaStream(LinphoneStreamTypeAudio)
: ((stats_type == 1) ? Call::toCpp(call)->getMediaStream(LinphoneStreamTypeVideo)
@@ -717,7 +720,8 @@ void linphone_reporting_on_rtcp_update(LinphoneCall *call, SalStreamType stats_t
metrics->delay.round_trip_delay += (int)(1000 * rtt);
}
}
- } while (rtcp_next_packet(block));
+ } while ((rtcpMessage = rtcp_parser_context_next_packet(&rtcpctx)) != nullptr);
+ rtcp_parser_context_uninit(&rtcpctx);
/* check if we should send an interval report - use a random sending time to
dispatch reports and avoid sending them too close from each other */
diff --git a/src/c-wrapper/api/c-call-stats.cpp b/src/c-wrapper/api/c-call-stats.cpp
index ad1c2c283a..8dd9f387e5 100644
--- a/src/c-wrapper/api/c-call-stats.cpp
+++ b/src/c-wrapper/api/c-call-stats.cpp
@@ -304,16 +304,15 @@ float linphone_call_stats_get_sender_loss_rate(const LinphoneCallStats *stats) {
ms_warning("linphone_call_stats_get_sender_loss_rate(): there is no RTCP packet sent.");
return 0.0;
}
- /* Perform msgpullup() to prevent crashes in rtcp_is_SR() or rtcp_is_RR() if the RTCP packet is composed of several
- * mblk_t structure */
- if (stats->sent_rtcp->b_cont != NULL) msgpullup(stats->sent_rtcp, (size_t)-1);
+ RtcpParserContext parserCtx;
+ const mblk_t *rtcpMessage = rtcp_parser_context_init(&parserCtx, stats->sent_rtcp);
do {
- if (rtcp_is_SR(stats->sent_rtcp)) srb = rtcp_SR_get_report_block(stats->sent_rtcp, 0);
- else if (rtcp_is_RR(stats->sent_rtcp)) srb = rtcp_RR_get_report_block(stats->sent_rtcp, 0);
+ if (rtcp_is_SR(rtcpMessage)) srb = rtcp_SR_get_report_block(rtcpMessage, 0);
+ else if (rtcp_is_RR(rtcpMessage)) srb = rtcp_RR_get_report_block(rtcpMessage, 0);
if (srb) break;
- } while (rtcp_next_packet(stats->sent_rtcp));
- rtcp_rewind(stats->sent_rtcp);
+ } while ((rtcpMessage = rtcp_parser_context_next_packet(&parserCtx)) != nullptr);
+ rtcp_parser_context_uninit(&parserCtx);
if (!srb) return 0.0;
return 100.0f * (float)report_block_get_fraction_lost(srb) / 256.0f;
}
@@ -325,16 +324,15 @@ float linphone_call_stats_get_receiver_loss_rate(const LinphoneCallStats *stats)
ms_warning("linphone_call_stats_get_receiver_loss_rate(): there is no RTCP packet received.");
return 0.0;
}
- /* Perform msgpullup() to prevent crashes in rtcp_is_SR() or rtcp_is_RR() if the RTCP packet is composed of several
- * mblk_t structure */
- if (stats->received_rtcp->b_cont != NULL) msgpullup(stats->received_rtcp, (size_t)-1);
+ RtcpParserContext parserCtx;
+ const mblk_t *rtcpMessage = rtcp_parser_context_init(&parserCtx, stats->received_rtcp);
do {
- if (rtcp_is_RR(stats->received_rtcp)) rrb = rtcp_RR_get_report_block(stats->received_rtcp, 0);
- else if (rtcp_is_SR(stats->received_rtcp)) rrb = rtcp_SR_get_report_block(stats->received_rtcp, 0);
+ if (rtcp_is_RR(rtcpMessage)) rrb = rtcp_RR_get_report_block(rtcpMessage, 0);
+ else if (rtcp_is_SR(rtcpMessage)) rrb = rtcp_SR_get_report_block(rtcpMessage, 0);
if (rrb) break;
- } while (rtcp_next_packet(stats->received_rtcp));
- rtcp_rewind(stats->received_rtcp);
+ } while ((rtcpMessage = rtcp_parser_context_next_packet(&parserCtx)) != nullptr);
+ rtcp_parser_context_uninit(&parserCtx);
if (!rrb) return 0.0;
return 100.0f * (float)report_block_get_fraction_lost(rrb) / 256.0f;
}
diff --git a/src/conference/session/ms2-stream.cpp b/src/conference/session/ms2-stream.cpp
index ba5277b71f..d419b099e1 100644
--- a/src/conference/session/ms2-stream.cpp
+++ b/src/conference/session/ms2-stream.cpp
@@ -1375,16 +1375,18 @@ void MS2Stream::handleEvents() {
/*This MUST be done before any call to "linphone_call_stats_fill" since it has ownership over evd->packet*/
if (evt == ORTP_EVENT_RTCP_PACKET_RECEIVED) {
+ RtcpParserContext rtcpctx;
+ const mblk_t *rtcpMessage = rtcp_parser_context_init(&rtcpctx, evd->packet);
do {
- if (evd->packet && rtcp_is_RTPFB(evd->packet)) {
- if (rtcp_RTPFB_get_type(evd->packet) == RTCP_RTPFB_TMMBR) {
+ if (rtcpMessage && rtcp_is_RTPFB(rtcpMessage)) {
+ if (rtcp_RTPFB_get_type(rtcpMessage) == RTCP_RTPFB_TMMBR) {
CallSessionListener *listener = getMediaSessionPrivate().getCallSessionListener();
listener->onTmmbrReceived(getMediaSession().getSharedFromThis(), (int)getIndex(),
- (int)rtcp_RTPFB_tmmbr_get_max_bitrate(evd->packet));
+ (int)rtcp_RTPFB_tmmbr_get_max_bitrate(rtcpMessage));
}
}
- } while (rtcp_next_packet(evd->packet));
- rtcp_rewind(evd->packet);
+ } while ((rtcpMessage = rtcp_parser_context_next_packet(&rtcpctx)) != nullptr);
+ rtcp_parser_context_uninit(&rtcpctx);
}
if (ms) linphone_call_stats_fill(mStats, ms, ev);
bool isIceEvent = false;
diff --git a/tester/tester.c b/tester/tester.c
index 9aa49e6837..185d7fe089 100644
--- a/tester/tester.c
+++ b/tester/tester.c
@@ -3753,21 +3753,24 @@ void dtmf_received(LinphoneCore *lc, BCTBX_UNUSED(LinphoneCall *call), int dtmf)
counters->dtmf_count++;
}
-bool_t rtcp_is_type(const mblk_t *m, rtcp_type_t type) {
+static bool_t rtcp_is_type(const mblk_t *m, rtcp_type_t type) {
const rtcp_common_header_t *ch = rtcp_get_common_header(m);
return (ch != NULL && rtcp_common_header_get_packet_type(ch) == type);
}
-void rtcp_received(stats *counters, mblk_t *packet) {
+static void rtcp_received(stats *counters, mblk_t *packet) {
+ RtcpParserContext rtcpParser;
+
+ const mblk_t *rtcpMessage = rtcp_parser_context_init(&rtcpParser, packet);
do {
- if (rtcp_is_type(packet, RTCP_RTPFB)) {
- if (rtcp_RTPFB_get_type(packet) == RTCP_RTPFB_TMMBR) {
+ if (rtcp_is_type(rtcpMessage, RTCP_RTPFB)) {
+ if (rtcp_RTPFB_get_type(rtcpMessage) == RTCP_RTPFB_TMMBR) {
counters->number_of_tmmbr_received++;
- counters->last_tmmbr_value_received = (int)rtcp_RTPFB_tmmbr_get_max_bitrate(packet);
+ counters->last_tmmbr_value_received = (int)rtcp_RTPFB_tmmbr_get_max_bitrate(rtcpMessage);
}
}
- } while (rtcp_next_packet(packet));
- rtcp_rewind(packet);
+ } while ((rtcpMessage = rtcp_parser_context_next_packet(&rtcpParser)) != NULL);
+ rtcp_parser_context_uninit(&rtcpParser);
}
void call_stats_updated(LinphoneCore *lc, LinphoneCall *call, const LinphoneCallStats *lstats) {
--
GitLab