RFC: New volume functionality for PulseAudio

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2014-02-14 at 11:55 +0100, David Henningsson wrote:
> On 02/14/2014 10:49 AM, Tanu Kaskinen wrote:
> > Does the grouping have any benefits? If pa_volume_control_info has a
> > field with this pa_cvolume2 type, then it could be argued that for
> > symmetry there should also be function
> > pa_context_set_volume_control_volume() that takes a pa_cvolume2 struct
> > as a parameter. However, when an application sets the attributes of a
> > volume control object, usually it will only modify the overall volume or
> > the balance, not both, and the channel map is immutable, so passing a
> > pa_cvolume2 struct to the setter function is inconvenient.
> 
> A volume control UI with presets would typically set both. So would any
> volume control UI do that does not look exactly the way you anticipate.
> 
> E g, pavucontrol has two sliders, one left and one right, and gnome
> volume control has main volume + balance.

I thought about pavucontrol, but not thoroughly enough. I thought that
if you change only one channel, only balance needs to be changed, but of
course that only works if the channel in question isn't the highest
volume channel.

> The grouping would also enable potential helper functions such as those
> we have for pa_cvolume today.

Good point.

I think grouping the overall volume, the balance and the channel map
together makes sense. About the struct naming: do you know what "c" in
"cvolume" refers to? I would guess that it means "channel". pa_cvolume
very concretely has per-channel volume (a pa_volume_t value for each
channel). It could be argued that pa_cvolume2 is more balance-oriented
than channel volume oriented, so here's an idea for a name without
numbers: pa_bvolume. Thoughts?

> > I'd like to keep mute controls completely separate from volume. They are
> > not inherently linked to each other (even though they very often appear
> > together).
> 
> I don't understand this argument. A mute control *is* a volume control
> (with two distinct values).

That doesn't change the fact that the volume and mute controls should be
separate: I don't want to bundle two volume controls together either.

However, I'd be ok with adding a possibly-NULL reference from a volume
control object to a mute control object and vice versa. There are other
ways to correlate a volume control with a mute control, but direct
reference would definitely make it easier.

> In this case, it's a way to avoid volume
> information lost on muted audio.
> Well, and that everything said so far in this thread would apply equally
> well to mute controls, so I guess you will come back in a few months and
> want to add another core object for mute controls if we don't get it in
> now ;-)

Indeed, I'd like to have mute control objects too. That's not so high
priority for me, however, so I didn't include it in my initial proposal.
It can be added cleanly later.

> > What would you think about putting the icon name as a separate field to
> > the pa_volume_control_info struct? I'm not against adding a proplist for
> > undocumented metadata, but let's not abuse the proplist for things that
> > are part of the documented stable API (any more than what we already
> > do).
> 
> IIRC, icon names are in proplists everywhere else, so having them in a
> proplist here as well would avoid inconsistency.

True. While I'd prefer a separate field, this is a very small issue that
can be fixed at any point in the future if we want, so let's agree to
put the icon name to a proplist for now.

> >> The volume classes however seem more like routing/policy implementation
> >> internals. They could be kept inside the routing/policy module, exported
> >> through property lists, or something like that. Either that, or I didn't
> >> really understand the concept.
> > 
> > Applications should have a unified way to query what volume classes
> > exist and what volume control they are associated with. An example of a
> > volume class is the event volume class. If applications want to access
> > that, they currently need to use a module-stream-restore specific API,
> > even though the concept of an event volume class is not specific to
> > module-stream-restore.
> 
> The concept "volume class" does not exist in module-stream-restore, at
> least not with that name.
> 
> Could you give a clear definition of what "volume classes" are? If it's
> just a way of grouping volume control objects, then volume control
> objects could just have some property of what class they belong to.

An attempt at a definition: a volume class is a named set of rules for
grouping volumes.

"A way of grouping volume control objects" is not an entirely accurate
description. You probably were thinking of a setup where each stream has
a volume control object, and those are somehow grouped by a volume
class. In my proposal streams whose volume is governed by the volume
class do not have their own volume control objects. Instead, the streams
reference the volume control object of the volume class.

This is not a very important distinction, however, because your
suggestion of using a property works either way. You could set a
"volume_class = event" property on a volume control to tell clients that
this volume control object controls the volume of event sounds.

Apart from my usual complaint about "abusing" proplists, there are some
other issues too with the property-on-a-volume-control approach:

1) You can't get a list of all volume classes in the system. Ok, you
can, by checking the "volume_class" property on every volume control
object. Not pretty, but you can do it.

2) After getting the list of all volume classes, you can't show a
human-readable label for them in a user interface, because all you have
is the non-translatable volume class identifier. The volume control
description can probably be usually used as a replacement, however, so
this is probably not a big issue in practice. (An example where reusing
the volume control label for volume classes might be problematic: on the
Nokia N9 the volume classes point to different volume controls depending
on the current routing.)

3) The human-readable label is one example of a volume class attribute.
There could be others in the future, but those can't be implemented if
there's no object to attach the attributes to.

In my opinion volume classes deserve to be treated as full-blown
objects, but if I'm the only one thinking that, I won't insist on doing
it that way. The property-on-a-volume-control approach works too.

-- 
Tanu



[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux