Re: OSS sound emulation broken between 2.6.0-test2 and test3

[Martin Schlemmer <azarah@nosferatu.za.org>]

[-- Attachment #1: Type: text/plain, Size: 5172 bytes --]

On Sat, 2003-12-27 at 22:25, Martin Schlemmer wrote:
> On Sat, 2003-12-27 at 20:35, Martin J. Bligh wrote:
> > >> > Something appears to have broken OSS sound emulation between 
> > >> > test2 and test3. Best I can tell (despite the appearance of the BK logs), 
> > >> > that included ALSA updates 0.9.5 and 0.9.6. Hopefully someone who
> > >> > understands the sound architecture better than I can fix this?
> > >> > 
> > >> 
> > >> I wont say I understand it, but a quick look seems the major change is
> > >> the addition of the 'whole-frag' and 'no-silence' opts.  You might try
> > >> the following to revert what 'no-silence' change at least does:
> > >> 
> > >> --
> > >>  # echo 'xmms 0 0 no-silence' > /proc/asound/card0/pcm0p/oss
> > >>  # echo 'xmms 0 0 whole-frag' > /proc/asound/card0/pcm0p/oss
> > >> --
> > > 
> > > Thanks, that fixes it for me. I too have been seeing terrible problems
> > > with XMMS since the early 2.6 pre- kernels.
> > > 
> > > Because it only happens in XMMS I thought it was one of those
> > > application bugs brought out by scheduler changes. I now use Zinf BTW
> > > -It's better for large music collections (although not as stable or
> > > flash).
> > > 
> > > I guess someone ought to revert the standard behaviour.
> > 
> > OK, the following patch from Andrew fixes it up 80% or so, but I still 
> > don't think it's as good as test2 was - turning on whole-frag seems to
> > fix the rest of it. It's much more difficult to tell now though, so I'd 
> > like other people's opinions on it. If you want to switch between the
> > two, the above switches it on, and:
> > 
> > # echo 'clear' > /proc/asound/card0/pcm0p/oss
> > 
> > switches whole-frag back off. I'm using a 192kbps MP3 to test it, repeating
> > the first 30s of the same song again and again (I'm gonna hate that song 
> > soon ;-)). Different bitrates might give better differentation of the problem.
> > 
> > Please, please experiment with this, and let us know.
> > 
> > M.
> > 
> > --- compile/sound/core/oss/pcm_oss.c.old	Mon Nov 17 18:29:43 2003
> > +++ compile/sound/core/oss/pcm_oss.c	Sat Dec 27 10:32:30 2003
> > @@ -814,7 +814,7 @@
> >  			xfer += tmp;
> >  			if (substream->oss.setup == NULL || !substream->oss.setup->wholefrag ||
> >  			    runtime->oss.buffer_used == runtime->oss.period_bytes) {
> > -				tmp = snd_pcm_oss_write2(substream, runtime->oss.buffer, runtime->oss.buffer_used, 1);
> > +				tmp = snd_pcm_oss_write2(substream, runtime->oss.buffer, runtime->oss.period_bytes, 1);
> >  				if (tmp <= 0)
> >  					return xfer > 0 ? (snd_pcm_sframes_t)xfer : tmp;
> >  				runtime->oss.bytes += tmp;
> 
> I cannot see that this is a very clean approach.  Sure, this is how it
> was, but the test was also different:
> 
> --
> if (runtime->oss.buffer_used == runtime->oss.period_bytes) {
> --
> 
> Meaning the buffer was only written when it was full (period_bytes is
> your period size, meaning buffer size).  What you will have now, is that
> you will write the full buffer with your valid data, and whatever crud
> is left from a previous pass that used the buffer to its full extend,
> or at least more of it than this pass (which might very well be the
> reason for the 20% noise/whatever left).  Basically writing a chunk of
> crap at the end of every user buffer.
> 
> You might try something like below, but I will be honest if I do not
> know for a fact that
> 
> --
> --- a/sound/core/oss/pcm_oss.c    2003-12-27 12:53:06.000000000 +0200
> +++ b/sound/core/oss/pcm_oss.c    2003-12-27 22:00:47.323058872 +0200
> @@ -814,6 +814,12 @@
>                         xfer += tmp;
>                         if (substream->oss.setup == NULL || !substream->oss.setup->wholefrag ||
>                             runtime->oss.buffer_used == runtime->oss.period_bytes) {
> +                               if (runtime->oss.buffer_used != runtime->oss.period_bytes) {
> +
> +                                       memset(runtime->oss.buffer + runtime->oss.buffer_used,
> +                                               (u_int8_t)snd_pcm_format_silence_64(snd_pcm_oss_format_from(runtime->oss.format)),
> +                                               runtime->oss.period_bytes - runtime->oss.buffer_used);
> +                               }
>                                 tmp = snd_pcm_oss_write2(substream, runtime->oss.buffer, runtime->oss.buffer_used, 1);
>                                 if (tmp <= 0)
>                                         return xfer > 0 ? (snd_pcm_sframes_t)xfer : tmp;
> --
> 
> But I would imagine the 'silent parts' in the song (if that long, and
> dont anyhow sound like skips or choppy) will still be annoying =)

Besides for me forgetting to change runtime->oss.buffer_used to
runtime->oss.period_bytes as other patch, the memset part should really
test if it should use 8, 16, 32 or 64 bit first, and I am sure there is
more to it to get the start position aligned, etc ...  But then, I do
not see how its going to fix anything, as you will now have silence
instead of it breaking up.

-- 
Martin Schlemmer

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]