Re: Audio device mmap and kevents

[Tetsuya Isaki <isaki%pastel-flower.jp@localhost>]

It's better to use write(2) for this purpose.

At first, mmap'ing audio device does not lead to improve any
performance or latency.  And I think that write(2) already has
the optimal synchronization mechanism especially for stream-type
data like audio.  You don't have to monitor the kernel event.
You don't have to calculate the circular buffer's offset.

There are comments below.

At Fri, 18 Jan 2019 20:32:45 +0100,
yarl-baudig%mailoo.org@localhost wrote:
> Hello,
> 
> I would like to propose a small improvement to the audio system.
> I think that it is very interesting to be able to mmap the audio device 
> for better
> performance and smaller latency but It seems neither clean nor optimal 
> to use
> AUDIO_GETOOFFS ioctl and sleep to synchronize.
> 
> Why don't we use kernel events?
> 
> Something like audio.patch for the kernel.
> I also implemented that use in audioplay and tried to clean a bit the 
> code in audioplay.patch.
> 
> There is certainly more work to be done but I have to ask now. What do 
> you think?
> 
> Thanks.
:
> --- a/sys/dev/audio.c
> +++ b/sys/dev/audio.c
> @@ -3463,11 +3466,21 @@ filt_audiowrite(struct knote *kn, long hint)
>  	mutex_enter(sc->sc_intr_lock);
>  
>  	stream = chan->vc->sc_pustream;
> -	kn->kn_data = (stream->end - stream->start)
> -		- audio_stream_get_used(stream);
> +	if (vc->sc_mpr.mmapped) {
> +		off_t offset;
> +		offset = stream->outp - stream->start
> +			+ vc->sc_mpr.blksize;
> +		if (stream->start + offset >= stream->end)
> +			offset = 0;
> +		kn->kn_data = offset;
> +	} else {
> +		kn->kn_data = (stream->end - stream->start)
> +			- audio_stream_get_used(stream);
> +		rv = kn->kn_data > 0;
> +	}
>  	mutex_exit(sc->sc_intr_lock);
>  
> -	return kn->kn_data > 0;
> +	return rv;
>  }

filt_audiowrite() has to return zero if it's not writeable.
And it has to store the number of bytes writeable to kn->kn_data,
not offset.

> --- a/usr.bin/audio/play/audioplay.1
> +++ b/usr.bin/audio/play/audioplay.1
> @@ -118,6 +118,12 @@ sample rate.
>  Print a help message.
>  .It Fl i
>  If the audio device cannot be opened, exit now rather than wait for it.
> +.It Fl l
> +List the audio encodings supported by the device and if it emulated
> +or not. Useful to know what format to use when using

It is already implemented as "audioctl encodings" command.
# Although I don't know whether it's the optimal way or not.
And emulated flag has no meaning after 8.0.

> --- a/usr.bin/audio/play/play.c
> +++ b/usr.bin/audio/play/play.c

> -/*
> - * play the file on the file descriptor fd
> - */
> -static void
> -play_fd(const char *file, int fd)
> -{

Merging play() and play_fd() makes it harder to read.

Thanks.
---
Tetsuya Isaki <isaki%pastel-flower.jp@localhost / isaki%NetBSD.org@localhost>