[alsa-devel] [alsa-lib][RFC][PATCH 0/9] pcm: handle status/control mapping independently
Hi,
This RFC patchset is for recent discussion about new version of PCM protocol/interface (2.0.14). This is accumulated on the below patchset. http://mailman.alsa-project.org/pipermail/alsa-devel/2017-June/122066.html
In the discussion, we found an advantage to handle page mapping for status/control data of runtime of PCM substream independently. Aim of this patchset is a proof-of-concept for alsa-lib.
I tested this patchset with two machines based on x86/armv7a architectures. This looks work well without any regression. But I've never tested a case in which mapping is failed for one of the status/control data.
For your information.
Takashi Sakamoto (9): pcm: obsolete 'mmap_emulation' parameter of snd_pcm_hw_open_fd() pcm: minor code refactoring for ioctl call pcm: handle status/control mapping independently pcm: add a helper function to query status of PCM substream pcm: add a helper function to query hwptr pcm: add a helper function to query applptr pcm: add a helper function to issue avail_min pcm: add a helper function to issue applptr pcm: code refactoring to use helper functions
src/pcm/pcm_direct.c | 2 +- src/pcm/pcm_hw.c | 324 +++++++++++++++++++++++++++++++++------------------ src/pcm/pcm_local.h | 3 +- 3 files changed, 214 insertions(+), 115 deletions(-)
A function, snd_pcm_hw_open_fd(), is just for internal use. This function has an obsoleted parameter and we can remove it without any compatibility issue.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- src/pcm/pcm_direct.c | 2 +- src/pcm/pcm_hw.c | 6 ++---- src/pcm/pcm_local.h | 3 ++- 3 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/src/pcm/pcm_direct.c b/src/pcm/pcm_direct.c index 43702601..9fd376d8 100644 --- a/src/pcm/pcm_direct.c +++ b/src/pcm/pcm_direct.c @@ -1523,7 +1523,7 @@ int snd_pcm_direct_open_secondary_client(snd_pcm_t **spcmp, snd_pcm_direct_t *dm int ret; snd_pcm_t *spcm;
- ret = snd_pcm_hw_open_fd(spcmp, client_name, dmix->hw_fd, 0, 0); + ret = snd_pcm_hw_open_fd(spcmp, client_name, dmix->hw_fd, 0); if (ret < 0) { SNDERR("unable to open hardware"); return ret; diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c index 66658599..f74bd0d7 100644 --- a/src/pcm/pcm_hw.c +++ b/src/pcm/pcm_hw.c @@ -1417,15 +1417,13 @@ static const snd_pcm_fast_ops_t snd_pcm_hw_fast_ops_timer = { * \param pcmp Returns created PCM handle * \param name Name of PCM * \param fd File descriptor - * \param mmap_emulation Obsoleted parameter * \param sync_ptr_ioctl Boolean flag for sync_ptr ioctl * \retval zero on success otherwise a negative error code * \warning Using of this function might be dangerous in the sense * of compatibility reasons. The prototype might be freely * changed in future. */ -int snd_pcm_hw_open_fd(snd_pcm_t **pcmp, const char *name, - int fd, int mmap_emulation ATTRIBUTE_UNUSED, +int snd_pcm_hw_open_fd(snd_pcm_t **pcmp, const char *name, int fd, int sync_ptr_ioctl) { int ver, mode; @@ -1632,7 +1630,7 @@ int snd_pcm_hw_open(snd_pcm_t **pcmp, const char *name, } } snd_ctl_close(ctl); - return snd_pcm_hw_open_fd(pcmp, name, fd, 0, sync_ptr_ioctl); + return snd_pcm_hw_open_fd(pcmp, name, fd, sync_ptr_ioctl); _err: snd_ctl_close(ctl); return ret; diff --git a/src/pcm/pcm_local.h b/src/pcm/pcm_local.h index e4f65218..7600daa3 100644 --- a/src/pcm/pcm_local.h +++ b/src/pcm/pcm_local.h @@ -935,7 +935,8 @@ snd_pcm_open_slave(snd_pcm_t **pcmp, snd_config_t *root,
#define snd_pcm_conf_generic_id(id) _snd_conf_generic_id(id)
-int snd_pcm_hw_open_fd(snd_pcm_t **pcmp, const char *name, int fd, int mmap_emulation, int sync_ptr_ioctl); +int snd_pcm_hw_open_fd(snd_pcm_t **pcmp, const char *name, int fd, + int sync_ptr_ioctl); int __snd_pcm_mmap_emul_open(snd_pcm_t **pcmp, const char *name, snd_pcm_t *slave, int close_slave);
When error occurs, return value from ioctl(2) is -1 and error code can be got thread local variable, errno. It's OK just to check the return value without any assignment.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- src/pcm/pcm_hw.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c index f74bd0d7..cf143f2f 100644 --- a/src/pcm/pcm_hw.c +++ b/src/pcm/pcm_hw.c @@ -135,12 +135,12 @@ static int sync_ptr1(snd_pcm_hw_t *hw, struct snd_pcm_sync_ptr *ptr, int err;
ptr->flags = flags; - err = ioctl(hw->fd, SNDRV_PCM_IOCTL_SYNC_PTR, ptr); - if (err < 0) { + if (ioctl(hw->fd, SNDRV_PCM_IOCTL_SYNC_PTR, ptr) < 0) { err = -errno; SYSMSG("SNDRV_PCM_IOCTL_SYNC_PTR failed (%i)", err); return err; } + return 0; }
In ALSA PCM interface, status/control data of PCM substream can be mapped into process' VMA of application. This has an advantage to share information of the PCM substream. This operation has limitation depending on running architecture and handled device. In this case, for convenience to applications, SNDRV_PCM_IOCTL_SYNC_PTR command is available as a fallback.
In current implementation, mapping operation for status/control data of PCM substream depends each other. If the operation for one side results in failure, operation for the another side is cancelled, thus no mapping is available. However, mapping for one side has advantages because nature of these two types of data is essentially different.
This commit handles these two mapping operations independently.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- src/pcm/pcm_hw.c | 201 ++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 126 insertions(+), 75 deletions(-)
diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c index cf143f2f..7da2db5b 100644 --- a/src/pcm/pcm_hw.c +++ b/src/pcm/pcm_hw.c @@ -90,11 +90,14 @@ typedef struct { int version; int fd; int card, device, subdevice; - int sync_ptr_ioctl; - int sync_applptr; - volatile struct snd_pcm_mmap_status * mmap_status; - struct snd_pcm_mmap_control *mmap_control; + struct snd_pcm_sync_ptr *sync_ptr; + volatile struct snd_pcm_mmap_status *mmap_status; + struct snd_pcm_mmap_control *mmap_control; + int mmap_status_fallbacked; + int mmap_control_fallbacked; + int sync_applptr; + int period_event; snd_timer_t *period_timer; struct pollfd period_timer_pfd; @@ -878,96 +881,149 @@ static snd_pcm_sframes_t snd_pcm_hw_readn(snd_pcm_t *pcm, void **bufs, snd_pcm_u return xfern.result; }
-static int snd_pcm_hw_mmap_status(snd_pcm_t *pcm) +static int map_status(snd_pcm_t *pcm, struct snd_pcm_sync_ptr *sync_ptr, + int force_fallback) { snd_pcm_hw_t *hw = pcm->private_data; - struct snd_pcm_sync_ptr sync_ptr; - void *ptr; - int err; - ptr = MAP_FAILED; - if (hw->sync_ptr_ioctl == 0) - ptr = mmap(NULL, page_align(sizeof(struct snd_pcm_mmap_status)), - PROT_READ, MAP_FILE|MAP_SHARED, - hw->fd, SNDRV_PCM_MMAP_OFFSET_STATUS); - if (ptr == MAP_FAILED || ptr == NULL) { - memset(&sync_ptr, 0, sizeof(sync_ptr)); - sync_ptr.c.control.appl_ptr = 0; - sync_ptr.c.control.avail_min = 1; - err = ioctl(hw->fd, SNDRV_PCM_IOCTL_SYNC_PTR, &sync_ptr); - if (err < 0) { - err = -errno; - SYSMSG("SNDRV_PCM_IOCTL_SYNC_PTR failed (%i)", err); - return err; - } - hw->sync_ptr = calloc(1, sizeof(struct snd_pcm_sync_ptr)); - if (hw->sync_ptr == NULL) - return -ENOMEM; - hw->mmap_status = &hw->sync_ptr->s.status; - hw->mmap_control = &hw->sync_ptr->c.control; - hw->sync_ptr_ioctl = 1; + struct snd_pcm_mmap_status *mmap_status; + int fallbacked; + + mmap_status = MAP_FAILED; + if (!force_fallback) { + mmap_status = mmap(NULL, page_align(sizeof(*mmap_status)), + PROT_READ, MAP_FILE|MAP_SHARED, + hw->fd, SNDRV_PCM_MMAP_OFFSET_STATUS); + } + + /* Switch to fallback mode. */ + if (mmap_status == MAP_FAILED || mmap_status == NULL) { + mmap_status = &sync_ptr->s.status; + fallbacked = 1; } else { - hw->mmap_status = ptr; + fallbacked = 0; } - snd_pcm_set_hw_ptr(pcm, &hw->mmap_status->hw_ptr, hw->fd, SNDRV_PCM_MMAP_OFFSET_STATUS + offsetof(struct snd_pcm_mmap_status, hw_ptr)); - return 0; + + /* Initialization. */ + snd_pcm_set_hw_ptr(pcm, &mmap_status->hw_ptr, hw->fd, + SNDRV_PCM_MMAP_OFFSET_STATUS + + offsetof(struct snd_pcm_mmap_status, hw_ptr)); + + hw->mmap_status = mmap_status; + return fallbacked; }
-static int snd_pcm_hw_mmap_control(snd_pcm_t *pcm) +static int map_control(snd_pcm_t *pcm, struct snd_pcm_sync_ptr *sync_ptr, + int force_fallback) { snd_pcm_hw_t *hw = pcm->private_data; - void *ptr; - int err; - if (hw->sync_ptr == NULL) { - ptr = mmap(NULL, page_align(sizeof(struct snd_pcm_mmap_control)), - PROT_READ|PROT_WRITE, MAP_FILE|MAP_SHARED, + struct snd_pcm_mmap_control *mmap_control; + int fallbacked; + + mmap_control = MAP_FAILED; + if (!force_fallback) { + mmap_control = mmap(NULL, page_align(sizeof(*mmap_control)), + PROT_READ|PROT_WRITE, MAP_FILE|MAP_SHARED, hw->fd, SNDRV_PCM_MMAP_OFFSET_CONTROL); - if (ptr == MAP_FAILED || ptr == NULL) { - err = -errno; - SYSMSG("control mmap failed (%i)", err); - return err; - } - hw->mmap_control = ptr; + } + + /* Switch to fallback mode. */ + if (mmap_control == MAP_FAILED || mmap_control == NULL) { + mmap_control = &sync_ptr->c.control; + fallbacked = 1; } else { - hw->mmap_control->avail_min = 1; + fallbacked = 0; } - snd_pcm_set_appl_ptr(pcm, &hw->mmap_control->appl_ptr, hw->fd, SNDRV_PCM_MMAP_OFFSET_CONTROL); - return 0; + + /* Initialization. */ + mmap_control->appl_ptr = 0; + mmap_control->avail_min = 1; + snd_pcm_set_appl_ptr(pcm, &mmap_control->appl_ptr, hw->fd, + SNDRV_PCM_MMAP_OFFSET_CONTROL); + + hw->mmap_control = mmap_control; + return fallbacked; }
-static int snd_pcm_hw_munmap_status(snd_pcm_t *pcm) +static int map_status_and_control_data(snd_pcm_t *pcm, int force_fallback) { snd_pcm_hw_t *hw = pcm->private_data; - int err; - if (hw->sync_ptr_ioctl) { - free(hw->sync_ptr); - hw->sync_ptr = NULL; + struct snd_pcm_sync_ptr *sync_ptr; + + /* Preparation for fallback from failure of mmap(2). */ + sync_ptr = malloc(sizeof(*sync_ptr)); + if (sync_ptr == NULL) + return -ENOMEM; + memset(sync_ptr, 0, sizeof(*sync_ptr)); + + hw->mmap_status_fallbacked = map_status(pcm, sync_ptr, force_fallback); + hw->mmap_control_fallbacked = map_control(pcm, sync_ptr, force_fallback); + + /* Any fallback mode needs to keep the buffer. */ + if (hw->mmap_status_fallbacked > 0 || hw->mmap_control_fallbacked > 0) { + hw->sync_ptr = sync_ptr; } else { - if (munmap((void*)hw->mmap_status, page_align(sizeof(*hw->mmap_status))) < 0) { - err = -errno; - SYSMSG("status munmap failed (%i)", err); - return err; + free(sync_ptr); + hw->sync_ptr = NULL; + } + + /* Initialize when control mapping is fallbacked. */ + if (hw->mmap_control_fallbacked > 0) { + int err = ioctl(hw->fd, SNDRV_PCM_IOCTL_SYNC_PTR, &hw->sync_ptr); + if (err < 0) { + SYSMSG("SNDRV_PCM_IOCTL_SYNC_PTR failed (%i)", -errno); + return -errno; } } + return 0; }
-static int snd_pcm_hw_munmap_control(snd_pcm_t *pcm) +static int unmap_status(snd_pcm_t *pcm) { snd_pcm_hw_t *hw = pcm->private_data; - int err; - if (hw->sync_ptr_ioctl) { - free(hw->sync_ptr); - hw->sync_ptr = NULL; - } else { - if (munmap(hw->mmap_control, page_align(sizeof(*hw->mmap_control))) < 0) { - err = -errno; - SYSMSG("control munmap failed (%i)", err); - return err; + + if (!hw->mmap_status_fallbacked) { + if (munmap((void*)hw->mmap_status, + page_align(sizeof(*hw->mmap_status))) < 0) { + SYSMSG("status munmap failed (%i)", -errno); + return -errno; } } + + hw->mmap_status = NULL; + hw->mmap_status_fallbacked = 0; + return 0; }
+static int unmap_control(snd_pcm_t *pcm) +{ + snd_pcm_hw_t *hw = pcm->private_data; + + if (!hw->mmap_control_fallbacked) { + if (munmap((void *)hw->mmap_control, + page_align(sizeof(*hw->mmap_control))) < 0) { + SYSMSG("control munmap failed (%i)", -errno); + return -errno; + } + } + + hw->mmap_control = NULL; + hw->mmap_control_fallbacked = 0; + + return 0; +} + +static void unmap_status_and_control_data(snd_pcm_t *pcm) +{ + snd_pcm_hw_t *hw = pcm->private_data; + + unmap_status(pcm); + unmap_control(pcm); + if (hw->sync_ptr) + free(hw->sync_ptr); +} + static int snd_pcm_hw_mmap(snd_pcm_t *pcm ATTRIBUTE_UNUSED) { return 0; @@ -986,8 +1042,9 @@ static int snd_pcm_hw_close(snd_pcm_t *pcm) err = -errno; SYSMSG("close failed (%i)\n", err); } - snd_pcm_hw_munmap_status(pcm); - snd_pcm_hw_munmap_control(pcm); + + unmap_status_and_control_data(pcm); + free(hw); return err; } @@ -1509,7 +1566,6 @@ int snd_pcm_hw_open_fd(snd_pcm_t **pcmp, const char *name, int fd, hw->device = info.device; hw->subdevice = info.subdevice; hw->fd = fd; - hw->sync_ptr_ioctl = sync_ptr_ioctl; /* no restriction */ hw->format = SND_PCM_FORMAT_UNKNOWN; hw->rate = 0; @@ -1533,12 +1589,7 @@ int snd_pcm_hw_open_fd(snd_pcm_t **pcmp, const char *name, int fd, #endif pcm->own_state_check = 1; /* skip the common state check */
- ret = snd_pcm_hw_mmap_status(pcm); - if (ret < 0) { - snd_pcm_close(pcm); - return ret; - } - ret = snd_pcm_hw_mmap_control(pcm); + ret = map_status_and_control_data(pcm, sync_ptr_ioctl); if (ret < 0) { snd_pcm_close(pcm); return ret;
When applications requests queueing/dequeueing some PCM frames by PCM read/write operations, ALSA PCM core handles the request and moves application-side position on PCM buffer. Usually, this can be visible from user space because page frame for the position can be mapped into process' VMA.
However, when mapping operation results in failure, applications need to execute ioctl(2) with SNDRV_PCM_IOCTL_SYNC_PTR to get current position. This is important task for the applications to maintain runtime of PCM substream.
This commit adds a helper function to unify relevant the codes for readability. Original implementation has a side effect to issue the size of avail_min. This commit add a flag to purge the effect.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- src/pcm/pcm_hw.c | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-)
diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c index 7da2db5b..23211cba 100644 --- a/src/pcm/pcm_hw.c +++ b/src/pcm/pcm_hw.c @@ -160,6 +160,16 @@ static int sync_applptr(snd_pcm_hw_t *hw) return sync_ptr1(hw, &ptr, 0); }
+static int query_status(snd_pcm_hw_t *hw) +{ + if (!hw->mmap_status_fallbacked) + return 0; + + return sync_ptr1(hw, hw->sync_ptr, + SNDRV_PCM_SYNC_PTR_APPL | + SNDRV_PCM_SYNC_PTR_AVAIL_MIN); +} + static int snd_pcm_hw_clear_timer_queue(snd_pcm_hw_t *hw) { if (hw->period_timer_need_poll) { @@ -814,8 +824,10 @@ static snd_pcm_sframes_t snd_pcm_hw_writei(snd_pcm_t *pcm, const void *buffer, s xferi.buf = (char*) buffer; xferi.frames = size; xferi.result = 0; /* make valgrind happy */ - err = ioctl(fd, SNDRV_PCM_IOCTL_WRITEI_FRAMES, &xferi); - err = err >= 0 ? sync_ptr(hw, SNDRV_PCM_SYNC_PTR_APPL) : -errno; + if (ioctl(fd, SNDRV_PCM_IOCTL_WRITEI_FRAMES, &xferi) < 0) + err = -errno; + else + err = query_status(hw); #ifdef DEBUG_RW fprintf(stderr, "hw_writei: frames = %li, xferi.result = %li, err = %i\n", size, xferi.result, err); #endif @@ -833,8 +845,10 @@ static snd_pcm_sframes_t snd_pcm_hw_writen(snd_pcm_t *pcm, void **bufs, snd_pcm_ memset(&xfern, 0, sizeof(xfern)); /* make valgrind happy */ xfern.bufs = bufs; xfern.frames = size; - err = ioctl(fd, SNDRV_PCM_IOCTL_WRITEN_FRAMES, &xfern); - err = err >= 0 ? sync_ptr(hw, SNDRV_PCM_SYNC_PTR_APPL) : -errno; + if (ioctl(fd, SNDRV_PCM_IOCTL_WRITEN_FRAMES, &xfern) < 0) + err = -errno; + else + err = query_status(hw); #ifdef DEBUG_RW fprintf(stderr, "hw_writen: frames = %li, result = %li, err = %i\n", size, xfern.result, err); #endif @@ -852,8 +866,10 @@ static snd_pcm_sframes_t snd_pcm_hw_readi(snd_pcm_t *pcm, void *buffer, snd_pcm_ xferi.buf = buffer; xferi.frames = size; xferi.result = 0; /* make valgrind happy */ - err = ioctl(fd, SNDRV_PCM_IOCTL_READI_FRAMES, &xferi); - err = err >= 0 ? sync_ptr(hw, SNDRV_PCM_SYNC_PTR_APPL) : -errno; + if (ioctl(fd, SNDRV_PCM_IOCTL_READI_FRAMES, &xferi) < 0) + err = -errno; + else + err = query_status(hw); #ifdef DEBUG_RW fprintf(stderr, "hw_readi: frames = %li, result = %li, err = %i\n", size, xferi.result, err); #endif @@ -871,8 +887,10 @@ static snd_pcm_sframes_t snd_pcm_hw_readn(snd_pcm_t *pcm, void **bufs, snd_pcm_u memset(&xfern, 0, sizeof(xfern)); /* make valgrind happy */ xfern.bufs = bufs; xfern.frames = size; - err = ioctl(fd, SNDRV_PCM_IOCTL_READN_FRAMES, &xfern); - err = err >= 0 ? sync_ptr(hw, SNDRV_PCM_SYNC_PTR_APPL) : -errno; + if (ioctl(fd, SNDRV_PCM_IOCTL_READN_FRAMES, &xfern) < 0) + err = -errno; + else + err = query_status(hw); #ifdef DEBUG_RW fprintf(stderr, "hw_readn: frames = %li, result = %li, err = %i\n", size, xfern.result, err); #endif
Usual PCM drivers in kernel land can respond applications' request about recent hardware-side position of data transmission. In most cases, the request is issued by ioctl(2) with SNDRV_PCM_IOCTL_HWSYNC, then the position is shared via mapped page frame. However, when failing the mapping, applications use SNDRV_PCM_IOCTL_SYNC_PTR command to do the same thing as a fallback.
This commit adds a helper function to unify the relevant codes for readability. Original implementation has a side effect to issue current application-side position and the size of avail_min, thus has a side effect. This commit adds some flags to purge the effect.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- src/pcm/pcm_hw.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c index 23211cba..e3a4ee7c 100644 --- a/src/pcm/pcm_hw.c +++ b/src/pcm/pcm_hw.c @@ -160,6 +160,17 @@ static int sync_applptr(snd_pcm_hw_t *hw) return sync_ptr1(hw, &ptr, 0); }
+static int query_hwptr(snd_pcm_hw_t *hw) +{ + if (!hw->mmap_status_fallbacked) + return 0; + + return sync_ptr1(hw, hw->sync_ptr, + SNDRV_PCM_SYNC_PTR_HWSYNC | + SNDRV_PCM_SYNC_PTR_APPL | + SNDRV_PCM_SYNC_PTR_AVAIL_MIN); +} + static int query_status(snd_pcm_hw_t *hw) { if (!hw->mmap_status_fallbacked) @@ -585,8 +596,8 @@ static int snd_pcm_hw_hwsync(snd_pcm_t *pcm) snd_pcm_hw_t *hw = pcm->private_data; int fd = hw->fd, err; if (SNDRV_PROTOCOL_VERSION(2, 0, 3) <= hw->version) { - if (hw->sync_ptr) { - err = sync_ptr1(hw, hw->sync_ptr, SNDRV_PCM_SYNC_PTR_HWSYNC); + if (hw->mmap_status_fallbacked) { + err = query_hwptr(hw); if (err < 0) return err; } else { @@ -735,7 +746,7 @@ static snd_pcm_sframes_t snd_pcm_hw_forward(snd_pcm_t *pcm, snd_pcm_uframes_t fr } else { snd_pcm_sframes_t avail;
- err = sync_ptr(hw, SNDRV_PCM_SYNC_PTR_HWSYNC); + err = query_hwptr(hw); if (err < 0) return err; switch (FAST_PCM_STATE(hw)) {
In some operations, application-side position for data transmission is moved by ALSA PCM core. In this case, applications need to query the position. Usually, this is done via mapped page frame. However, in some cases, this is done by ioctl(2) with SNDRV_PCM_IOCTL_SYNC_PTR.
This commit adds a helper function to unify the relevant codes for readability. Additionally, to purge side effects, this commit adds a flag.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- src/pcm/pcm_hw.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c index e3a4ee7c..a5cc5e46 100644 --- a/src/pcm/pcm_hw.c +++ b/src/pcm/pcm_hw.c @@ -160,6 +160,16 @@ static int sync_applptr(snd_pcm_hw_t *hw) return sync_ptr1(hw, &ptr, 0); }
+static int query_applptr(snd_pcm_hw_t *hw) +{ + if (!hw->mmap_control_fallbacked) + return 0; + + return sync_ptr1(hw, hw->sync_ptr, + SNDRV_PCM_SYNC_PTR_APPL | + SNDRV_PCM_SYNC_PTR_AVAIL_MIN); +} + static int query_hwptr(snd_pcm_hw_t *hw) { if (!hw->mmap_status_fallbacked) @@ -644,7 +654,7 @@ static int snd_pcm_hw_reset(snd_pcm_t *pcm) SYSMSG("SNDRV_PCM_IOCTL_RESET failed (%i)", err); return err; } - return sync_ptr(hw, SNDRV_PCM_SYNC_PTR_APPL); + return query_applptr(hw); }
static int snd_pcm_hw_start(snd_pcm_t *pcm) @@ -718,7 +728,7 @@ static snd_pcm_sframes_t snd_pcm_hw_rewind(snd_pcm_t *pcm, snd_pcm_uframes_t fra SYSMSG("SNDRV_PCM_IOCTL_REWIND failed (%i)", err); return err; } - err = sync_ptr(hw, SNDRV_PCM_SYNC_PTR_APPL); + err = query_applptr(hw); if (err < 0) return err; return frames; @@ -739,7 +749,7 @@ static snd_pcm_sframes_t snd_pcm_hw_forward(snd_pcm_t *pcm, snd_pcm_uframes_t fr SYSMSG("SNDRV_PCM_IOCTL_FORWARD failed (%i)", err); return err; } - err = sync_ptr(hw, SNDRV_PCM_SYNC_PTR_APPL); + err = query_applptr(hw); if (err < 0) return err; return frames; @@ -766,7 +776,7 @@ static snd_pcm_sframes_t snd_pcm_hw_forward(snd_pcm_t *pcm, snd_pcm_uframes_t fr if (frames > (snd_pcm_uframes_t)avail) frames = avail; snd_pcm_mmap_appl_forward(pcm, frames); - err = sync_ptr(hw, 0); + err = issue_applptr(hw); if (err < 0) return err; return frames;
Runtime of PCM substream has a parameter, avail_min, as a threshold to the amount of available PCM frames in PCM buffer. Usually, this value is shared with user space via mapped page frame. However, when failing the mapping, applications perform fallback operation with SNDRV_PCM_IOCTL_SYNC_PTR.
This commit add a helper function so that the above design is explicit.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- src/pcm/pcm_hw.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c index a5cc5e46..045e7890 100644 --- a/src/pcm/pcm_hw.c +++ b/src/pcm/pcm_hw.c @@ -160,6 +160,15 @@ static int sync_applptr(snd_pcm_hw_t *hw) return sync_ptr1(hw, &ptr, 0); }
+static int issue_availmin(snd_pcm_hw_t *hw) +{ + if (!hw->mmap_control_fallbacked) + return 0; + + return sync_ptr1(hw, hw->sync_ptr, + SNDRV_PCM_SYNC_PTR_APPL); +} + static int query_applptr(snd_pcm_hw_t *hw) { if (!hw->mmap_control_fallbacked) @@ -495,7 +504,7 @@ static int snd_pcm_hw_sw_params(snd_pcm_t *pcm, snd_pcm_sw_params_t * params) params->silence_size == pcm->silence_size && old_period_event == hw->period_event) { hw->mmap_control->avail_min = params->avail_min; - return sync_ptr(hw, 0); + return issue_availmin(hw); } if (params->tstamp_type == SND_PCM_TSTAMP_TYPE_MONOTONIC_RAW && hw->version < SNDRV_PROTOCOL_VERSION(2, 0, 12)) {
When handling PCM frames by mmap operation for PCM buffer, in usual cases, applications have no need to issue application-side position into stuffs of kernel land. In the cases, specific page frame is mapped for the position. However, in a case to fail the mapping, applications can use a fallback method. This is done by ioctl(2) with SNDRV_PCM_IOCTL_SYNC_PTR.
This commit adds a helper function to unify the relevant code. Additionally, this commit adds a flag to purge side effect of the call.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- src/pcm/pcm_hw.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c index 045e7890..2d68fe12 100644 --- a/src/pcm/pcm_hw.c +++ b/src/pcm/pcm_hw.c @@ -160,6 +160,15 @@ static int sync_applptr(snd_pcm_hw_t *hw) return sync_ptr1(hw, &ptr, 0); }
+static int issue_applptr(snd_pcm_hw_t *hw) +{ + if (!hw->mmap_control_fallbacked) + return 0; + + return sync_ptr1(hw, hw->sync_ptr, + SNDRV_PCM_SYNC_PTR_AVAIL_MIN); +} + static int issue_availmin(snd_pcm_hw_t *hw) { if (!hw->mmap_control_fallbacked) @@ -592,7 +601,7 @@ static int snd_pcm_hw_status(snd_pcm_t *pcm, snd_pcm_status_t * status) static snd_pcm_state_t snd_pcm_hw_state(snd_pcm_t *pcm) { snd_pcm_hw_t *hw = pcm->private_data; - int err = sync_ptr(hw, 0); + int err = issue_applptr(hw); if (err < 0) return err; return (snd_pcm_state_t) hw->mmap_status->state; @@ -1016,7 +1025,7 @@ static int map_status_and_control_data(snd_pcm_t *pcm, int force_fallback)
/* Initialize when control mapping is fallbacked. */ if (hw->mmap_control_fallbacked > 0) { - int err = ioctl(hw->fd, SNDRV_PCM_IOCTL_SYNC_PTR, &hw->sync_ptr); + int err = issue_applptr(hw); if (err < 0) { SYSMSG("SNDRV_PCM_IOCTL_SYNC_PTR failed (%i)", -errno); return -errno; @@ -1119,7 +1128,7 @@ static snd_pcm_sframes_t snd_pcm_hw_avail_update(snd_pcm_t *pcm) snd_pcm_hw_t *hw = pcm->private_data; snd_pcm_uframes_t avail;
- sync_ptr(hw, 0); + issue_applptr(hw); avail = snd_pcm_mmap_avail(pcm); switch (FAST_PCM_STATE(hw)) { case SNDRV_PCM_STATE_RUNNING:
In former commits, some helper functions were added. This commit applies refactoring to use them.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- src/pcm/pcm_hw.c | 64 +++++++++++++++++++++++++------------------------------- 1 file changed, 28 insertions(+), 36 deletions(-)
diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c index 2d68fe12..4759b5bb 100644 --- a/src/pcm/pcm_hw.c +++ b/src/pcm/pcm_hw.c @@ -132,13 +132,11 @@ struct timespec snd_pcm_hw_fast_tstamp(snd_pcm_t *pcm) } #endif /* DOC_HIDDEN */
-static int sync_ptr1(snd_pcm_hw_t *hw, struct snd_pcm_sync_ptr *ptr, - unsigned int flags) +static int do_sync_ptr(snd_pcm_hw_t *hw, struct snd_pcm_sync_ptr *sync_ptr) { int err;
- ptr->flags = flags; - if (ioctl(hw->fd, SNDRV_PCM_IOCTL_SYNC_PTR, ptr) < 0) { + if (ioctl(hw->fd, SNDRV_PCM_IOCTL_SYNC_PTR, sync_ptr) < 0) { err = -errno; SYSMSG("SNDRV_PCM_IOCTL_SYNC_PTR failed (%i)", err); return err; @@ -147,26 +145,23 @@ static int sync_ptr1(snd_pcm_hw_t *hw, struct snd_pcm_sync_ptr *ptr, return 0; }
-static inline int sync_ptr(snd_pcm_hw_t *hw, unsigned int flags) +static int issue_applptr(snd_pcm_hw_t *hw) { - return hw->sync_ptr ? sync_ptr1(hw, hw->sync_ptr, flags) : 0; -} + struct snd_pcm_sync_ptr *sync_ptr, buf = {0};
-/* explicit notification of appl_ptr update to kernel */ -static int sync_applptr(snd_pcm_hw_t *hw) -{ - struct snd_pcm_sync_ptr ptr; - ptr.c.control = *hw->mmap_control; - return sync_ptr1(hw, &ptr, 0); -} + if (!hw->mmap_control_fallbacked) { + if (!hw->sync_applptr) + return 0;
-static int issue_applptr(snd_pcm_hw_t *hw) -{ - if (!hw->mmap_control_fallbacked) - return 0;
- return sync_ptr1(hw, hw->sync_ptr, - SNDRV_PCM_SYNC_PTR_AVAIL_MIN); + buf.c.control = *hw->mmap_control; + sync_ptr = &buf; + } else { + sync_ptr = hw->sync_ptr; + } + + sync_ptr->flags = SNDRV_PCM_SYNC_PTR_AVAIL_MIN; + return do_sync_ptr(hw, sync_ptr); }
static int issue_availmin(snd_pcm_hw_t *hw) @@ -174,8 +169,8 @@ static int issue_availmin(snd_pcm_hw_t *hw) if (!hw->mmap_control_fallbacked) return 0;
- return sync_ptr1(hw, hw->sync_ptr, - SNDRV_PCM_SYNC_PTR_APPL); + hw->sync_ptr->flags = SNDRV_PCM_SYNC_PTR_APPL; + return do_sync_ptr(hw, hw->sync_ptr); }
static int query_applptr(snd_pcm_hw_t *hw) @@ -183,9 +178,9 @@ static int query_applptr(snd_pcm_hw_t *hw) if (!hw->mmap_control_fallbacked) return 0;
- return sync_ptr1(hw, hw->sync_ptr, - SNDRV_PCM_SYNC_PTR_APPL | - SNDRV_PCM_SYNC_PTR_AVAIL_MIN); + hw->sync_ptr->flags = SNDRV_PCM_SYNC_PTR_APPL | + SNDRV_PCM_SYNC_PTR_AVAIL_MIN; + return do_sync_ptr(hw, hw->sync_ptr); }
static int query_hwptr(snd_pcm_hw_t *hw) @@ -193,10 +188,10 @@ static int query_hwptr(snd_pcm_hw_t *hw) if (!hw->mmap_status_fallbacked) return 0;
- return sync_ptr1(hw, hw->sync_ptr, - SNDRV_PCM_SYNC_PTR_HWSYNC | - SNDRV_PCM_SYNC_PTR_APPL | - SNDRV_PCM_SYNC_PTR_AVAIL_MIN); + hw->sync_ptr->flags = SNDRV_PCM_SYNC_PTR_HWSYNC | + SNDRV_PCM_SYNC_PTR_APPL | + SNDRV_PCM_SYNC_PTR_AVAIL_MIN; + return do_sync_ptr(hw, hw->sync_ptr); }
static int query_status(snd_pcm_hw_t *hw) @@ -204,9 +199,9 @@ static int query_status(snd_pcm_hw_t *hw) if (!hw->mmap_status_fallbacked) return 0;
- return sync_ptr1(hw, hw->sync_ptr, - SNDRV_PCM_SYNC_PTR_APPL | - SNDRV_PCM_SYNC_PTR_AVAIL_MIN); + hw->sync_ptr->flags = SNDRV_PCM_SYNC_PTR_APPL | + SNDRV_PCM_SYNC_PTR_AVAIL_MIN; + return do_sync_ptr(hw, hw->sync_ptr); }
static int snd_pcm_hw_clear_timer_queue(snd_pcm_hw_t *hw) @@ -1113,10 +1108,7 @@ static snd_pcm_sframes_t snd_pcm_hw_mmap_commit(snd_pcm_t *pcm, snd_pcm_hw_t *hw = pcm->private_data;
snd_pcm_mmap_appl_forward(pcm, size); - if (hw->sync_ptr) - sync_ptr(hw, 0); - else if (hw->sync_applptr) - sync_applptr(hw); + issue_applptr(hw); #ifdef DEBUG_MMAP fprintf(stderr, "appl_forward: hw_ptr = %li, appl_ptr = %li, size = %li\n", *pcm->hw.ptr, *pcm->appl.ptr, size); #endif
participants (1)
-
Takashi Sakamoto