On Tue, 2014-07-01 at 13:16 +0200, David Henningsson wrote:
> It's more common than not to have volume bundled with a mute: almost all
> sinks, sources, and streams have that. Therefore, it feels logically
> more correct to keep them together, rather having every GUI having to
> link them together themselves.
>
> That said, I do acknowledge that there are valid use cases, especially
> for mutes without volumes. But it the answer to that really to separate
> *all* mutes from *all* volumes?
My proposal is that yes, that is the answer. I see the point of
optimizing for the common case, though, so you could give a more
concrete proposal that covers also cases where there is only one of
volume and mute. Just adding a mute field to pa_bvolume doesn't work,
because it doesn't cover the only-one-or-the-other cases.
> > The second "odd" assumption was that having pointers from controls to
> > sinks/streams/etc would be somehow relevant for sink/stream/etc
> > introspection. If you want to introspect a sink, you need a pointer from
> > the sink to the control, not the other way around.
>
> Yes, this is what I meant: sinks and sources would have pointers to the
> control. Like this:
>
> struct pa_sink_info {
> /..../
> uint32_t volume_control_index;
> uint32_t mute_control_index;
> }
>
> Are you planning to add that?
Oh, you meant that. Yes, I'm planning to add that.
> > Now, some kind of mechanism to figure out the "primary" purpose of a
> > volume control object is needed, if a UI wants to indicate that the main
> > volume currently controls the call volume, or the headphones device, for
> > example. I initially thought that it's enough to check what stream,
> > device, or audio group is using the same volume control as what is
> > assigned as the main volume control, but this is ambiguous if multiple
> > entities share the volume control.
>
> Hmm, but are you also planning to change the internals of PulseAudio so
> that if a stream and a sink share the same volume control, only one of
> them will actually apply the volume? Otherwise the volume will have
> double effect.
Yes, the shared control would be tied only to the device volume. It
doesn't really need any changes to PulseAudio internals, other than to
redirect volume change requests that come via the "legacy" volume API to
apply to the assigned volume control of the stream, which would point to
the device's volume control in this case. There's no risk of applying
the same volume twice (doing so would require conscious effort to apply
the volume in two places).
> >>>>> type is either PA_CONTROL_TYPE_VOLUME or PA_CONTROL_TYPE_MUTE. data
> >>>>> points to one of these structs depending on the control type:
> >>>>>
> >>>>> struct pa_volume_control_data {
> >>>>> pa_bvolume volume;
> >>>>> int convertible_to_dB;
> >>>>
> >>>> Btw, could you explain what this field (convertible_to_dB) means?
> >>>
> >>> Sinks and sources have the DECIBEL_VOLUME flag. That's exposed here, so
> >>> that clients know when they can convert pa_volume_t to decibels.
>
> I think we should call it "int decibel_volume" instead for consistency.
I prefer convertible_to_dB. I'll give the reasons below, but if those
don't convince you, I can rename it to decibel_volume.
"decibel_volume" is a bit misleading, because it doesn't really sound
like a boolean (and being forced to use int in the public API doesn't
help). "convertible_to_dB" is more obviously a boolean, and it's also
more clear about what the variable is supposed to be used for.
I see the consistency point, but I think in this case consistency isn't
very important, because with the new volume API clients don't need to
care about the sink's DECIBEL_VOLUME flag at all.
> > Steps are indeed something that probably should be added to volume
> > controls at some point, to allow UIs to show the user what steps are
> > available.
>
> Since pa_sink_info already have a n_volume_steps field, I think it would
> make sense to have that here too - shouldn't be too hard to add?
Yes, it shouldn't be hard. So, would you prefer to have the steps
already in the first iteration?
Note that I think that just copying n_volume_steps isn't enough. We
should also provide the pa_volume_t values that the steps correspond to,
because the distance between steps can vary within one volume control.
For example, in a hardware-only route we may have access to decibel
information but not be able to apply software volume to emulate a
step-less control. Also, this would be useful for limiting the dynamic
range the way Sailfish does.
> > Another use case for volume steps is to limit the dynamic
> > range by using artificial volume steps and setting the lowest non-mute
> > step to relatively high value. This is useful if the device has low
> > dynamic range and is too quiet on lower volumes. There might be some
> > better solution for the dynamic range problem than forcing the use of
> > artificial volume steps, but I've heard that Sailfish is using volume
> > steps for this purpose (via a custom volume API).
> >
> > I'm not sure what design you're thinking here. Earlier you suggested a
> > design where a device would have one control that has both volume and
> > mute information. Is this paragraph about a design where a device would
> > have two volume controls, one of which actually represents mute with a
> > two-step volume slider? Another possibility is that you're thinking a
> > design where a device has just one merged volume/mute control without
> > separate mute field, but that's not workable because then we couldn't
> > remember the previous volume when the device is muted.
>
> This was not a design proposal, rather the opposite, an attempt to step
> back a little.
>
> For me, the common case is bundled volume and mute. Hence one bvolume
> with both volume and mute is my primary suggestion.
>
> Then we have all the specials: mutes without volumes, per-channel mutes,
> volume in steps only, and so on. The question is which one(s) of all
> those specials do we want to, and need to, solve.
I think we need to solve "mutes without volume" and "volume in steps
only". If the design can also handle (or can be extended in a non-ugly
way later to handle) per-channel mutes, that's a plus.
> > Bundling volume and mute into one control is not feasible if we want to
> > have flexible control sharing (see my example of sharing the device
> > volume control with Totem's stream,
>
> I don't think the Totem example makes sense. If Totem wants to control
> the device volume instead of the stream volume, then Totem can just do that.
I doubt that that the Totem maintainers would accept a patch for that.
And even if they did, other media players would need "fixing" too (my
preferred player can vary over time). Configuring it in PulseAudio would
be much nicer.
However, if we don't use shared volume controls, the whole problem
disappears, so this doesn't block bundling volume and mute together.
> > Then there's the possibility that we drop the control sharing feature.
> > This makes some issues go away, with the downside that we lose the
> > transparency in how different volumes relate to each other, so if we
> > want to describe in the client API how stream volumes and mutes are
> > grouped, some other mechanism is needed in place of control object
> > sharing.
> >
> > I briefly mentioned my problem with flat volumes earlier, I'll explain
> > it a bit more here. In flat volume mode, the idea is that clients see
> > the absolute volume of the stream, i.e. the full hardware volume range
> > is available through the stream volume control. If a stream volume is
> > shared with an audio group, this becomes a problem, because the audio
> > group's volume control can't represent the absolute volume, because
> > there can be multiple streams connected to multiple devices - some
> > devices may use flat volume and some not, but even if all devices use
> > flat volume, their volume scales are different, so absolute volume means
> > different things on different devices. Therefore, the only sane choice
> > is that the audio group volume represents the relative stream volume.
> > But then, if a stream shares the volume control with an audio group, the
> > clients see the audio group's volume as the stream volume, so the
> > requirement of providing the full hw volume range is not met.
> >
> > The solution is to create a volume control object for providing access
> > to the stream absolute volume, and propagate changes to that volume
> > control internally to the audio group's volume control. But then
> > transparency is lost, i.e. clients don't see the connection between the
> > stream volume and the audio group volume, which I wanted to be visible.
> > Since the volume control sharing doesn't work in a common case like
> > this, I don't see it as a big loss if we add the constraint that one
> > volume control can be associated with only one entity. It probably makes
> > some things simpler.
>
> Yeah - where possible/convenient, let's design the API so we can add
> volume sharing later (if we figure out that's a good idea), but for now
> it seems simpler and easier to understand if one volume control controls
> one volume and nothing else.
Ok, that's a documentation issue only. That is, we should not promise
application developers anything.
--
Tanu
