Hi Arun,
As you requested, I'm reviving the per-stream flat volumes discussion.
My preference would still be to add a separate "stream-local" volume
control pointer to pa_sink_input_info, but assuming that you haven't
changed your mind, that idea doesn't seem feasible to get through.
You seemed positive about extending the pa_stream API by adding
pa_stream_get_volume() and pa_stream_set_volume(), and some mechanism to
choose whether those functions should deal with the stream local volume
or the possibly-flat volume.
You suggested using a stream flag for controlling the volume mode, but
using a flag would prevent the application from switching the mode after
creating the stream, so maybe pa_stream_set_volume_mode() would be
better?
Another solution would be to have separate functions for dealing with
the stream-local volume and the possibly-flat volume. For example:
pa_stream_get_volume() (possibly-flat)
pa_stream_set_volume() (possibly-flat)
pa_stream_get_local_volume() (stream-local)
pa_stream_set_local_volume() (stream-local)
Then there's the question of whether to use pa_cvolume or something
else. My opinion is that we shouldn't use pa_cvolume - I'm trying to
move away from that representation with the volume control work. If we
don't use pa_cvolume, then the alternatives are to use pa_bvolume or
separate attributes for volume and balance (and mute?).
(A note about pa_bvolume: while the current plan is to introduce
pa_bvolume in the volume control work, I'm not 100% sure that that plan
is going to hold - I have some suspicion that pa_bvolume might not be
needed after all. Instead, it might be better to have separate
attributes for volume, balance and mute in volume controls.)
Another open question is that should pa_stream_get_volume() be an async
operation, or should the pa_stream object automatically track the stream
and device volume? I'd prefer the getter function(s) to not be
asynchronous, that is, pa_stream would automatically subscribe to stream
and device changes and cache the volume locally in the application
process. If that's what we should do, the next question is that should
*every* application subscribe to the changes even if the application
doesn't actually care about the volume? If not, then the subscription
needs to be explicitly enabled somehow, for example with
pa_stream_set_track_volume().
-- Tanu
Here's my earlier mail to which I never got an answer, in case you want
to review it before replying:
On Fri, 2014-08-15 at 20:06 +0300, Tanu Kaskinen wrote:
> On Wed, 2014-08-13 at 16:58 +0530, Arun Raghavan wrote:
> > On 11 August 2014 20:31, Tanu Kaskinen <tanu.kaskinen at linux.intel.com> wrote:
> > > I don't really agree on the API niceness argument, because in my
> > > proposal there are just two uint32_t fields added (indexes of the
> > > always-relative volume control objects), one to pa_sink_input_info and
> > > one to pa_source_output_info. I don't see much difference to adding a
> > > flag.
> >
> > It's not just a matter of number of fields, but also the number of
> > ideas the developer using our API has to walk through to use it
> > successfully. Most developers don't need to deal with this problem, so
> > we not add complexity for them.
>
> I suppose the complexity that you mean is the need to make a choice
> between using the volume_control field or the relative_volume_control
> field. The same choice has to be made with the flag: should the flag be
> used or not. When you say "don't need to deal with this problem", I
> suppose "not dealing" means being ignorant about the possibility of
> using relative volume. It may indeed be easier to be ignorant of the
> existence of the flag than being ignorant about the
> relative_volume_control field, but as soon as the developer reads
> through the list of available stream flags, s/he will be hit by the same
> complexity either way.
>
> But I don't strictly need the relative_volume_control fields in the
> client API, so I may as well leave those out, if a satisfactory solution
> is found for the UI problems in your original proposal. The fields may
> be added later if needed.
>
> > >> One option is to have the relative volume not be exposed as API, but
> > >> be the volume that is controlled if this stream flag is set (i.e.
> > >> pa_sink_input_set_volume() would modify the relative volume if the
> > >> stream flag is set).
> > >
> > > So the idea is that the browser sees relative volume, but pavucontrol
> > > sees flat volume. This would otherwise work, but since the browser and
> > > pavucontrol use the same API, there's no way for the server to know
> > > which behaviour is desired for a given set or get volume operation,
> > > except by making the flag client-specific, and that seems ugly to me.
> > > What if you want to implement pavucontrol in the browser and the
> > > in-browser-pavucontrol and the stream share the same client connection?
> > > In that case the server has absolutely no way to distinguish which
> > > behaviour is wanted for a given get/set volume operation.
> >
> > I possibly don't understand this - the browser sets the flag on the
> > stream that it wants to apply this behaviour to, so the client
> > connection it sets up is unaffected by it.
>
> Did I interpret you wrong then, you did not mean to make a difference
> between how pavucontrol and the web browser see the volume of the web
> browser's stream? What does this proposal (I mean the quoted part above:
> "One option is to have...") solve, then - how is the end result any
> different from what you proposed first?
>
> > > One solution would be to add pa_stream_set_volume() and
> > > pa_stream_get_volume() functions instead of the flag, which would be
> > > available only to the stream owner. Those functions could then have a
> > > boolean argument for deciding between relative/absolute. But then this
> > > is certainly not any simpler than the two added integers that I proposed
> > > for the introspect API...
> >
> > I like the idea of a set/get volume API, but not of exposing the
> > relative vs. absolute thing (for the same reason of added API
> > complexity). I would rather have the flag set on the stream and have
> > the API pick the right behaviour based on that.
>
> Do you mean that the flag would be a client-side-only thing? A
> client-side flag + pa_stream_get/set_volume() should work without the UI
> problems of the original proposal.
>
> If we go this route, what's your opinion about the choice between
> pa_cvolume and pa_bvolume in the pa_stream_get/set_volume() API? I'd
> suggest using pa_bvolume, because in my mind pa_cvolume is legacy stuff
> that should not be used in new APIs.
>
> One alternative would be to only add pa_stream_get_volume_control(),
> which would just return the volume control index (different index
> depending on the new flag), and actual volume manipulation would be done
> through the control API. This would require one extra
> pa_context_get_control_info_by_index() round-trip to get the initial
> volume, though...
>
