February 2013 - Alsa-devel - mailman.alsa-project.org

[alsa-devel] [PATCH TINYCOMPRESS 13/14] compress: Prevent divide-by-zero faults in get_hpointer()
by Richard Fitzgerald 22 Feb '13

22 Feb '13

Do not assume that the returned snd_compr_avail contains a snd_compr_tstamp with a valid sampling_rate. Check for zero before attempting to use it. diff --git a/compress.c b/compress.c index cfd0219..c0e81a3 100644 --- a/compress.c +++ b/compress.c @@ -297,10 +297,15 @@ int compress_get_hpointer(struct compress *compress, if (ioctl(compress->fd, SNDRV_COMPRESS_AVAIL, &kavail)) return oops(compress, errno, "cannot get avail"); *avail = (unsigned int)kavail.avail; - time = kavail.tstamp.pcm_io_frames / kavail.tstamp.sampling_rate; - tstamp->tv_sec = time; - time = kavail.tstamp.pcm_io_frames % kavail.tstamp.sampling_rate; - tstamp->tv_nsec = time * 1000000000 / kavail.tstamp.sampling_rate; + if (kavail.tstamp.sampling_rate != 0) { + time = kavail.tstamp.pcm_io_frames / kavail.tstamp.sampling_rate; + tstamp->tv_sec = time; + time = kavail.tstamp.pcm_io_frames % kavail.tstamp.sampling_rate; + tstamp->tv_nsec = time * 1000000000 / kavail.tstamp.sampling_rate; + } else { + tstamp->tv_sec = 0; + tstamp->tv_nsec = 0; + } return 0; } -- 1.7.2.5

1 1

[alsa-devel] [PATCH TINYCOMPRESS 12/14] compress: Add makefile for building under standard linux
by Richard Fitzgerald 22 Feb '13

22 Feb '13

Signed-off-by: Richard Fitzgerald <rf(a)opensource.wolfsonmicro.com> diff --git a/makefile.linux b/makefile.linux new file mode 100644 index 0000000..60d6c04 --- /dev/null +++ b/makefile.linux @@ -0,0 +1,35 @@ +LIB = libtinycompress +BIN = cplay +VER = 0.1 +LIBSRC = compress.c +SRC = cplay.c +LIBOBJ = ${LIBSRC:.c=.o} +OBJ = ${SRC:.c=.o} + +CC = gcc +CROSS_COMPILE = + +CFLAGS += -std=c99 -Wall -Wextra -Wunused -DVERSION=\"${VER}\" -I./include +LDFLAGS += -L. -ltinycompress +LIBLDFLAGS = -lasound + +all: libtinycompress cplay + +$(LIB): ${LIBOBJ} + @echo " LD "$@ + @${CROSS_COMPILE}${CC} ${CFLAGS} -shared -Wl,-soname,$@.so -o $@.so ${LIBOBJ} ${LIBLDFLAGS} + +$(BIN): ${OBJ} ${LIB} + @echo " LD "$@ + @${CROSS_COMPILE}${CC} ${CFLAGS} -o $@ ${OBJ} ${LDFLAGS} + + +%.o: %.c + @echo " CC "$< + @${CROSS_COMPILE}${CC} ${CFLAGS} -c -fPIC -o $@ $< + +clean: + @rm -rf ${BIN} ${OBJ} ${LIB}.so ${LIBOBJ} + + +.PHONY: all clean -- 1.7.2.5

1 1

[alsa-devel] [PATCH TINYCOMPRESS 11/14] compress: Add missing header includes
by Richard Fitzgerald 22 Feb '13

22 Feb '13

This fixes build problems caused by dependencies that were not explicitly included. diff --git a/compress.c b/compress.c index 7ee287d..34936bd 100644 --- a/compress.c +++ b/compress.c @@ -66,6 +66,7 @@ #include <sys/time.h> #include <limits.h> +#include <linux/types.h> #include <linux/ioctl.h> #define __force #define __bitwise diff --git a/cplay.c b/cplay.c index d2a968c..3af6d13 100644 --- a/cplay.c +++ b/cplay.c @@ -51,6 +51,8 @@ * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA. */ +#include <stdint.h> +#include <linux/types.h> #include <fcntl.h> #include <errno.h> #include <unistd.h> @@ -60,6 +62,7 @@ #include <signal.h> #include <stdbool.h> #include <getopt.h> +#include <sys/time.h> #define __force #define __bitwise #define __user -- 1.7.2.5

1 1

[alsa-devel] [PATCH TINYCOMPRESS 10/14] compress: Block if unable to write all data in single write()
by Richard Fitzgerald 22 Feb '13

22 Feb '13

Previously compress_write() would only block on poll() if the available space < fragment_size. If the device has a small fragment size this could lead to it never blocking and instead looping around doing many small writes. This is bad for power saving. If we were unable to write all the remaining data in a single write we want to block until the device reaches a buffer high water mark, to allow the CPU to sleep. This change will always attempt to issue the first write providing avail >= fragment_size. All subsequent loops will block on poll() before attempting another write() unless there is enough buffer space to write all remaining data. diff --git a/compress.c b/compress.c index 7c8ddc8..7ee287d 100644 --- a/compress.c +++ b/compress.c @@ -317,7 +317,8 @@ int compress_write(struct compress *compress, const void *buf, unsigned int size { struct snd_compr_avail avail; struct pollfd fds; - int to_write, written, total = 0, ret; + int to_write = 0; /* zero indicates we haven't written yet */ + int written, total = 0, ret; const char* cbuf = buf; if (!(compress->flags & COMPRESS_IN)) @@ -333,24 +334,21 @@ int compress_write(struct compress *compress, const void *buf, unsigned int size if (ioctl(compress->fd, SNDRV_COMPRESS_AVAIL, &avail)) return oops(compress, errno, "cannot get avail"); - /* we will write only when avail > fragment size */ - if (avail.avail < compress->config.fragment_size) { - /* nothing to write so wait */ - ret = poll(&fds, 1, compress->max_poll_wait_ms); - /* A pause will cause -EBADFD or zero return from driver - * This is not an error, just stop writing + if ( (avail.avail < compress->config.fragment_size) + || ((to_write != 0) && (avail.avail < size)) ) { + /* not enough space for one fragment, or we have done + * a short write and there isn't enough space for all + * the remaining data */ + ret = poll(&fds, 1, compress->max_poll_wait_ms); + /* A pause will cause -EBADFD or zero. + * This is not an error, just stop writing */ if ((ret == 0) || (ret == -EBADFD)) break; if (ret < 0) return oops(compress, errno, "poll error"); if (fds.revents & POLLOUT) { - if (ioctl(compress->fd, SNDRV_COMPRESS_AVAIL, &avail)) - return oops(compress, errno, "cannot get avail"); - if (avail.avail == 0) { - oops(compress, -EIO, "woken up even when avail is 0!!!"); - continue; - } + continue; } if (fds.revents & POLLERR) { return oops(compress, -EIO, "poll returned error!"); -- 1.7.2.5

1 1

[alsa-devel] [PATCH TINYCOMPRESS 9/14] compress: Allow client to pass "don't care" for fragment size/number
by Richard Fitzgerald 22 Feb '13

22 Feb '13

Client cannot know what is a sensible fragment size and count for the device unless it has prior knowledge of the precise hardware. To allow for generic clients not hardcoded for particular hardware, this change allows fragment_size and fragments to be passed as 0, meaning "don't care" and tinycompress will choose default values read from the driver. diff --git a/compress.c b/compress.c index 7457550..7c8ddc8 100644 --- a/compress.c +++ b/compress.c @@ -123,19 +123,14 @@ int is_compress_ready(struct compress *compress) return (compress->fd > 0) ? 1 : 0; } -static bool _is_codec_supported(struct compress *compress, struct compr_config *config) +static bool _is_codec_supported(struct compress *compress, struct compr_config *config, + const struct snd_compr_caps *caps) { - struct snd_compr_caps caps; bool codec = false; unsigned int i; - if (ioctl(compress->fd, SNDRV_COMPRESS_GET_CAPS, &caps)) { - oops(compress, errno, "cannot get device caps"); - return false; - } - - for (i = 0; i < caps.num_codecs; i++) { - if (caps.codecs[i] == config->codec->id) { + for (i = 0; i < caps->num_codecs; i++) { + if (caps->codecs[i] == config->codec->id) { /* found the codec */ codec = true; break; @@ -146,24 +141,24 @@ static bool _is_codec_supported(struct compress *compress, struct compr_config * return false; } - if (config->fragment_size < caps.min_fragment_size) { + if (config->fragment_size < caps->min_fragment_size) { oops(compress, -EINVAL, "requested fragment size %d is below min supported %d\n", - config->fragment_size, caps.min_fragment_size); + config->fragment_size, caps->min_fragment_size); return false; } - if (config->fragment_size > caps.max_fragment_size) { + if (config->fragment_size > caps->max_fragment_size) { oops(compress, -EINVAL, "requested fragment size %d is above max supported %d\n", - config->fragment_size, caps.max_fragment_size); + config->fragment_size, caps->max_fragment_size); return false; } - if (config->fragments < caps.min_fragments) { + if (config->fragments < caps->min_fragments) { oops(compress, -EINVAL, "requested fragments %d are below min supported %d\n", - config->fragments, caps.min_fragments); + config->fragments, caps->min_fragments); return false; } - if (config->fragments > caps.max_fragments) { + if (config->fragments > caps->max_fragments) { oops(compress, -EINVAL, "requested fragments %d are above max supported %d\n", - config->fragments, caps.max_fragments); + config->fragments, caps->max_fragments); return false; } @@ -213,9 +208,6 @@ struct compress *compress_open(unsigned int card, unsigned int device, if (!compress || !config) return &bad_compress; - /* take a local copy of config */ - compress->config = *config; - snprintf(fn, sizeof(fn), "/dev/snd/comprC%uD%u", card, device); compress->max_poll_wait_ms = DEFAULT_MAX_POLL_WAIT_MS; @@ -236,11 +228,27 @@ struct compress *compress_open(unsigned int card, unsigned int device, oops(compress, errno, "cannot open device '%s'", fn); goto input_fail; } + + struct snd_compr_caps caps; + if (ioctl(compress->fd, SNDRV_COMPRESS_GET_CAPS, &caps)) { + oops(compress, errno, "cannot get device caps"); + goto codec_fail; + } + + /* If caller passed "don't care" fill in default values */ + if ((config->fragment_size == 0) || (config->fragments == 0)) { + config->fragment_size = caps.min_fragment_size; + config->fragments = caps.max_fragments; + } + + /* take a local copy of config */ + compress->config = *config; + #if 0 /* FIXME need to turn this On when DSP supports * and treat in no support case */ - if (_is_codec_supported(compress, config) == false) { + if (_is_codec_supported(compress, config, &caps) == false) { oops(compress, errno, "codec not supported\n"); goto codec_fail; } diff --git a/include/tinycompress/tinycompress.h b/include/tinycompress/tinycompress.h index 1ab53c6..1309d65 100644 --- a/include/tinycompress/tinycompress.h +++ b/include/tinycompress/tinycompress.h @@ -58,6 +58,8 @@ extern "C" { #endif /* * struct compr_config: config structure, needs to be filled by app + * If fragment_size or fragments are zero, this means "don't care" + * and tinycompress will choose values that the driver supports * * @fragment_size: size of fragment requested, in bytes * @fragments: number of fragments @@ -78,11 +80,14 @@ struct snd_compr_tstamp; /* * compress_open: open a new compress stream * returns the valid struct compress on success, NULL on failure + * If config does not specify a requested fragment size, on return + * it will be updated with the size and number of fragments that + * were configured * * @card: sound card number * @device: device number * @flags: device flags can be COMPRESS_OUT or COMPRESS_IN - * @config: stream config requested + * @config: stream config requested. Returns actual fragment config */ struct compress *compress_open(unsigned int card, unsigned int device, unsigned int flags, struct compr_config *config); -- 1.7.2.5

1 1

[alsa-devel] [PATCH TINYCOMPRESS 8/14] compress: Handle case of pause() during compress_write()
by Richard Fitzgerald 22 Feb '13

22 Feb '13

If stream is paused() during a compress_write() the poll() will return 0 or -EBADFD, or the write() will return -EBADFD. This is not an error so compress_write() should not pass an error code to the caller. It should abort the write and then return the number of bytes it had written up to that point. diff --git a/compress.c b/compress.c index b706f8b..7457550 100644 --- a/compress.c +++ b/compress.c @@ -329,10 +329,13 @@ int compress_write(struct compress *compress, const void *buf, unsigned int size if (avail.avail < compress->config.fragment_size) { /* nothing to write so wait */ ret = poll(&fds, 1, compress->max_poll_wait_ms); + /* A pause will cause -EBADFD or zero return from driver + * This is not an error, just stop writing + */ + if ((ret == 0) || (ret == -EBADFD)) + break; if (ret < 0) return oops(compress, errno, "poll error"); - if (ret == 0) - return oops(compress, -EPIPE, "Poll timeout, Broken Pipe"); if (fds.revents & POLLOUT) { if (ioctl(compress->fd, SNDRV_COMPRESS_AVAIL, &avail)) return oops(compress, errno, "cannot get avail"); @@ -351,6 +354,9 @@ int compress_write(struct compress *compress, const void *buf, unsigned int size else to_write = size; written = write(compress->fd, cbuf, to_write); + /* If play was paused the write returns -EBADFD */ + if (written == -EBADFD) + break; if (written < 0) return oops(compress, errno, "write failed!\n"); -- 1.7.2.5

1 1

[alsa-devel] [PATCH TINYCOMPRESS 7/14] compress: Remove hardcoded limit on length of poll() wait
by Richard Fitzgerald 22 Feb '13

22 Feb '13

For best power-saving we want to sleep on poll() for as long as possible, we don't want to wake unnecessarily for arbitrary time limits. Adds function compress_set_max_poll_wait() so that client can configure the poll() timeout. diff --git a/compress.c b/compress.c index 5235ea4..b706f8b 100644 --- a/compress.c +++ b/compress.c @@ -77,12 +77,16 @@ #define COMPR_ERR_MAX 32 +/* Default maximum time we will wait in a poll() - 20 seconds */ +#define DEFAULT_MAX_POLL_WAIT_MS 20000 + struct compress { int fd; unsigned int flags; char error[COMPR_ERR_MAX]; struct compr_config config; int running; + int max_poll_wait_ms; }; static int oops(struct compress *compress, int e, const char *fmt, ...) @@ -214,6 +218,8 @@ struct compress *compress_open(unsigned int card, unsigned int device, snprintf(fn, sizeof(fn), "/dev/snd/comprC%uD%u", card, device); + compress->max_poll_wait_ms = DEFAULT_MAX_POLL_WAIT_MS; + compress->flags = flags; if (!((flags & COMPRESS_OUT) || (flags & COMPRESS_IN))) { oops(compress, -EINVAL, "can't deduce device direction from given flags\n"); @@ -321,8 +327,8 @@ int compress_write(struct compress *compress, const void *buf, unsigned int size /* we will write only when avail > fragment size */ if (avail.avail < compress->config.fragment_size) { - /* nothing to write so wait for 10secs */ - ret = poll(&fds, 1, 1000000); + /* nothing to write so wait */ + ret = poll(&fds, 1, compress->max_poll_wait_ms); if (ret < 0) return oops(compress, errno, "poll error"); if (ret == 0) @@ -432,3 +438,8 @@ bool is_codec_supported(unsigned int card, unsigned int device, return ret; } +void compress_set_max_poll_wait(struct compress *compress, int milliseconds) +{ + compress->max_poll_wait_ms = milliseconds; +} + diff --git a/include/tinycompress/tinycompress.h b/include/tinycompress/tinycompress.h index e285bd4..1ab53c6 100644 --- a/include/tinycompress/tinycompress.h +++ b/include/tinycompress/tinycompress.h @@ -189,6 +189,16 @@ int compress_drain(struct compress *compress); bool is_codec_supported(unsigned int card, unsigned int device, unsigned int flags, struct snd_codec *codec); +/* + * compress_set_max_poll_wait: set the maximum time tinycompress + * will wait for driver to signal a poll(). Interval is in + * milliseconds. + * Pass interval of -1 to disable timeout and make poll() wait + * until driver signals. + * If this function is not used the timeout defaults to 20 seconds. + */ +void compress_set_max_poll_wait(struct compress *compress, int milliseconds); + int is_compress_running(struct compress *compress); int is_compress_ready(struct compress *compress); -- 1.7.2.5

2 2

[alsa-devel] [PATCH TINYCOMPRESS 6/14] compress: Fix incorrect error message on compress_resume()
by Richard Fitzgerald 22 Feb '13

22 Feb '13

Was reporting compress_resume() errors as "cannot pause" diff --git a/compress.c b/compress.c index 871c19e..5235ea4 100644 --- a/compress.c +++ b/compress.c @@ -392,7 +392,7 @@ int compress_pause(struct compress *compress) int compress_resume(struct compress *compress) { if (ioctl(compress->fd, SNDRV_COMPRESS_RESUME)) - return oops(compress, errno, "cannot pause the stream\n"); + return oops(compress, errno, "cannot resume the stream\n"); return 0; } -- 1.7.2.5

1 1

[alsa-devel] [PATCH TINYCOMPRESS 5/14] compress: Take local copy of compr_config structure
by Richard Fitzgerald 22 Feb '13

22 Feb '13

Take a copy of the compr_config structure passed by the client. This allows the client to use a structure in temporary memory and avoids the possibility that tinycompress can be broken by the client changing the content of its compr_config. diff --git a/compress.c b/compress.c index c4f215c..871c19e 100644 --- a/compress.c +++ b/compress.c @@ -81,7 +81,7 @@ struct compress { int fd; unsigned int flags; char error[COMPR_ERR_MAX]; - struct compr_config *config; + struct compr_config config; int running; }; @@ -209,7 +209,8 @@ struct compress *compress_open(unsigned int card, unsigned int device, if (!compress || !config) return &bad_compress; - compress->config = config; + /* take a local copy of config */ + compress->config = *config; snprintf(fn, sizeof(fn), "/dev/snd/comprC%uD%u", card, device); @@ -319,7 +320,7 @@ int compress_write(struct compress *compress, const void *buf, unsigned int size return oops(compress, errno, "cannot get avail"); /* we will write only when avail > fragment size */ - if (avail.avail < compress->config->fragment_size) { + if (avail.avail < compress->config.fragment_size) { /* nothing to write so wait for 10secs */ ret = poll(&fds, 1, 1000000); if (ret < 0) -- 1.7.2.5

1 1

[alsa-devel] [PATCH TINYCOMPRESS 4/14] compress: Change running from bitfield to plain int
by Richard Fitzgerald 22 Feb '13

22 Feb '13

Declaring running as a single-bit bitfield does not save any space or give any coding advantages, but does leave potential opportunities for the compiler to be inefficient in how it handles bitfields compared to how it would handle a machine int. diff --git a/compress.c b/compress.c index fa2d43f..c4f215c 100644 --- a/compress.c +++ b/compress.c @@ -82,7 +82,7 @@ struct compress { unsigned int flags; char error[COMPR_ERR_MAX]; struct compr_config *config; - int running:1; + int running; }; static int oops(struct compress *compress, int e, const char *fmt, ...) -- 1.7.2.5

1 1