On 11 August 2014 20:31, Tanu Kaskinen <tanu.kaskinen at linux.intel.com> wrote: > On Mon, 2014-08-11 at 19:32 +0530, Arun Raghavan wrote: >> On 6 August 2014 14:15, Tanu Kaskinen <tanu.kaskinen at linux.intel.com> wrote: >> > On Wed, 2014-08-06 at 13:31 +0530, Arun Raghavan wrote: >> >> I guess we're in disagreement there. We can discuss this further, >> >> either in a separate thread, or at GstConf (or both), but I would like >> >> to get some more opinions on getting this patch in soon, since it's a >> >> practical fix that we can use now. >> > >> > Ok, so we have two proposals for preventing web apps from having access >> > to flat volume: >> > >> > 1) Disable flat volume for individual streams. >> > >> > 2) Introduce substreams that the browser would group under a single tab >> > stream. Web apps would only have access to the substream volumes, which >> > would not interact with the flat volume logic. >> > >> > I have one of my own (I already mentioned this in [1]): >> > >> > 3) Add a second volume control to streams, one which represents the >> > stream's own volume only, i.e. never flat volume. Applications that want >> > to avoid flat volume can use that volume control instead of the primary >> > volume control. >> > >> > Even if proposals 1 and 2 are implemented, I'd still like to implement >> > proposal 3 too, because it's simple (I need the second volume control at >> > server side anyway, and adding it to the client API is just a matter of >> > adding one field to pa_sink_input_info and pa_source_output_info) and >> > because it provides some new possibilities for applications: for >> > example, pavucontrol could have an option to not show flat volumes even >> > when flat volumes are enabled. >> >> While in theory I like this, I have one pretty large reservation - I >> would like to only expand the API for clearly defined use-cases. If >> not, adding another volume control for very specific use cases makes >> the overall API more complex for the general case (I am approaching >> this from a usability point-of-view for developers trying to >> understand and use our API). >> >> Do you see concrete client-side use-cases for this other than the >> flat-volume case I'm trying to solve (for which, I think my solution >> is nicer from the API perspective, if not as clean as yours, >> implementation-wise). > > I mentioned the case of a mixer application like pavucontrol that wants > to show non-flat volumes to the user (maybe with an option to switch > between the two modes). I don't have other use cases in mind. > > 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. >> 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. > 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. Relatedly, maybe the flag should be named PA_STREAM_HAS_PROGRAMMATIC_VOL_CTRL (new record for longest flag name!), or something less horrible, but closer to describing the purpose. As you point out above, we can't really treat the browser as something that's only dealing with webpages, since it can also be a runtime that we want to treat as a first-class app. I think that having a simple flag would allow this situation to be easily dealt with without increasing the API surface (imo) unduly. -- Arun
