Re: Remove libao plugin's write_size constraint from mpd

2012-03-07 Thread 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-portsm=128630571914847
 
 http://marc.info/?l=openbsd-portsm=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

2012-03-05 Thread 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-portsm=128630571914847
 
 http://marc.info/?l=openbsd-portsm=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 dco...@openbsd.org
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-portsm=128630571914847
+http://marc.info/?l=openbsd-portsm=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 size in bytes
+-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 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-portsm=128630571914847
+http://marc.info/?l=openbsd-portsm=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);