[Bug 792131] rtspsrc: add an action signal to send SET_PARAMETER

[GStreamer (GNOME Bugzilla)]

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.