On 2015/10/20 07:27, Mike Abegg <m...@crossfireconsulting.com> wrote: > On 10/19/2015 05:12 AM, Max Kellermann wrote: > >Documentation missing (doc/protocol.xml). > Yes it is, I apologize. I had assumed it was generated from comments > Javadocs style and didn't give it much thought. I should have been more > attentive to this. > I have now spent about an hour trying to figure out how to make this, is > this set up in the make file? What is the procedure for building this part > of the documentation? I don't see any instructions anywere and running > doxygen doesn't seem to touch the protocol documentation. I have added > appropriate documentation but I have not been able to actually check to make > sure it builds correctly.
"--enable-documentation" will look for "xmlto" to convert DocBook to HTML. If you find a way to generate protocol documentation from code comments, that would be fine for me, but that's not how we do it currently. I edit the DocBook file with my text editor. > The reason for this is because if an output gets set to 0 as a result of > adjusting the master volume it will be impossible to get it out of that > state without manually setting the volume of that output specifically to > something other than 0. If you are lowering the volume this code will result > in the volume staying at 0, if you raise the volume it will result in the > volume raising by some (smallish) amount. It's something of an edge case > because it will only happen in one of two cases. 1) the master volume is set > to 0, which could alternatively be handled by a special case check to see if > all volumes are the same and if so don't bother with the complicated > rescaling nonsense. 2) when there is a large difference between two output > volumes and the master volume is lowered to near zero, the more outputs you > have the bigger a problem this is. Sounds like that scaling is somwhat fragile, and a kludge to work around a corner case where this fragility collapses.. (see below) > >>+ idle_add(IDLE_OUTPUT); > >Again: omit IDLE_OUTPUT here. > This I was pretty unsure about, so I'm fine with this change. I went the way > I did is because this is somewhat semantically inconsistent. Using the mixer > signal to mean that (volume) values on outputs have changed, when we have an > output event. But I totally see what you're saying. This does have the same > end result effect in that when ever the main volume is changed the client is > going to have to reload all output volumes, it's just different events > causing it. This might be a sign I'm going about this in the wrong way. I understand why you decided to use IDLE_OUTPUT, and it isn't completely "wrong", just not a good idea in my opinion. Our idle events are one-dimensional, and they should be optimized to reduce spurious client wakeups. A client waiting for IDLE_OUTPUT probably wants to show a list of outputs, and is not interested in volume levels; if he really was, he could easily add IDLE_MIXER to his idle mask. If every IDLE_MIXER always implies IDLE_OUTPUT, IDLE_MIXER would be redundant. What is important here is to document this behavior in the protocol documentation. > I have attached an updated patch incorporating your feedback. After making > these changes I'm starting to think that maybe this would be better > implemented by adding a scaling factor to each output and use that for > scaling from the master volume rather than setting the volume of each output > directly, which would be closer to the original concept. This would fix the > issues with volume changes resulting in both mixer and output changes > needing to be fetched (which is still an issue no matter what events are > generated), and would also address the issue of volumes getting set to 0 and > getting stuck in a fairly graceful manner. Thoughts? I would welcome an idea that fixes the scaling fragility mentioned above; I didn't like that part of the code anyway. Making the global volume a scaling factor for each output sounds like a good approach to that. Open questions: - what volume level is reported for each output? With or without global scaling applied? - what volume level is expected to be sent by the client for the "outputvolume" command? - what if global volume is 50% but the client wants one output go "all the way to eleven", that is, make it 100% - setting just one output to 100% will still apply global volume. (I guess this is where it gets complicated.) Make it consistent, and make it easy to understand and easy to use. Max _______________________________________________ mpd-devel mailing list mpd-devel@musicpd.org http://mailman.blarg.de/listinfo/mpd-devel
