Re: Remove libao plugin's "write_size" constraint from mpd

[Alexandre Ratchov]

On Mon, Mar 05, 2012 at 11:04:00AM +0100, David Coppa wrote:
> I'd like to remove this "write_size" constraint, as suggested by
> jakemsr@ some time ago:
> 

afaiu, removing it is desirable, but might be tricky (see below). Does
removing it improve stability?

> http://marc.info/?l=openbsd-ports&m=128630571914847
> 
> http://marc.info/?l=openbsd-ports&m=128630890119517
> 
> As jake said, 1024 bytes is only 256 samples of 16-bit stereo.
> At 44.1kHz, that's only 5.8 miliseconds. If mpd takes more than
> 5.8 ms between writes, for whatever reason, it will skip.
> So, this represents a performance penalty on fast machines and
> afaik sndiod should already cope with eventual stuffups just
> fine on slower ones...
> 
> Alexandre, what do you think about this? Is it reasonable?

write() blocks until the last byte of the chuck is stored in the
device buffer. If the chuck is very large then write() will block for
very long and the program will idle instead of decoding more data; in
this case, the program doesn't use the cpu optimally which may cause
drops on slow or busy systems. So it's more efficient to write small
chuncks of audio, ideally a small multiple of the device block size.

Unfortunately, libao doesn't expose the device block size, afaiu
that's why mpd lets the user enter it manually. Using a small chuck
like 1024, should make mpd work on most setups. I see no elegant fix
that wouldn't involve libao api change or switching to another
backend.

-- Alexandre

Remove libao plugin's "write_size" constraint from mpd

[David Coppa]

Uff... Subject forgotten :(
Sorry for my lameness!

On Mon, 05 Mar 2012, David Coppa wrote:

> I'd like to remove this "write_size" constraint, as suggested by
> jakemsr@ some time ago:
> 
> http://marc.info/?l=openbsd-ports&m=128630571914847
> 
> http://marc.info/?l=openbsd-ports&m=128630890119517
> 
> As jake said, 1024 bytes is only 256 samples of 16-bit stereo.
> At 44.1kHz, that's only 5.8 miliseconds. If mpd takes more than
> 5.8 ms between writes, for whatever reason, it will skip.
> So, this represents a performance penalty on fast machines and
> afaik sndiod should already cope with eventual stuffups just
> fine on slower ones...
> 
> Alexandre, what do you think about this? Is it reasonable?
> 
> ciao
> David

Index: Makefile
===
RCS file: /cvs/ports/audio/mpd/Makefile,v
retrieving revision 1.43
diff -u -p -r1.43 Makefile
--- Makefile4 Mar 2012 16:57:19 -   1.43
+++ Makefile5 Mar 2012 10:10:23 -
@@ -2,6 +2,7 @@
 
 COMMENT =  Music Player Daemon
 DISTNAME = mpd-0.16.7
+REVISION = 0
 CATEGORIES =   audio
 HOMEPAGE = http://www.musicpd.org/
 MAINTAINER =   David Coppa 
Index: patches/patch-doc_mpd_conf_5
===
RCS file: patches/patch-doc_mpd_conf_5
diff -N patches/patch-doc_mpd_conf_5
--- /dev/null   1 Jan 1970 00:00:00 -
+++ patches/patch-doc_mpd_conf_55 Mar 2012 10:10:23 -
@@ -0,0 +1,20 @@
+$OpenBSD$
+
+Remove "write_size" constraint. See:
+http://marc.info/?l=openbsd-ports&m=128630571914847
+http://marc.info/?l=openbsd-ports&m=128630890119517
+
+--- doc/mpd.conf.5.origMon Mar  5 10:33:42 2012
 doc/mpd.conf.5 Mon Mar  5 10:34:09 2012
+@@ -390,11 +390,6 @@ for available options for some commonly used drivers. 
+ using "=", and ";" is used to separate options.  An example for oss:
+ "dsp=/dev/dsp".  An example for alsa09: "dev=hw:0,0;buf_size=4096".  The
+ default is "".
+-.TP
+-.B write_size 
+-This specifies how many bytes to write to the audio device at once.  This
+-parameter is to work around a bug in older versions of libao on sound cards
+-with very small buffers.  The default is 1024.
+ .SH REQUIRED FIFO OUTPUT PARAMETERS
+ .TP
+ .B path 
Index: patches/patch-src_output_ao_plugin_c
===
RCS file: patches/patch-src_output_ao_plugin_c
diff -N patches/patch-src_output_ao_plugin_c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ patches/patch-src_output_ao_plugin_c5 Mar 2012 10:10:23 -
@@ -0,0 +1,35 @@
+$OpenBSD$
+
+Remove "write_size" constraint. See:
+http://marc.info/?l=openbsd-ports&m=128630571914847
+http://marc.info/?l=openbsd-ports&m=128630890119517
+
+--- src/output/ao_plugin.c.origMon Mar  5 10:34:57 2012
 src/output/ao_plugin.c Mon Mar  5 10:35:21 2012
+@@ -32,7 +32,6 @@ static const ao_sample_format OUR_AO_FORMAT_INITIALIZE
+ static unsigned ao_output_ref;
+ 
+ struct ao_data {
+-  size_t write_size;
+   int driver;
+   ao_option *options;
+   ao_device *device;
+@@ -89,8 +88,6 @@ ao_output_init(G_GNUC_UNUSED const struct audio_format
+ 
+   ad->options = NULL;
+ 
+-  ad->write_size = config_get_block_unsigned(param, "write_size", 1024);
+-
+   if (ao_output_ref == 0) {
+   ao_initialize();
+   }
+@@ -230,9 +227,6 @@ ao_output_play(void *data, const void *chunk, size_t s
+  GError **error)
+ {
+   struct ao_data *ad = (struct ao_data *)data;
+-
+-  if (size > ad->write_size)
+-  size = ad->write_size;
+ 
+   if (ao_play_deconst(ad->device, chunk, size) == 0) {
+   ao_output_error(error);