[Bug 792131] rtspsrc: add an action signal to send SET_PARAMETER
GStreamer (GNOME Bugzilla)
bugzilla at gnome.org
Tue Aug 14 07:28:22 UTC 2018
https://bugzilla.gnome.org/show_bug.cgi?id=792131
--- Comment #20 from ulfo at axis.com ---
(In reply to Sebastian Dröge (slomo) from comment #19)
> Review of attachment 373308 [details] [review]:
>
> ::: gst/rtsp/gstrtspsrc.c
> @@ +1073,3 @@
> + G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION, G_STRUCT_OFFSET (GstRTSPSrcClass,
> + get_parameter), NULL, NULL, g_cclosure_marshal_generic,
> + G_TYPE_BOOLEAN, 3, G_TYPE_STRING, G_TYPE_STRING, G_TYPE_POINTER);
>
> This should be GST_TYPE_PROMISE, not GST_TYPE_POINTER
>
> @@ +1091,3 @@
> + G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION, G_STRUCT_OFFSET (GstRTSPSrcClass,
> + get_parameters), NULL, NULL, g_cclosure_marshal_generic,
> + G_TYPE_BOOLEAN, 3, G_TYPE_STRV, G_TYPE_STRING, G_TYPE_POINTER);
>
> And here
>
> @@ +1111,3 @@
> + set_parameter), NULL, NULL, g_cclosure_marshal_generic,
> + G_TYPE_BOOLEAN, 4, G_TYPE_STRING, G_TYPE_STRING, G_TYPE_STRING,
> + G_TYPE_POINTER);
>
> And here
>
> @@ +9036,3 @@
> + {
> +
> + if (req) {
>
> Shouldn't we expire/fail the promise in all other cases here or does that
> automatically happen later?
>
> Same about the other functions below
>
> @@ +9074,3 @@
> +
> + GST_ELEMENT_ERROR (src, LIBRARY, INIT, (NULL),
> + ("Could not create request. (%s)", str));
>
> All the errors here, do they have to be fatal (error messages are fatal)? Or
> could we instead just continue normally and only signal an error via the
> promise?
>
> @@ +9083,3 @@
> +
> + GST_ELEMENT_ERROR (src, LIBRARY, INIT, (NULL),
> + ("Could add content header. (%s)", str));
>
> "Could not" instead of "Could" I assume? Also above :)
Yes, it is better to pass on the errors via the promise. Should we do that for
all potential errors in gst_rtspsrc_set_parameter/gst_rtspsrc_get_parameter?
if (req) {
is unnecessary (req == NULL is a major bug) but currently "promises" can only
expire when gst_rtspsrc_cleanup is called. So the question is, should a promise
just live in the queue for a limited period of time? Plus, should we limit the
number of SET/GET requests in the queue?
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
More information about the gstreamer-bugs
mailing list