Re: OSS SNDCTL_DSP_GETISPACE, non-blocking read: messy :-\

[Takashi Iwai <tiwai@suse.de>]

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

At Mon, 27 Oct 2003 13:11:58 +0100,
Andreas Mohr wrote:
> 
> Hello Takashi,
> 
> On Mon, Oct 27, 2003 at 01:01:37PM +0100, Takashi Iwai wrote:
> > At Sat, 25 Oct 2003 22:02:21 +0200,
> > Andreas Mohr wrote:
> > > Or, as a short summary:
> > > The application is perfectly well aware of how many bytes there are left
> > > to read (from calling SNDCTL_DSP_GETISPACE) and then does a read() with
> > > this amount of bytes, however since the ALSA OSS layer attempts to read
> > > this byte amount in blocks of runtime->oss.period_bytes bytes from the sound
> > > device, we ARTIFICIALLY cause a -EAGAIN to be returned due to insufficient
> > > available data, thus potentially confusing many OSS applications.
> > > 
> > > Now what to do here?
> > 
> > this problem is a bit touch, because the ALSA OSS layer does the
> > sample-rate conversion, etc.
> > when the sample rate is converted between 44.1kHz and 48kHz, some
> > round error may happen and it will be accumulated.   hence, the size
> > will be different between two cases: reading a whole period once and
> > reading a period by multiple calls.
> > 
> > i'll take a more deeper look...
> Ah, thanks! (also for the explanation given above)
> 
> Given that 1.0 is approaching, it'd certainly be useful to get such a
> problem fixed ;-)

i come to believe that it's a bug, too.
as you wrote, the fix is easy except for the rare problem what i
mentioned above.  in that case (e.g. the sample-rate conversion
between 44.1 and 48khz required), you'll still get -EAGAIN
occasionally.  but normally, it seems ok.

the attached is the patch to fix the original problem.
to take back to the old behavior (always reading a whole period),
you can write "whole-frag" command to the proc file.

anyway, i'll try a bit more to solve the problem above.


Takashi

[-- Attachment #2: oss-capture-fix.dif --]
[-- Type: application/octet-stream, Size: 2517 bytes --]

Index: alsa-kernel/core/oss/pcm_oss.c
===================================================================
RCS file: /suse/tiwai/cvs/alsa/alsa-kernel/core/oss/pcm_oss.c,v
retrieving revision 1.42
diff -u -r1.42 pcm_oss.c
--- alsa-kernel/core/oss/pcm_oss.c	30 Sep 2003 10:08:04 -0000	1.42
+++ alsa-kernel/core/oss/pcm_oss.c	27 Oct 2003 18:12:14 -0000
@@ -665,6 +665,10 @@
 		ret = snd_pcm_oss_capture_position_fixup(substream, &delay);
 		if (ret < 0)
 			break;
+#if 0 // xxx
+		if (delay < frames)
+			printk(KERN_DEBUG "insanity read %d to req %d\n", (int)delay, (int)frames);
+#endif
 		if (in_kernel) {
 			mm_segment_t fs;
 			fs = snd_enter_user();
@@ -879,16 +883,23 @@
 	while (bytes > 0) {
 		if (bytes < runtime->oss.period_bytes || runtime->oss.buffer_used > 0) {
 			if (runtime->oss.buffer_used == 0) {
-				tmp = snd_pcm_oss_read2(substream, runtime->oss.buffer, runtime->oss.period_bytes, 1);
+				size_t filled;
+				if ((! substream->oss.setup || ! substream->oss.setup->wholefrag) &&
+				    bytes < runtime->oss.period_bytes)
+					filled = bytes;
+				else
+					filled = runtime->oss.period_bytes;
+				tmp = snd_pcm_oss_read2(substream, runtime->oss.buffer, filled, 1);
 				if (tmp <= 0)
 					return xfer > 0 ? (snd_pcm_sframes_t)xfer : tmp;
 				runtime->oss.bytes += tmp;
-				runtime->oss.buffer_used = runtime->oss.period_bytes;
+				runtime->oss.buffer_used = filled;
+				runtime->oss.buffer_filled = filled;
 			}
 			tmp = bytes;
 			if ((size_t) tmp > runtime->oss.buffer_used)
 				tmp = runtime->oss.buffer_used;
-			if (copy_to_user(buf, runtime->oss.buffer + (runtime->oss.period_bytes - runtime->oss.buffer_used), tmp))
+			if (copy_to_user(buf, runtime->oss.buffer + (runtime->oss.buffer_filled - runtime->oss.buffer_used), tmp))
 				return xfer > 0 ? (snd_pcm_sframes_t)xfer : -EFAULT;
 			buf += tmp;
 			bytes -= tmp;
Index: alsa-kernel/include/pcm_oss.h
===================================================================
RCS file: /suse/tiwai/cvs/alsa/alsa-kernel/include/pcm_oss.h,v
retrieving revision 1.6
diff -u -r1.6 pcm_oss.h
--- alsa-kernel/include/pcm_oss.h	30 Sep 2003 10:08:16 -0000	1.6
+++ alsa-kernel/include/pcm_oss.h	27 Oct 2003 16:54:00 -0000
@@ -57,6 +57,7 @@
 	size_t mmap_bytes;
 	char *buffer;				/* vmallocated period */
 	size_t buffer_used;			/* used length from period buffer */
+	size_t buffer_filled;			/* filled size in the period buffer */
 	snd_pcm_plugin_t *plugin_first;
 	snd_pcm_plugin_t *plugin_last;
 	unsigned int prev_hw_ptr_interrupt;