On 2014-06-24 15:01, Tanu Kaskinen wrote:
> On Tue, 2014-06-24 at 13:19 +0200, David Henningsson wrote:
>>
>> On 2014-06-24 11:34, Tanu Kaskinen wrote:
>>> David suggested merging the volume and mute control implementations at
>>> least in the server code. I didn't first quite understand what that
>>> would mean in practice, but now that I've worked on the code for a
>>> while, I see that David was right. A lot of duplication can be avoided
>>> if volume and mute controls share some of the implementation in one way
>>> or another. I'm not sure if this should be reflected also in the client
>>> API.
>>
>> I think it should, or all clients will have to duplicate their code...
>>
>> I remember thinking that pa_ext_volume_api_bvolume should include a "int
>> mute" field. Then every mute specific stuff, including mute controls,
>> can be skipped.
>
> I don't like that. Any struct that contains both volume and mute will
> also need "bool has_volume" and "bool has_mute" fields (or a type enum
> for either/or choice).
Wouldn't be typical for a control to have both volume and mute? E g, a
sink has both volume and mute, right? Instead of having two objects and
leave to the client to group them together, it would make more sense to
keep them as one object and let clients turn it into two if necessary.
Or, why wouldn't a volume also have a mute?
We've already added extra information in order not to lose the balance
when the volume is -inf, we could just as well add a mute too.
> I don't think pa_bvolume is the right struct to
> do that. This would make more sense to me:
>
> struct pa_control_info {
> uint32_t index;
> char *name;
> char *description;
> pa_proplist *proplist;
> pa_control_type_t type;
> void *data;
> };
>
> 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?
> };
>
> struct pa_mute_control_data {
> int mute;
> };
>
> It would be nicer to use a union containing pa_volume_control_data and
> pa_mute_control_data instead of a void data pointer, but that would make
> it impossible to extend the structs later without breaking the ABI.
>
> Note that in the future we will probably have more control types than
> just volume and mute. Filter controls should be similar to volume and
> mute controls, and perhaps routing nodes should also be modelled as
> "routing controls".
Ok, that seems to be a reasonable argument for having a
pa_control_type_t type, but for me, volume and mute could still be the
same type.
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
