From 2498457b2f77380c2810e7cd3aa05dc899fb5e86 Mon Sep 17 00:00:00 2001 From: Christian Wick Date: Wed, 9 Nov 2022 12:04:08 +0000 Subject: [PATCH] rtspsrc: Introduce new action signal `push-backchannel-sample` with correct ownership semantics Signals are not supposed to take ownership of their arguments but only borrow them for the scope of the signal emission. The old action signal `push-backchannel-buffer` is now marked deprecated. Part-of: --- .../gst-plugins-good/docs/gst_plugins_cache.json | 15 ++++++++ subprojects/gst-plugins-good/gst/rtsp/gstrtspsrc.c | 45 +++++++++++++++++++--- subprojects/gst-plugins-good/gst/rtsp/gstrtspsrc.h | 1 + .../tests/examples/rtsp/test-onvif.c | 12 ++++-- 4 files changed, 65 insertions(+), 8 deletions(-) diff --git a/subprojects/gst-plugins-good/docs/gst_plugins_cache.json b/subprojects/gst-plugins-good/docs/gst_plugins_cache.json index f776fc8..0b03c5b 100644 --- a/subprojects/gst-plugins-good/docs/gst_plugins_cache.json +++ b/subprojects/gst-plugins-good/docs/gst_plugins_cache.json @@ -21342,6 +21342,21 @@ "return-type": "GstFlowReturn", "when": "last" }, + "push-backchannel-sample": { + "action": true, + "args": [ + { + "name": "arg0", + "type": "guint" + }, + { + "name": "arg1", + "type": "GstSample" + } + ], + "return-type": "GstFlowReturn", + "when": "last" + }, "request-rtcp-key": { "args": [ { diff --git a/subprojects/gst-plugins-good/gst/rtsp/gstrtspsrc.c b/subprojects/gst-plugins-good/gst/rtsp/gstrtspsrc.c index 6922dea..8763f1b 100644 --- a/subprojects/gst-plugins-good/gst/rtsp/gstrtspsrc.c +++ b/subprojects/gst-plugins-good/gst/rtsp/gstrtspsrc.c @@ -151,6 +151,7 @@ enum SIGNAL_GET_PARAMETER, SIGNAL_GET_PARAMETERS, SIGNAL_SET_PARAMETER, + SIGNAL_PUSH_BACKCHANNEL_SAMPLE, LAST_SIGNAL }; @@ -467,6 +468,9 @@ static gboolean set_parameter (GstRTSPSrc * src, const gchar * name, static GstFlowReturn gst_rtspsrc_push_backchannel_buffer (GstRTSPSrc * src, guint id, GstSample * sample); +static GstFlowReturn gst_rtspsrc_push_backchannel_sample (GstRTSPSrc * src, + guint id, GstSample * sample); + typedef struct { guint8 pt; @@ -1226,15 +1230,34 @@ gst_rtspsrc_class_init (GstRTSPSrcClass * klass) /** * GstRTSPSrc::push-backchannel-buffer: * @rtspsrc: a #GstRTSPSrc + * @id: stream ID where the sample should be sent * @sample: RTP sample to send back * + * Deprecated: 1.22: Use action signal GstRTSPSrc::push-backchannel-sample instead. + * IMPORTANT: Please note that this signal decrements the reference count + * of sample internally! So it cannot be used from other + * language bindings in general. * */ gst_rtspsrc_signals[SIGNAL_PUSH_BACKCHANNEL_BUFFER] = g_signal_new ("push-backchannel-buffer", G_TYPE_FROM_CLASS (klass), - G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION, G_STRUCT_OFFSET (GstRTSPSrcClass, - push_backchannel_buffer), NULL, NULL, NULL, - GST_TYPE_FLOW_RETURN, 2, G_TYPE_UINT, GST_TYPE_SAMPLE); + G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION | G_SIGNAL_DEPRECATED, + G_STRUCT_OFFSET (GstRTSPSrcClass, push_backchannel_buffer), NULL, NULL, + NULL, GST_TYPE_FLOW_RETURN, 2, G_TYPE_UINT, GST_TYPE_SAMPLE); + + /** + * GstRTSPSrc::push-backchannel-sample: + * @rtspsrc: a #GstRTSPSrc + * @id: stream ID where the sample should be sent + * @sample: RTP sample to send back + * + * Since: 1.22 + */ + gst_rtspsrc_signals[SIGNAL_PUSH_BACKCHANNEL_SAMPLE] = + g_signal_new ("push-backchannel-sample", G_TYPE_FROM_CLASS (klass), + G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION | G_SIGNAL_DEPRECATED, + G_STRUCT_OFFSET (GstRTSPSrcClass, push_backchannel_buffer), NULL, NULL, + NULL, GST_TYPE_FLOW_RETURN, 2, G_TYPE_UINT, GST_TYPE_SAMPLE); /** * GstRTSPSrc::get-parameter: @@ -1307,6 +1330,7 @@ gst_rtspsrc_class_init (GstRTSPSrcClass * klass) gstbin_class->handle_message = gst_rtspsrc_handle_message; klass->push_backchannel_buffer = gst_rtspsrc_push_backchannel_buffer; + klass->push_backchannel_sample = gst_rtspsrc_push_backchannel_sample; klass->get_parameter = GST_DEBUG_FUNCPTR (get_parameter); klass->get_parameters = GST_DEBUG_FUNCPTR (get_parameters); klass->set_parameter = GST_DEBUG_FUNCPTR (set_parameter); @@ -3331,6 +3355,19 @@ static GstFlowReturn gst_rtspsrc_push_backchannel_buffer (GstRTSPSrc * src, guint id, GstSample * sample) { + GstFlowReturn res; + + res = gst_rtspsrc_push_backchannel_sample (src, id, sample); + + gst_sample_unref (sample); + + return res; +} + +static GstFlowReturn +gst_rtspsrc_push_backchannel_sample (GstRTSPSrc * src, guint id, + GstSample * sample) +{ GstFlowReturn res = GST_FLOW_OK; GstRTSPStream *stream; @@ -3376,8 +3413,6 @@ gst_rtspsrc_push_backchannel_buffer (GstRTSPSrc * src, guint id, } out: - gst_sample_unref (sample); - return res; } diff --git a/subprojects/gst-plugins-good/gst/rtsp/gstrtspsrc.h b/subprojects/gst-plugins-good/gst/rtsp/gstrtspsrc.h index 93ad288..9457972 100644 --- a/subprojects/gst-plugins-good/gst/rtsp/gstrtspsrc.h +++ b/subprojects/gst-plugins-good/gst/rtsp/gstrtspsrc.h @@ -339,6 +339,7 @@ struct _GstRTSPSrcClass { gboolean (*get_parameters) (GstRTSPSrc *rtsp, gchar **parameters, const gchar *content_type, GstPromise *promise); gboolean (*set_parameter) (GstRTSPSrc *rtsp, const gchar *name, const gchar *value, const gchar *content_type, GstPromise *promise); GstFlowReturn (*push_backchannel_buffer) (GstRTSPSrc *src, guint id, GstSample *sample); + GstFlowReturn (*push_backchannel_sample) (GstRTSPSrc *src, guint id, GstSample *sample); }; GType gst_rtspsrc_get_type(void); diff --git a/subprojects/gst-plugins-good/tests/examples/rtsp/test-onvif.c b/subprojects/gst-plugins-good/tests/examples/rtsp/test-onvif.c index 2f47445..ddaaeee 100644 --- a/subprojects/gst-plugins-good/tests/examples/rtsp/test-onvif.c +++ b/subprojects/gst-plugins-good/tests/examples/rtsp/test-onvif.c @@ -14,15 +14,21 @@ new_sample (GstElement * appsink, GstElement * rtspsrc) g_assert (stream_id != -1); + /* get the sample from appsink */ g_signal_emit_by_name (appsink, "pull-sample", &sample); if (!sample) - goto out; + goto nosample; - g_signal_emit_by_name (rtspsrc, "push-backchannel-buffer", stream_id, sample, + g_signal_emit_by_name (rtspsrc, "push-backchannel-sample", stream_id, sample, &ret); -out: + /* Action signal callbacks don't take ownership of the arguments passed, so we must unref the sample here. + * (The "push-backchannel-buffer" callback unrefs the sample, which is wrong and doesn't work with bindings + * but could not be changed, hence the new "push-backchannel-sample" callback that behaves correctly.) */ + gst_sample_unref (sample); + +nosample: return ret; } -- 2.7.4