On Tue, 2014-04-15 at 21:07 +0600, Alexander E. Patrakov wrote:
> 15.04.2014 16:56, Tanu Kaskinen wrote:
> > +typedef int (*pa_sink_get_mute_cb_t)(pa_sink *s, bool *mute);
> > +
> > struct pa_sink {
> > pa_msgobject parent;
> >
> > @@ -191,14 +193,14 @@ struct pa_sink {
> > * set this callback. */
> > pa_sink_cb_t write_volume; /* may be NULL */
> >
> > - /* Called when the mute setting is queried. A PA_SINK_MESSAGE_GET_MUTE
> > - * message will also be sent. Called from IO thread if PA_SINK_DEFERRED_VOLUME
> > - * flag is set otherwise from main loop context. If refresh_mute is false
> > - * neither this function is called nor a message is sent.
> > + /* Called when the mute setting is queried. Called from the IO thread if
> > + * the PA_SINK_DEFERRED_VOLUME flag is set, otherwise called from the main
> > + * thread. The implementation is expected to set the mute parameter and
> > + * return 0 on success, or return -1 on failure.
> > *
> > * You must use the function pa_sink_set_get_mute_callback() to
> > * set this callback. */
> > - pa_sink_cb_t get_mute; /* may be NULL */
> > + pa_sink_get_mute_cb_t get_mute; /* may be NULL */
>
> It is not clear from the documentation how a NULL get_mute meets the
> expectation above (i.e. get_mute == NULL has unclear semantics). Same
> for sources.
Ok. The whole idea of the get_mute callback could be explained better.
It's not really used for what you'd expect: normally, when someone calls
pa_sink_get_mute(), the callback is not called, because
pa_sink_get_mute() can simply return sink->muted.
The callback is only used as a mechanism for implementing
backend-initiated mute changes. When the sink implementation notices
that mute might have been changed (e.g. alsa sink gets a notification
about a mixer change), it can call pa_sink_get_mute() with
force_refresh=true. That will cause the get_mute() callback to be
called, and that will then make the sink mute state change if there was
an actual change in the backend.
Another mechanism is pa_sink_mute_changed(). Some sink implementations
(alsa and solaris) use pa_sink_get_mute() for notifying about changed
mute and the rest use pa_sink_mute_changed(). The pa_sink_get_mute()
approach is strange, and I think the alsa and solaris sinks should be
converted to use the more straightforward pa_sink_mute_changed() like
everyone else. pa_sink_get_mute() and the associated callback would then
become unnecessary. I plan to do this refactoring at some later point.
For now, would the following documentation be good?
/* If the sink mute can change "spontaneously" (i.e. initiated by the sink
* implementation, not by someone else calling pa_sink_set_mute()), then
* the sink implementation can notify about changed mute either by calling
* pa_sink_mute_changed() or by calling pa_sink_get_mute() with
* force_refresh=true. If the implementation chooses the latter approach,
* it should implement the get_mute callback. Otherwise get_mute can be
* NULL.
*
* This is called when pa_sink_get_mute() is called with
* force_refresh=true. This is called from the IO thread if the
* PA_SINK_DEFERRED_VOLUME flag is set, otherwise this is called from the
* main thread. On success, the implementation is expected to return 0 and
* set the mute parameter that is passed as a reference. On failure, the
* implementation is expected to return -1.
*
* You must use the function pa_sink_set_get_mute_callback() to
* set this callback. */
pa_sink_get_mute_cb_t get_mute;
--
Tanu
