From f64fb3998ccb438cbda2b8eafa8e33462f1e652d Mon Sep 17 00:00:00 2001 From: dvdli Date: Mon, 7 Dec 2020 18:34:13 +0800 Subject: fix bugs in the pcm_open_* functions 1. Align the document and the code. 2. Copy error messages to bad_pcm object for client to get more info when opening failed. --- src/pcm.c | 46 +++++++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 21 deletions(-) (limited to 'src/pcm.c') diff --git a/src/pcm.c b/src/pcm.c index 98ca9eb..17775d5 100644 --- a/src/pcm.c +++ b/src/pcm.c @@ -995,24 +995,25 @@ int pcm_close(struct pcm *pcm) * - @ref PCM_MONOTONIC * @param config The hardware and software parameters to open the PCM with. * @returns A PCM structure. - * If an error occurs allocating memory for the PCM, NULL is returned. - * Otherwise, client code should check that the PCM opened properly by calling @ref pcm_is_ready. - * If @ref pcm_is_ready, check @ref pcm_get_error for more information. + * If an error occurs, the pointer of bad_pcm is returned. + * Otherwise, it returns the pointer of PCM object. + * Client code should check that the PCM opened properly by calling @ref pcm_is_ready. + * If @ref pcm_is_ready returns false, check @ref pcm_get_error for more information. * @ingroup libtinyalsa-pcm */ struct pcm *pcm_open_by_name(const char *name, unsigned int flags, const struct pcm_config *config) { - unsigned int card, device; - if ((name[0] != 'h') - || (name[1] != 'w') - || (name[2] != ':')) { - return NULL; - } else if (sscanf(&name[3], "%u,%u", &card, &device) != 2) { - return NULL; - } - return pcm_open(card, device, flags, config); + unsigned int card, device; + if (name[0] != 'h' || name[1] != 'w' || name[2] != ':') { + oops(&bad_pcm, 0, "name format is not matched"); + return &bad_pcm; + } else if (sscanf(&name[3], "%u,%u", &card, &device) != 2) { + oops(&bad_pcm, 0, "name format is not matched"); + return &bad_pcm; + } + return pcm_open(card, device, flags, config); } /** Opens a PCM. @@ -1029,9 +1030,10 @@ struct pcm *pcm_open_by_name(const char *name, * - @ref PCM_MONOTONIC * @param config The hardware and software parameters to open the PCM with. * @returns A PCM structure. - * If an error occurs allocating memory for the PCM, NULL is returned. - * Otherwise, client code should check that the PCM opened properly by calling @ref pcm_is_ready. - * If @ref pcm_is_ready, check @ref pcm_get_error for more information. + * If an error occurs, the pointer of bad_pcm is returned. + * Otherwise, it returns the pointer of PCM object. + * Client code should check that the PCM opened properly by calling @ref pcm_is_ready. + * If @ref pcm_is_ready returns false, check @ref pcm_get_error for more information. * @ingroup libtinyalsa-pcm */ struct pcm *pcm_open(unsigned int card, unsigned int device, @@ -1042,8 +1044,10 @@ struct pcm *pcm_open(unsigned int card, unsigned int device, int rc; pcm = calloc(1, sizeof(struct pcm)); - if (!pcm) + if (!pcm) { + oops(&bad_pcm, ENOMEM, "can't allocate PCM object"); return &bad_pcm; + } /* Default to hw_ops, attemp plugin open only if hw (/dev/snd/pcm*) open fails */ pcm->ops = &hw_ops; @@ -1055,7 +1059,7 @@ struct pcm *pcm_open(unsigned int card, unsigned int device, pcm->snd_node = snd_utils_open_pcm(card, device); pcm_type = snd_utils_get_node_type(pcm->snd_node); if (!pcm->snd_node || pcm_type != SND_NODE_TYPE_PLUGIN) { - oops(pcm, -ENODEV, "no device (hw/plugin) for card(%u), device(%u)", + oops(&bad_pcm, ENODEV, "no device (hw/plugin) for card(%u), device(%u)", card, device); goto fail_close_dev_node; } @@ -1064,7 +1068,7 @@ struct pcm *pcm_open(unsigned int card, unsigned int device, } #endif if (pcm->fd < 0) { - oops(pcm, errno, "cannot open device (%u) for card (%u)", + oops(&bad_pcm, errno, "cannot open device (%u) for card (%u)", device, card); goto fail_close_dev_node; } @@ -1072,7 +1076,7 @@ struct pcm *pcm_open(unsigned int card, unsigned int device, pcm->flags = flags; if (pcm->ops->ioctl(pcm->data, SNDRV_PCM_IOCTL_INFO, &info)) { - oops(pcm, errno, "cannot get info"); + oops(&bad_pcm, errno, "cannot get info"); goto fail_close; } pcm->subdevice = info.subdevice; @@ -1082,7 +1086,7 @@ struct pcm *pcm_open(unsigned int card, unsigned int device, rc = pcm_hw_mmap_status(pcm); if (rc < 0) { - oops(pcm, errno, "mmap status failed"); + oops(&bad_pcm, errno, "mmap status failed"); goto fail; } @@ -1091,7 +1095,7 @@ struct pcm *pcm_open(unsigned int card, unsigned int device, int arg = SNDRV_PCM_TSTAMP_TYPE_MONOTONIC; rc = pcm->ops->ioctl(pcm->data, SNDRV_PCM_IOCTL_TTSTAMP, &arg); if (rc < 0) { - oops(pcm, errno, "cannot set timestamp type"); + oops(&bad_pcm, errno, "cannot set timestamp type"); goto fail; } } -- cgit v1.2.3 From 565fc0e3da9bf0ccea99eb4386a4890cdba56134 Mon Sep 17 00:00:00 2001 From: dvdli Date: Wed, 9 Dec 2020 15:45:04 +0800 Subject: fix mmap-related functions' bugs 1. sync hw ptr before calculating the avail 2. return zero when reading or writing successfully --- src/pcm.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) (limited to 'src/pcm.c') diff --git a/src/pcm.c b/src/pcm.c index 17775d5..169d982 100644 --- a/src/pcm.c +++ b/src/pcm.c @@ -1242,6 +1242,7 @@ static inline int pcm_mmap_capture_avail(struct pcm *pcm) int pcm_mmap_avail(struct pcm *pcm) { + pcm_sync_ptr(pcm, SNDRV_PCM_SYNC_PTR_HWSYNC); if (pcm->flags & PCM_IN) return pcm_mmap_capture_avail(pcm); else @@ -1552,8 +1553,14 @@ int pcm_mmap_write(struct pcm *pcm, const void *data, unsigned int count) if ((~pcm->flags) & (PCM_OUT | PCM_MMAP)) return -ENOSYS; - return pcm_mmap_transfer(pcm, (void *)data, - pcm_bytes_to_frames(pcm, count)); + unsigned int frames = pcm_bytes_to_frames(pcm, count); + int res = pcm_mmap_transfer(pcm, (void *) data, frames); + + if (res < 0) { + return res; + } + + return (unsigned int) res == frames ? 0 : -EIO; } int pcm_mmap_read(struct pcm *pcm, void *data, unsigned int count) @@ -1561,7 +1568,14 @@ int pcm_mmap_read(struct pcm *pcm, void *data, unsigned int count) if ((~pcm->flags) & (PCM_IN | PCM_MMAP)) return -ENOSYS; - return pcm_mmap_transfer(pcm, data, pcm_bytes_to_frames(pcm, count)); + unsigned int frames = pcm_bytes_to_frames(pcm, count); + int res = pcm_mmap_transfer(pcm, data, frames); + + if (res < 0) { + return res; + } + + return (unsigned int) res == frames ? 0 : -EIO; } /* Returns current read/write position in the mmap buffer with associated time stamp. */ -- cgit v1.2.3 From 855354a87d2983e190088bc5330c129094b900ca Mon Sep 17 00:00:00 2001 From: dvdli Date: Tue, 12 Jan 2021 14:53:42 +0800 Subject: remove pcm_prepare in pcm_open For the dynamic pcm device, it is valid to specify the backend after opening it. However, it is invalid to call prepare on a dynamic pcm device without connecting any backend device. We should not do pcm_prepare in pcm_open. See also: Issue #197 --- src/pcm.c | 4 ---- 1 file changed, 4 deletions(-) (limited to 'src/pcm.c') diff --git a/src/pcm.c b/src/pcm.c index 169d982..8678919 100644 --- a/src/pcm.c +++ b/src/pcm.c @@ -1101,10 +1101,6 @@ struct pcm *pcm_open(unsigned int card, unsigned int device, } #endif - /* prepare here so the user does not need to do this later */ - if (pcm_prepare(pcm)) - goto fail; - pcm->xruns = 0; return pcm; -- cgit v1.2.3 From c0f924738de2b8f5478e8b454b4ad5521802f3f6 Mon Sep 17 00:00:00 2001 From: dvdli Date: Thu, 28 Jan 2021 11:53:30 +0800 Subject: prepare in pcm_generic_transfer --- src/pcm.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/pcm.c') diff --git a/src/pcm.c b/src/pcm.c index 8678919..3d7da0c 100644 --- a/src/pcm.c +++ b/src/pcm.c @@ -1637,6 +1637,10 @@ static int pcm_generic_transfer(struct pcm *pcm, void *data, if (frames > INT_MAX) return -EINVAL; + if (pcm_state(pcm) == PCM_STATE_SETUP && pcm_prepare(pcm) != 0) { + return -1; + } + again: if (pcm->flags & PCM_MMAP) -- cgit v1.2.3 From 3694925022aa16b10978c92b9a2a69661dbbcdcf Mon Sep 17 00:00:00 2001 From: dvdli Date: Thu, 28 Jan 2021 15:03:17 +0800 Subject: Let pcm_mmap_read/write call pcm_readi/writei The pcm_readi and pcm_writei also can deal with the mmap read/write. Also mark the pcm_mmap_read/write functions as deprecated functions and update the test case. Add mmap playback support to tinyplay --- src/pcm.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src/pcm.c') diff --git a/src/pcm.c b/src/pcm.c index 3d7da0c..10e477b 100644 --- a/src/pcm.c +++ b/src/pcm.c @@ -1547,10 +1547,10 @@ int pcm_mmap_transfer(struct pcm *pcm, void *buffer, unsigned int frames) int pcm_mmap_write(struct pcm *pcm, const void *data, unsigned int count) { if ((~pcm->flags) & (PCM_OUT | PCM_MMAP)) - return -ENOSYS; + return -EINVAL; unsigned int frames = pcm_bytes_to_frames(pcm, count); - int res = pcm_mmap_transfer(pcm, (void *) data, frames); + int res = pcm_writei(pcm, (void *) data, frames); if (res < 0) { return res; @@ -1562,10 +1562,10 @@ int pcm_mmap_write(struct pcm *pcm, const void *data, unsigned int count) int pcm_mmap_read(struct pcm *pcm, void *data, unsigned int count) { if ((~pcm->flags) & (PCM_IN | PCM_MMAP)) - return -ENOSYS; + return -EINVAL; unsigned int frames = pcm_bytes_to_frames(pcm, count); - int res = pcm_mmap_transfer(pcm, data, frames); + int res = pcm_readi(pcm, data, frames); if (res < 0) { return res; -- cgit v1.2.3