From 0e72bf84b9d3a45de98bb3dbb30ef2d2aaabb4ca Mon Sep 17 00:00:00 2001 Message-Id: <0e72bf84b9d3a45de98bb3dbb30ef2d2aaabb4ca.1715426086.git.mdw@distorted.org.uk> From: Mark Wooding Date: Sat, 11 Apr 2009 20:13:04 +0100 Subject: [PATCH] Abolish playrtp's "readahead" variable (and associated -b option). There's just minbuffer and maxbuffer now. minbuffer has the same meaning as before but is additionally used for what readahead used to be. maxbuffer defaults to 2*minbuffer. Organization: Straylight/Edgeware From: Richard Kettlewell The man page mentions them, but refers you to the source code, which now goes into more detail in comment than it did before. playrtp also no longer adjusts the socket buffer size, accepting the operating system's default. Even the relatively small (~40K) Mac one gives us a couple of dozen typical-sized packets; experimentally it's quite enough to cope even with a 'make -j' of DisOrder running on the same host. --- clients/playrtp.c | 31 ++++++++++++++++--------------- doc/disorder-playrtp.1.in | 12 ++++-------- 2 files changed, 20 insertions(+), 23 deletions(-) diff --git a/clients/playrtp.c b/clients/playrtp.c index f566409..a5542db 100644 --- a/clients/playrtp.c +++ b/clients/playrtp.c @@ -81,8 +81,6 @@ #include "version.h" #include "uaudio.h" -#define readahead linux_headers_are_borked - /** @brief Obsolete synonym */ #ifndef IPV6_JOIN_GROUP # define IPV6_JOIN_GROUP IPV6_ADD_MEMBERSHIP @@ -101,11 +99,6 @@ static FILE *logfp; * We'll stop playing if there's only this many samples in the buffer. */ unsigned minbuffer = 2 * 44100 / 10; /* 0.2 seconds */ -/** @brief Buffer high watermark - * - * We'll only start playing when this many samples are available. */ -static unsigned readahead = 44100; /* 0.5 seconds */ - /** @brief Maximum buffer size * * We'll stop reading from the network if we have this many samples. */ @@ -204,7 +197,6 @@ static const struct option options[] = { { "device", required_argument, 0, 'D' }, { "min", required_argument, 0, 'm' }, { "max", required_argument, 0, 'x' }, - { "buffer", required_argument, 0, 'b' }, { "rcvbuf", required_argument, 0, 'R' }, #if HAVE_SYS_SOUNDCARD_H || EMPEG_HOST { "oss", no_argument, 0, 'o' }, @@ -440,12 +432,15 @@ static void *listen_thread(void attribute((unused)) *arg) { * Must be called with @ref lock held. */ void playrtp_fill_buffer(void) { + /* Discard current buffer contents */ while(nsamples) drop_first_packet(); info("Buffering..."); - while(nsamples < readahead) { + /* Wait until there's at least minbuffer samples available */ + while(nsamples < minbuffer) { pthread_cond_wait(&cond, &lock); } + /* Start from whatever is earliest */ next_timestamp = pheap_first(&packets)->timestamp; active = 1; } @@ -480,7 +475,6 @@ static void help(void) { "Options:\n" " --device, -D DEVICE Output device\n" " --min, -m FRAMES Buffer low water mark\n" - " --buffer, -b FRAMES Buffer high water mark\n" " --max, -x FRAMES Buffer maximum size\n" " --rcvbuf, -R BYTES Socket receive buffer size\n" " --config, -C PATH Set configuration file\n" @@ -568,7 +562,7 @@ int main(int argc, char **argv) { struct addrinfo *res; struct stringlist sl; char *sockname; - int rcvbuf, target_rcvbuf = 131072; + int rcvbuf, target_rcvbuf = 0; socklen_t len; struct ip_mreq mreq; struct ipv6_mreq mreq6; @@ -598,14 +592,13 @@ int main(int argc, char **argv) { mem_init(); if(!setlocale(LC_CTYPE, "")) fatal(errno, "error calling setlocale"); backend = uaudio_apis[0]; - while((n = getopt_long(argc, argv, "hVdD:m:b:x:L:R:M:aocC:re:P:", options, 0)) >= 0) { + while((n = getopt_long(argc, argv, "hVdD:m:x:L:R:M:aocC:re:P:", options, 0)) >= 0) { switch(n) { case 'h': help(); case 'V': version("disorder-playrtp"); case 'd': debugging = 1; break; case 'D': uaudio_set("device", optarg); break; case 'm': minbuffer = 2 * atol(optarg); break; - case 'b': readahead = 2 * atol(optarg); break; case 'x': maxbuffer = 2 * atol(optarg); break; case 'L': logfp = fopen(optarg, "w"); break; case 'R': target_rcvbuf = atoi(optarg); break; @@ -628,7 +621,7 @@ int main(int argc, char **argv) { } if(config_read(0)) fatal(0, "cannot read configuration"); if(!maxbuffer) - maxbuffer = 4 * readahead; + maxbuffer = 2 * minbuffer; argc -= optind; argv += optind; switch(argc) { @@ -794,7 +787,15 @@ int main(int argc, char **argv) { pthread_mutex_unlock(&lock); backend->activate(); pthread_mutex_lock(&lock); - /* Wait until the buffer empties out */ + /* Wait until the buffer empties out + * + * If there's a packet that we can play right now then we definitely + * continue. + * + * Also if there's at least minbuffer samples we carry on regardless and + * insert silence. The assumption is there's been a pause but more data + * is now available. + */ while(nsamples >= minbuffer || (nsamples > 0 && contains(pheap_first(&packets), next_timestamp))) { diff --git a/doc/disorder-playrtp.1.in b/doc/disorder-playrtp.1.in index c313ece..480a935 100644 --- a/doc/disorder-playrtp.1.in +++ b/doc/disorder-playrtp.1.in @@ -98,25 +98,21 @@ Display a usage message. Display version number. .SS "Buffer Control Options" You shouldn't need to use these options. +You should consult the source code for details of their effects. .TP .B \-\-min \fIFRAMES\fR, \fB\-m \fIFRAMES\fR Specifies the buffer low watermark in frames. -If the number of frames falls below this value then playing will be -stopped until the buffer fills up. -.TP -.B \-\-buffer \fIFRAMES\fR, \fB\-b \fIFRAMES\fR -Specifies the buffer high watermark in frames. -Once there are this many frames in the buffer, playing will be (re-)started. .TP .B \-\-max \fIFRAMES\fR, \fB\-x \fIFRAMES\fR Specifies the maximum buffer size in frames. If there are this many frames in the buffer then reading from the network socket will be suspended. -The default is four times the \fB\-\-buffer\fR value. +The default is twice the \fB\-\-min\fR value. .TP .B \-\-rcvbuf \fIBYTES\fR, \fB\-R \fIBYTES\fR Specifies socket receive buffer size. -The default is 131072 (128Kbytes). +The default is not to change the buffer size, i.e. you get whatever the +local operating system chooses. The buffer size will not be reduced below the operating system's default. .SH "REMOTE CONTROL" The -- [mdw]