RFC: GstRtpBaseDepayload: RTP header extensions get lost when depayloader aggregates RTP packets
In gstrtpbasedepay.c:gst_rtp_base_depayload_set_headers() calls read_rtp_header_extensions() with priv->input_buffer. In case of rtph264depay and others where several RTP buffers are aggregated before a buffer is pushed this means that only the header extension of the last RTP buffer used for output buffer aggregation is parsed. All other header extensions are silently ignored.
I would expect that every RTP buffer is parsed for header extensions and this should not depend on the depayloader implementation.
The header extensions that I currently implement are the full and short IV counters for HDCP over RTP (https://www.digital-cp.com/sites/default/files/HDCP%20Direct%20Adaptation%20Amendment%20Spec%20Rev2_3.pdf)
The full IV counter extension is added to the first RTP packet of a HDU (a video frame in case of video) and the short IV counter is added to all subsequent RTP packets of the same frame. On the depayloader side I only get the short IV counters if the frame is split among multiple RTP packets (which is quite likely).
As a starting point for further discussions, this is a patch that I quickly hacked up to solve the issue (and likely introduce new issues) which remembers all buffers that have been pushed to the derived class that add to the next buffer pushed on the src pad of the depayloader:
modified subprojects/gst-plugins-base/gst-libs/gst/rtp/gstrtpbasedepayload.c
@@ -70,6 +70,7 @@ struct _GstRTPBaseDepayloadPrivate
gboolean source_info;
GstBuffer *input_buffer;
+ GSList *unpushed_buffers;
GstFlowReturn process_flow_ret;
@@ -824,6 +825,9 @@ gst_rtp_base_depayload_handle_buffer (GstRTPBaseDepayload * filter,
priv->process_flow_ret = gst_rtp_base_depayload_push (filter, out_buf);
else
gst_buffer_unref (out_buf);
+ } else {
+ gst_buffer_ref (in);
+ priv->unpushed_buffers = g_slist_prepend (priv->unpushed_buffers, in);
}
gst_buffer_unref (in);
@@ -1289,6 +1293,7 @@ gst_rtp_base_depayload_set_headers (GstRTPBaseDepayload * depayload,
{
GstRTPBaseDepayloadPrivate *priv = depayload->priv;
GstClockTime pts, dts, duration;
+ gboolean ret = FALSE;
pts = GST_BUFFER_PTS (buffer);
dts = GST_BUFFER_DTS (buffer);
@@ -1318,10 +1323,20 @@ gst_rtp_base_depayload_set_headers (GstRTPBaseDepayload * depayload,
if (priv->source_info)
add_rtp_source_meta (buffer, priv->input_buffer);
- return read_rtp_header_extensions (depayload, priv->input_buffer, buffer);
+ priv->unpushed_buffers = g_slist_reverse (priv->unpushed_buffers);
+ for (gint n = 0; n < g_slist_length (priv->unpushed_buffers); ++n) {
+ GstBuffer *buf = g_slist_nth_data (priv->unpushed_buffers, n);
+ ret |= read_rtp_header_extensions (depayload, buf, buffer);
+ }
}
- return FALSE;
+ return ret;
+}
+
+static void
+gst_rtp_base_depayload_unpushed_buffer_unref (void * buf)
+{
+ gst_buffer_unref ((GST_BUFFER (buf)));
}
static GstFlowReturn
@@ -1335,6 +1350,9 @@ gst_rtp_base_depayload_finish_push (GstRTPBaseDepayload * filter,
GST_DEBUG_OBJECT (filter, "Pushed newsegment event on this first buffer");
}
+ g_slist_free_full (g_steal_pointer (&filter->priv->unpushed_buffers),
+ gst_rtp_base_depayload_unpushed_buffer_unref);
+
if (is_list) {
GstBufferList *blist = obj;
return gst_pad_push_list (filter->srcpad, blist);