Hey there,
Good to be back hacking on PJSIP :-)
I noticed some nice improvements in parsing RTCP feedback packets, nice!
Full PLI support looked to be in the TODO list, and since we (Jitsi)
need it for a project, here are some patches which complete PLI support
by building on top of the existing APIs and adding where needed.
Patch 001: makes vid_stream automagically send a keyframe when a PLI is
received.
Patch 002: adds an API to vid_stream in order to send a PLI packet.
Patch 003: make pjsua (also) send PLI packets when a keyframe is missing.
Cheers,
--
Saúl
From 7d1ba08f9858e4c52c25e4425564473c6b6f337c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sa=C3=BAl=20Ibarra=20Corretg=C3=A9?= <s@xxxxxxxxxx>
Date: Thu, 28 Nov 2019 14:35:20 +0100
Subject: [PATCH] pjsua: also send RTCP PLI when a keyframe is missing
---
pjsip/include/pjsua-lib/pjsua.h | 1 -
pjsip/src/pjsua-lib/pjsua_media.c | 29 ++++++++++++++++++++++-------
2 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/pjsip/include/pjsua-lib/pjsua.h b/pjsip/include/pjsua-lib/pjsua.h
index a81fcb94d..8a3685679 100644
--- a/pjsip/include/pjsua-lib/pjsua.h
+++ b/pjsip/include/pjsua-lib/pjsua.h
@@ -4664,7 +4664,6 @@ typedef enum pjsua_vid_req_keyframe_method
/**
* Requesting keyframe via Picture Loss Indication of RTCP feedback.
- * This is currently not supported.
*/
PJSUA_VID_REQ_KEYFRAME_RTCP_PLI = 2
diff --git a/pjsip/src/pjsua-lib/pjsua_media.c b/pjsip/src/pjsua-lib/pjsua_media.c
index 0b6684b04..c7a9bbedd 100644
--- a/pjsip/src/pjsua-lib/pjsua_media.c
+++ b/pjsip/src/pjsua-lib/pjsua_media.c
@@ -1502,15 +1502,17 @@ pj_status_t call_media_on_event(pjmedia_event *event,
pj_status_t status = PJ_SUCCESS;
switch(event->type) {
- case PJMEDIA_EVENT_KEYFRAME_MISSING:
+ case PJMEDIA_EVENT_KEYFRAME_MISSING: {
+ pj_timestamp now;
+
+ pj_get_timestamp(&now);
+ if (pj_elapsed_msec(&call_med->last_req_keyframe, &now) <
+ PJSUA_VID_REQ_KEYFRAME_INTERVAL) {
+ break;
+ }
+
if (call->opt.req_keyframe_method & PJSUA_VID_REQ_KEYFRAME_SIP_INFO)
{
- pj_timestamp now;
-
- pj_get_timestamp(&now);
- if (pj_elapsed_msec(&call_med->last_req_keyframe, &now) >=
- PJSUA_VID_REQ_KEYFRAME_INTERVAL)
- {
pjsua_msg_data msg_data;
const pj_str_t SIP_INFO = {"INFO", 4};
const char *BODY_TYPE = "application/media_control+xml";
@@ -1534,9 +1536,22 @@ pj_status_t call_media_on_event(pjmedia_event *event,
} else {
call_med->last_req_keyframe = now;
}
+ }
+
+ if (call->opt.req_keyframe_method & PJSUA_VID_REQ_KEYFRAME_RTCP_PLI)
+ {
+ PJ_LOG(4,(THIS_FILE,
+ "Sending video keyframe request via RTCP PLI"));
+ status = pjmedia_vid_stream_send_rtcp_pli(call_med->strm.v.stream);
+ if (status != PJ_SUCCESS) {
+ PJ_PERROR(3,(THIS_FILE, status,
+ "Failed requesting keyframe via RTCP PLI"));
+ } else {
+ call_med->last_req_keyframe = now;
}
}
break;
+ }
#if PJSUA_HAS_VIDEO
case PJMEDIA_EVENT_FMT_CHANGED:
From 376dcb8537153d8da83f8754d86131ea48427f30 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sa=C3=BAl=20Ibarra=20Corretg=C3=A9?= <s@xxxxxxxxxx>
Date: Thu, 28 Nov 2019 14:34:12 +0100
Subject: [PATCH] vid_stream: add API for sending an RTCP PLI
---
pjmedia/include/pjmedia/vid_stream.h | 10 ++++++
pjmedia/src/pjmedia/vid_stream.c | 51 +++++++++++++++++++++++-----
2 files changed, 52 insertions(+), 9 deletions(-)
diff --git a/pjmedia/include/pjmedia/vid_stream.h b/pjmedia/include/pjmedia/vid_stream.h
index 7e69d63e1..40743b8a5 100644
--- a/pjmedia/include/pjmedia/vid_stream.h
+++ b/pjmedia/include/pjmedia/vid_stream.h
@@ -451,6 +451,16 @@ PJ_DECL(pj_status_t) pjmedia_vid_stream_send_rtcp_sdes(
PJ_DECL(pj_status_t) pjmedia_vid_stream_send_rtcp_bye(
pjmedia_vid_stream *stream);
+/**
+ * Send RTCP PLI for the media stream.
+ *
+ * @param stream The media stream.
+ *
+ * @return PJ_SUCCESS on success.
+ */
+PJ_DECL(pj_status_t) pjmedia_vid_stream_send_rtcp_pli(
+ pjmedia_vid_stream *stream);
+
/**
* Get the RTP session information of the video media stream. This function
* can be useful for app with custom media transport to inject/filter some
diff --git a/pjmedia/src/pjmedia/vid_stream.c b/pjmedia/src/pjmedia/vid_stream.c
index 1ca3145fc..4cd54b0e5 100644
--- a/pjmedia/src/pjmedia/vid_stream.c
+++ b/pjmedia/src/pjmedia/vid_stream.c
@@ -204,7 +204,8 @@ static pj_status_t decode_frame(pjmedia_vid_stream *stream,
static pj_status_t send_rtcp(pjmedia_vid_stream *stream,
pj_bool_t with_sdes,
- pj_bool_t with_bye);
+ pj_bool_t with_bye,
+ pj_bool_t with_pli);
static void on_rx_rtcp( void *data,
void *pkt,
@@ -483,7 +484,7 @@ static void send_keep_alive_packet(pjmedia_vid_stream *stream)
pkt_len);
/* Send RTCP */
- send_rtcp(stream, PJ_TRUE, PJ_FALSE);
+ send_rtcp(stream, PJ_TRUE, PJ_FALSE, PJ_FALSE);
/* Update stats in case the stream is paused */
stream->rtcp.stat.rtp_tx_last_seq = pj_ntohs(stream->enc->rtp.out_hdr.seq);
@@ -519,7 +520,8 @@ static void send_keep_alive_packet(pjmedia_vid_stream *stream)
static pj_status_t send_rtcp(pjmedia_vid_stream *stream,
pj_bool_t with_sdes,
- pj_bool_t with_bye)
+ pj_bool_t with_bye,
+ pj_bool_t with_pli)
{
void *sr_rr_pkt;
pj_uint8_t *pkt;
@@ -530,7 +532,7 @@ static pj_status_t send_rtcp(pjmedia_vid_stream *stream,
/* Build RTCP RR/SR packet */
pjmedia_rtcp_build_rtcp(&stream->rtcp, &sr_rr_pkt, &len);
- if (with_sdes || with_bye) {
+ if (with_sdes || with_bye || with_pli) {
pkt = (pj_uint8_t*) stream->out_rtcp_pkt;
pj_memcpy(pkt, sr_rr_pkt, len);
max_len = stream->out_rtcp_pkt_size;
@@ -572,6 +574,20 @@ static pj_status_t send_rtcp(pjmedia_vid_stream *stream,
}
}
+ /* Build RTCP PLI packet */
+ if (with_pli) {
+ pj_size_t pli_len;
+
+ pli_len = max_len - len;
+ status = pjmedia_rtcp_fb_build_pli(&stream->rtcp, pkt+len, &pli_len);
+ if (status != PJ_SUCCESS) {
+ PJ_PERROR(4,(stream->name.ptr, status,
+ "Error generating RTCP PLI"));
+ } else {
+ len += (int)pli_len;
+ }
+ }
+
/* Send! */
status = pjmedia_transport_send_rtcp(stream->transport, pkt, len);
if (status != PJ_SUCCESS) {
@@ -608,7 +624,8 @@ static void check_tx_rtcp(pjmedia_vid_stream *stream, pj_uint32_t timestamp)
} else if (timestamp - stream->rtcp_last_tx >= stream->rtcp_interval) {
pj_status_t status;
- status = send_rtcp(stream, !stream->rtcp_sdes_bye_disabled, PJ_FALSE);
+ status = send_rtcp(stream, !stream->rtcp_sdes_bye_disabled,
+ PJ_FALSE, PJ_FALSE);
if (status != PJ_SUCCESS) {
PJ_PERROR(4,(stream->name.ptr, status,
"Error sending RTCP"));
@@ -886,7 +903,7 @@ static void on_rx_rtp( pjmedia_tp_cb_param *param)
/* Send RTCP RR and SDES after we receive some RTP packets */
if (stream->rtcp.received >= 10 && !stream->initial_rr) {
status = send_rtcp(stream, !stream->rtcp_sdes_bye_disabled,
- PJ_FALSE);
+ PJ_FALSE, PJ_FALSE);
if (status != PJ_SUCCESS) {
PJ_PERROR(4,(stream->name.ptr, status,
"Error sending initial RTCP RR"));
@@ -1954,7 +1971,7 @@ PJ_DEF(pj_status_t) pjmedia_vid_stream_destroy( pjmedia_vid_stream *stream )
/* Send RTCP BYE (also SDES) */
if (stream->transport && !stream->rtcp_sdes_bye_disabled) {
- send_rtcp(stream, PJ_TRUE, PJ_TRUE);
+ send_rtcp(stream, PJ_TRUE, PJ_TRUE, PJ_FALSE);
}
/* Unsubscribe from RTCP session events */
@@ -2217,7 +2234,7 @@ PJ_DEF(pj_status_t) pjmedia_vid_stream_send_rtcp_sdes(
{
PJ_ASSERT_RETURN(stream, PJ_EINVAL);
- return send_rtcp(stream, PJ_TRUE, PJ_FALSE);
+ return send_rtcp(stream, PJ_TRUE, PJ_FALSE, PJ_FALSE);
}
@@ -2230,7 +2247,23 @@ PJ_DEF(pj_status_t) pjmedia_vid_stream_send_rtcp_bye(
PJ_ASSERT_RETURN(stream, PJ_EINVAL);
if (stream->enc && stream->transport) {
- return send_rtcp(stream, PJ_TRUE, PJ_TRUE);
+ return send_rtcp(stream, PJ_TRUE, PJ_TRUE, PJ_FALSE);
+ }
+
+ return PJ_SUCCESS;
+}
+
+
+/*
+ * Send RTCP PLI.
+ */
+PJ_DEF(pj_status_t) pjmedia_vid_stream_send_rtcp_pli(
+ pjmedia_vid_stream *stream)
+{
+ PJ_ASSERT_RETURN(stream, PJ_EINVAL);
+
+ if (stream->enc && stream->transport) {
+ return send_rtcp(stream, PJ_FALSE, PJ_FALSE, PJ_TRUE);
}
return PJ_SUCCESS;
From 53d31f3171f35888efa42ca66bfba9752559bf8c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sa=C3=BAl=20Ibarra=20Corretg=C3=A9?= <s@xxxxxxxxxx>
Date: Thu, 28 Nov 2019 14:06:40 +0100
Subject: [PATCH] vid_stream: send a keyframe when a RTCP PLI is received
---
pjmedia/src/pjmedia/rtcp.c | 12 ++++++++++--
pjmedia/src/pjmedia/vid_stream.c | 26 ++++++++++++++++++++++++++
2 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/pjmedia/src/pjmedia/rtcp.c b/pjmedia/src/pjmedia/rtcp.c
index 08703e1ed..0393869e2 100644
--- a/pjmedia/src/pjmedia/rtcp.c
+++ b/pjmedia/src/pjmedia/rtcp.c
@@ -794,10 +794,18 @@ static void parse_rtcp_fb(pjmedia_rtcp_session *sess,
/* Sync publish, i.e: don't use PJMEDIA_EVENT_PUBLISH_POST_EVENT */
pjmedia_event_publish(NULL, sess, &ev, 0);
-
- /* For other FB type implementations later
} else if (pjmedia_rtcp_fb_parse_pli(pkt, size)==PJ_SUCCESS)
{
+ const pj_str_t pli_param = { "pli", 3 };
+ pjmedia_event_init(&ev, PJMEDIA_EVENT_RX_RTCP_FB, &ts_now, sess);
+ ev_data.cap.type = PJMEDIA_RTCP_FB_OTHER;
+ ev_data.cap.param = pli_param;
+ ev.data.ptr = &ev_data;
+
+ /* Sync publish, i.e: don't use PJMEDIA_EVENT_PUBLISH_POST_EVENT */
+ pjmedia_event_publish(NULL, sess, &ev, 0);
+
+ /* For other FB type implementations later
} else if (pjmedia_rtcp_fb_parse_sli(pkt, size, &cnt, sli)==PJ_SUCCESS)
{
} else if (pjmedia_rtcp_fb_parse_rpsi(pkt, size, &rpsi)==PJ_SUCCESS)
diff --git a/pjmedia/src/pjmedia/vid_stream.c b/pjmedia/src/pjmedia/vid_stream.c
index 564548661..1ca3145fc 100644
--- a/pjmedia/src/pjmedia/vid_stream.c
+++ b/pjmedia/src/pjmedia/vid_stream.c
@@ -21,6 +21,7 @@
#include <pjmedia/event.h>
#include <pjmedia/rtp.h>
#include <pjmedia/rtcp.h>
+#include <pjmedia/rtcp_fb.h>
#include <pjmedia/jbuf.h>
#include <pj/array.h>
#include <pj/assert.h>
@@ -400,6 +401,23 @@ static pj_status_t stream_event_cb(pjmedia_event *event,
pj_memcpy(&stream->miss_keyframe_event, event, sizeof(*event));
return PJ_SUCCESS;
+ default:
+ break;
+ }
+ } else if (event->epub == &stream->rtcp) {
+ /* This is RTCP event */
+ switch (event->type) {
+ case PJMEDIA_EVENT_RX_RTCP_FB: {
+ pjmedia_event_rx_rtcp_fb_data *data
+ = (pjmedia_event_rx_rtcp_fb_data*)event->data.ptr;
+ if (data->cap.type == PJMEDIA_RTCP_FB_OTHER
+ && pj_strcmp2(&data->cap.param, "pli") == 0) {
+ PJ_LOG(4, (stream->name.ptr, "PLI received, sending keyframe"));
+ return pjmedia_vid_stream_send_keyframe(stream);
+ }
+ break;
+ }
+
default:
break;
}
@@ -1815,6 +1833,10 @@ PJ_DEF(pj_status_t) pjmedia_vid_stream_create(
rtcp_setting.samples_per_frame = 1;
pjmedia_rtcp_init2(&stream->rtcp, &rtcp_setting);
+
+ /* Subscribe to RTCP events */
+ pjmedia_event_subscribe(NULL, &stream_event_cb, stream,
+ &stream->rtcp);
}
/* Allocate outgoing RTCP buffer, should be enough to hold SR/RR, SDES,
@@ -1935,6 +1957,10 @@ PJ_DEF(pj_status_t) pjmedia_vid_stream_destroy( pjmedia_vid_stream *stream )
send_rtcp(stream, PJ_TRUE, PJ_TRUE);
}
+ /* Unsubscribe from RTCP session events */
+ pjmedia_event_unsubscribe(NULL, &stream_event_cb, stream,
+ &stream->rtcp);
+
/* Detach from transport
* MUST NOT hold stream mutex while detaching from transport, as
* it may cause deadlock. See ticket #460 for the details.
_______________________________________________
Visit our blog: http://blog.pjsip.org
pjsip mailing list
pjsip@xxxxxxxxxxxxxxx
http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
