Survey of functions returning `bool` under workspace `gstreamer-base`

Survey of functions returning bool under workspace gstreamer-base

This is a survey of functions returning bool under workspace gstreamer-base. I filtered out the functions which, from their name, obviously return a genuine boolean (is_, has_, ...) as well as the get_property_xxx and the parent_ counterparts in order to reduce the list. This is part of the discussions in #166 (closed) & !196 (closed) and a continuation of !200 (merged) and !207 (merged).

Proposal

Function	Location	Gst Function	Proposed Return Type	Optional?	Comment
Aggregator.negotiated_src_caps	subclass	GstAggregatorClass.negotiated_src_caps	Result<(), LoggableError>	yes	Notifies subclasses what caps format has been negotiated. Note: the return value seems to indicate whether the subclass is OK with the negotiation. @slomo comment: This is always an error it seems, and is basically like set_caps() elsewhere. I think I'd use BoolError here at least instead of a boolean
Aggregator.src_activate	subclass	GstAggregatorClass.src_activate	Result<(), LoggableError>	yes	Called when the src pad is activated, it will start/stop its pad task right after that call. Note: Pad's activate_function was changed to Result<...>
Aggregator.src_event	subclass	GstAggregatorClass.src_event	keep bool	yes	Called when an event is received on the src pad, the subclass should always chain up. Note: Event fct -> bool
Aggregator.src_query	subclass	GstAggregatorClass.src_query	keep bool	yes	Called when a query is received on the src pad, the subclass should always chain up. Note: Query fct -> bool
Aggregator.start	subclass	GstAggregatorClass.start	Result<(), ErrorMessage>	yes	Called when the element goes from READY to PAUSED. The subclass should get ready to process aggregated buffers. See § start & stop below
Aggregator.stop	subclass	GstAggregatorClass.stop	Result<(), ErrorMessage>	yes	Called when the element goes from PAUSED to READY. The subclass should free all resources and reset its state. See § start & stop below
AggregatorPad.drop_buffer	auto	gst_aggregator_pad_drop_buffer	keep bool	n/a	Returns TRUE if there was a buffer queued in pad, or FALSE if not. Note: not a failure.
BaseSink.event	subclass	GstBaseSinkClass.sink_event	keep bool	yes	Event fct -> bool
BaseSink.query	subclass	GstBaseSinkClass.query	keep bool	yes	Query fct -> bool
BaseSink.set_caps	subclass	GstBaseSinkClass.set_caps	Result<(), LoggableError>	yes	Notify subclass of changed caps.
BaseSink.start	subclass	GstBaseSinkClass.start	Result<(), ErrorMessage>	yes	Start processing. Ideal for opening resources in the subclass. See § start & stop below
BaseSink.stop	subclass	GstBaseSinkClass.stop	Result<(), ErrorMessage>	yes	Stop processing. Subclasses should use this to close resources. See § start & stop below
BaseSink.unlock	subclass	GstBaseSinkClass.unlock	Result<(), ErrorMessage>	yes	Unlock any pending access to the resource. Subclasses should unblock any blocked function ASAP and call gst_base_sink_wait_preroll(). Note: return value is ignored. @slomo comment: This seems like a bug in the C code too (want to send a MR?). If this fails it generally means that everything is broken, so there should be an error message even.
BaseSink.unlock_stop	subclass	GstBaseSinkClass.unlock_stop	Result<(), ErrorMessage>	yes	Clear the previous unlock request. Subclasses should clear any state they set during GstBaseSinkClass.unlock(), and be ready to continue where they left off after gst_base_sink_wait_preroll(), gst_base_sink_wait() or gst_wait_sink_wait_clock() return or GstBaseSinkClass.render() is called again. Note: return value is ignored. See comment for BaseSink.unlock
BaseSrc.do_seek	subclass	GstBaseSinkClass.do_seek	keep bool	yes	Perform seeking on the resource to the indicated segment.
BaseSrc.event	subclass	GstBaseSinkClass.event	keep bool	yes	Event fct -> bool
BaseSrc.negotiate	subclass	GstBaseSinkClass.negotiate	Result<(), LoggableError>	yes	Negotiated the caps with the peer.
BaseSrc.new_seamless_segment	auto	gst_base_src_new_seamless_segment	()	n/a	Returns TRUE if preparation of the seamless segment succeeded. @slomo comment: This can literally never fail and should probably become ()
BaseSrc.query	subclass	GstBaseSinkClass.query	keep bool	yes	Query fct -> bool
BaseSrc.set_caps	auto	gst_base_src_set_caps	Result<(), glib::BoolError>	n/a	Returns TRUE if the caps could be set.
BaseSrc.set_caps	subclass	GstBaseSinkClass.set_caps	Result<(), LoggableError>	yes	Notify the subclass of new caps.
BaseSrc.start	subclass	GstBaseSinkClass.start	Result<(), ErrorMessage>	yes	Start processing. Subclasses should open resources and prepare to produce data. Implementation should call gst_base_src_start_complete() when the operation completes, either from the current thread or any other thread that finishes the start operation asynchronously. See § start & stop below
BaseSrc.stop	subclass	GstBaseSinkClass.stop	Result<(), ErrorMessage>	yes	Stop processing. Subclasses should use this to close resources. See § start & stop below
BaseSrc.unlock	subclass	GstBaseSinkClass.unlock	Result<(), ErrorMessage>	yes	Unlock any pending access to the resource. Subclasses should unblock any blocked function ASAP and call gst_base_sink_wait_preroll(). Note: return value is ignored. See comment for BaseSink.unlock
BaseSrc.unlock_stop	subclass	GstBaseSinkClass.unlock_stop	Result<(), ErrorMessage>	yes	Clear the previous unlock request. Subclasses should clear any state they set during GstBaseSinkClass.unlock(), and be ready to continue where they left off after gst_base_sink_wait_preroll(), gst_base_sink_wait() or gst_wait_sink_wait_clock() return or GstBaseSinkClass.render() is called again. Note: return value is ignored. See comment for BaseSink.unlock
BaseTransform.sink_event	subclass	BaseTransformClass.sink_event	keep bool	yes	Event fct -> bool
BaseTransform.src_event	subclass	BaseTransformClass.src_event	keep bool	yes	Event fct -> bool
BaseTransform.start	subclass	BaseTransformClass.start	Result<(), ErrorMessage>	yes	Called when the element starts processing. Allows opening external resources. See § start & stop below
BaseTransform.stop	subclass	BaseTransformClass.stop	Result<(), ErrorMessage>	yes	Called when the element stops processing. Allows closing external resources. See § start & stop below
BaseTransform.update_src_caps	auto	gst_base_transform_update_src_caps	Result<(), glib::BoolError>	n/a	Returns TRUE if the caps could be sent downstream FALSE otherwise

Edit: updated with feedback from @slomo.

Edit: update further to #175 (comment 106495) and !214 (merged).

start & stop

In #166 (closed) and !196 (closed), we were considering using Result<(), ErrorMessage> as the return type for the start and stop functions. The idea was to make it mandatory for the implementer to characterize the error. The binding would take care of tracing the error and when it makes sense it would post the message on the bus.

In the table below, I tried to synthesize the behaviour of the start and stop function in the classes where they appear:

Function	Trace when fct returns FALSE	Msg posted on the bus	Consequence
Aggregator.start	ERROR "Subclass failed to start"	None	xxx_change_state returns GST_STATE_CHANGE_FAILURE
Aggregator.stop	ERROR "Subclass failed to stop."	None	None (change_state returns parent's ret)
BaseSink.start	DEBUG "failed to start"	ERROR CORE STATE_CHANGE "Failed to start"	xxx_change_state returns GST_STATE_CHANGE_FAILURE
BaseSink.stop	WARNING "failed to stop"	None	xxx_change_state returns GST_STATE_CHANGE_SUCCESS
BaseSrc.start	DEBUG "could not start"	ERROR CORE STATE_CHANGE "Failed to start"	xxx_activate_mode returns FALSE
BaseSrc.stop	DEBUG "Failed to stop in XXX mode"	None	xxx_activate_mode returns FALSE
BaseTransform.start	None	None	xxx_activate_mode returns FALSE
BaseTransform.stop	None	None	xxx_activate_mode returns FALSE

Each class has its own way of dealing with the start / stop failure. BaseSink.start & BaseSrc.start already post a message on the bus. Maybe it would be better to let the implementer in charge of the specific tracing / posting on the bus. If we'd adopted this approach, would it still make sense to use Result<(), ErrorMessage>? How about just Result<(), glib::BoolError> since glib::BoolError embeds the error building location now?

See reponse from @slomo in #175 (comment 105923) and #175 (comment 106331)