Re: CVS commit: src/sys/dev/ic

[Quentin Garnier <cube%cubidou.net@localhost>]

Le Tue, 30 Sep 2003 21:18:37 -0500
David Young a ecrit :
> On Tue, Sep 30, 2003 at 06:19:09AM +0200, Quentin Garnier wrote:
> > Le Mon, 29 Sep 2003 18:30:51 -0500
> > David Young a ecrit :
> > > On Sun, Sep 28, 2003 at 10:24:09PM +0000, Quentin Garnier wrote:
> > [...]
> > > > Log Message:
> > > > Change the way of setting a port value so that a change too small
> > > > to fit in the number of bits used for the port still does
> > > > something.
> > > > 
> > > > This fixes PR pkg/18741 for ac97-based hardware. Other audio
> > > > drivers might need a similar fix.
> > [...]
> > > I do not think that the kernel should second-guess the application
> > > this way. I think that most apps are (or will be) written with the
> > > expectation that the mixer settings are stateless. I.e., if I write
> > > volume=1 when the volume=0, then it should produce the same volume
> > > as when volume=1 and I write volume=1, namely volume=1.
> > 
> > But volume will never be at 1. The application will never read such
> > value from the mixer, if the delta is not 1.
> 
> 16, then, if the delta is 16. =) My concern about a stateful volume
> setting still stands.

I understand your concern, but I really don't understand your example.

What statefulness does here is that you don't necessarily get the value
you asked for. Well, this was *already* the case, except you could just
obtain no change at all.

The code I checked in tests if you wanted no change, thus in the case of
ac97 when delta is 8 most of the time, and 7 and 15 being the lowest
possible values, if you write 15 when volume was 7, you get 15. If you
write 15 then, there's no change.

My point is that anything that gets broken by this change is as dumb as
mp3blaster WRT mixer settings.

Maybe the most obvious bug here might be the fact that the ac97 mixer
doesn't round the values, but simply truncate them. Set the volume to 14,
13, 12, and down to 8 and you get 15. It's only at 7 that you get 7. Might
be stateless, but far from anything logical.

Would you prefer that? It's done in another sound driver, don't remember
which one exactly but I've seen it while I was looking for other affected
mixers. Besides, it might be enough for mp3blaster and friends and their
bogus mixer value single steps.

> > I'm ready to back this out, I'm not sure I'm ready to fix mp3blaster,
> > mplayer and all the other loosily programmed mixers.
> 
> I think that backing it out and fixing mp3blaster, etc., is the right
> thing.

Fixing mp3blaster might be a right thing to do. However, it uses OSS
calls. No ioctl to get the delta, and making a NetBSD-specific change for
a general issue doesn't look right.

Again, I'm not against backing it out, but I'm waiting for something more
convincing than "statefulness is evil, please fix mp3blaster (and mplayer,
and this, and that)". I might have spent too much time fixing mp3blaster
this week :)

-- 
Quentin Garnier - cube%cubidou.net@localhost - cube%NetBSD.org@localhost
"Feels like I'm fiddling while Rome is burning down.
Should I lay my fiddle down and take a rifle from the ground ?"
Leigh Nash/Sixpence None The Richer, Paralyzed, Divine Discontents, 2002.