On Wed, 2011-02-23 at 08:28 +0200, Tanu Kaskinen wrote: > On Tue, 2011-02-22 at 21:25 +0000, Colin Guthrie wrote: > > 'Twas brillig, and Tanu Kaskinen at 22/02/11 16:35 did gyre and gimble: > > > It would be nice if someone would test this with passthrough streams > > > also, but personally I think it would be ok to commit this patch to > > > master now. > > > > OK, I've applied this to master in my tree now after giving it a bit of > > a review (and removing the last bit of the commit message!) I was looking at some older commits, which helped me to notice that the client side implementation in this patch is severely lacking. My patch added the "has_volume" and "read_only_volume" fields to pa_sink_input_info, but those fields are never set. I hope I'll fix that next week, but no promises... > > Only question I had really was related to this inline if which appears a > > couple times: > > > > has_volume ? !pa_sink_input_is_volume_writable(s) : FALSE > > > > > > Would it make sense to just return FALSE from > > pa_sink_input_is_volume_writable in the case when there is no volume? > > The inline if is used when determining whether a stream has read-only > volume. If the ternary operator is removed, and only the middle part is > left, then the result is that read_only_volume gets assigned TRUE also > when there is no volume at all. I don't think that makes sense, although > it wouldn't be the end of the world, it would just have to be documented > that read_only_volume doesn't guarantee that the volume is actually > readable. > > I don't remember if I had any good reasons for having "has_volume" and > "read_only_volume" in the client api instead of "volume_readable" and > "volume_writable", except that I thought that the variables that I chose > were somehow a more "natural" encoding of the two bits. If I would have > chosen to put "readable" and "writable" to the client api, then this > conversion of course wouldn't be needed. I actually have started to feel that "volume_readable" and "volume_writable" would be better than "has_volume" and "read_only_volume" in pa_sink_input_info. What are others' opinions? One argument for "readable"/"writable" is that those terms also cover the write-only case, which will probably never occur in practice, but you never know... -- Tanu
