On Fri, 2014-08-29 at 15:28 +0200, David Henningsson wrote:
>
> On 2014-08-29 15:08, Tanu Kaskinen wrote:
> > On Fri, 2014-08-29 at 12:57 +0200, David Henningsson wrote:
> >>
> >> On 2014-08-29 12:32, Tanu Kaskinen wrote:
> >>> On Fri, 2014-08-22 at 16:43 +0200, David Henningsson wrote:
> >>>> To sum up today's discussions on IRC, I'm ending up with a separation
> >>>> between the actual data and the capabilities describing that data:
> >>>>
> >>>> struct pa_bvolume {
> >>>> pa_volume_t volume;
> >>>> double balance[PA_CHANNELS_MAX];
> >>>> int mute;
> >>>> };
> >>>
> >>> Now that the channel map is not any more in pa_bvolume, the struct is
> >>> missing the channel count information. This makes it impossible to
> >>> validate the struct just by looking at its contents - the channel count
> >>> needs to come from somewhere else when validating the struct.To avoid
> >>> depending on external information, I propose adding a channels field to
> >>> pa_bvolume. I think it's also fine to move the channel map back to
> >>> pa_bvolume. What's your opinion?
> >>
> >> I'm not sure what type of validating that is required here, but it seems
> >> to me like anything that needs to validate using channel count also
> >> needs to check has_volume, which is not in the struct.
> >
> > The place where I need validation right now is in the tagstruct
> > deserialization for get_control_info reply. I was implementing
> > pa_tagstruct_get_bvolume(), and I realized that I didn't have knowledge
> > of how many balance values to expect. It's possible to avoid adding a
> > channels field to pa_bvolume by adding an "expected_channels" argument
> > to pa_tagstruct_get_bvolume(), or by sending the channel count for
> > bvolume at protocol level and returning that value separately from
> > pa_tagstruct_get_bvolume() like this:
> >
> > /* sender */
> > pa_tagstruct put_bvolume(t, &bvolume, channels);
> >
> > /* receiver */
> > pa_tagstruct_get_bvolume(t, &bvolume, &channels);
>
> Right, so you don't want to send a lot of unused channel information,
> but sending the two extra "mute" and "volume" are okay even if the
> control does not have mute or volume?
>
> Yeah, I guess an extra "channel" argument would do there. But sending
> the full pa_bvolume_caps in instead would work too -
> pa_tagstruct_get_bvolume could then fail in case the channel count did
> not match.
How do you suggest pa_context_set_control_volume() to work?
pa_tagstruct_put_bvolume(t, &bvolume, channels) doesn't work as well as
I imagined it to work, because the channels parameter is unknown.
My proposal is (still) to add a channels field to pa_bvolume, but there
are other ways too. The application could pass the number of channels
explicitly to pa_context_set_control_volume(), or the application could
pass the whole pa_control_volume_data object. libpulse could also
somehow cache the channel count when the application queries the volume
control info.
--
Tanu
