[sugar] [PATCH] Add speaker device and icon by default
Eben Eliason
eben.eliason at gmail.com
Fri Apr 25 13:56:49 EDT 2008
On Fri, Apr 25, 2008 at 1:46 PM, Martin Dengler
<martin at martindengler.com> wrote:
> On Fri, Apr 25, 2008 at 01:21:50PM -0400, Eben Eliason wrote:
> > Alright...I'll go at it again, sits it's been sitting around for a day... =)
> >
> > > + nonzero_vols = [v for v in volumes if v > 0.0]
> >
> > Is it actually necessary to compare against a forced float (0.0) here?
>
> Probably not. The sequence elements are floats, so I figured why not.
It's not wrong. I've just never seen it done. =) Obviously, 0 is
losslessly representable as both int and float.
> > > + if len(nonzero_vols) != 0 and len(nonzero_vols) != len(volumes):
> > > + volumes = nonzero_vols
> >
> > The second condition seems unnecessary.
>
> Possibly. We want to eliminate the nonzero volumes, but what if there
> are no nonzero volumes? It's only unnecessary because sum([]) - used
If there are none at all, then the first condition short circuits
anyway, and we're in good shape.
> later on - turns out to be zero (which I thought of now because it
> wasn't 5am). So I could just replace the whole volumes = ... return
> volumne ... with:
>
> ...
> # sometimes we get a spurious zero from one/more channel(s)
> nonzero_volumes = self._mixer.get_volume(self._master)
> volume = sum(nonzero_volumes) / len(nonzero_volumes)
> ...
>
> ...and then it'd work. I'll do that.
Sounds much nicer.
> > > + #sometimes alsa fails to set all channels' volume, so try a few times
> >
> > What's the rate of failure here?
>
> About 25-50% if I hold down one of the arrow kesy while the slider has
> focus and watch alsamixer's indication of the L+R channels' volume
> from an ssh session into the laptop.
>
> > If we're always ignoring the zeros when reading the volume, does it
> > matter if we do extra work to be sure to set them all?
>
> Yes, because reading of zeros and the failure to set the volumes have
> different solutions. The comment should better read
>
> # sometimes alsa sets a channel's volume to zero rather than what we
> tell it to
>
> ...and the problem is now sufficiently clear to justify the retry code.
Yuck. And OK.
> > > + mute_item_text = 'Unmute'
> > ...
> > > + mute_item_text = 'Mute'
> >
> > These should be localized!
>
> They are:
>
>
> self._mute_item.get_child().set_text(_(mute_item_text))
Oh, I missed that. I would wrap the localization around the string
literal, so the connection is clear. (Yes, you have to use it twice
instead of once....but it was imported as _ for a reason. ;) )
- Eben
More information about the Sugar
mailing list