[alsa-devel] More snd_pcm_ioplug_avail_update() questions
Is it required that all available data has been committed between calls to snd_pcm_ioplug_avail_update() on an IO plugin capture stream?
If this is not a requirement then there can still be uncommitted data in the mmap after the current appl_ptr. This data will get overwritten by the transfer call.
If it is a requirement then snd_pcm_rate_avail_update() seems to be doing the wrong thing, because it only commits in units of the period size, which means that there could be a partial period left in the mmap.
Can someone clarify what the contract is for these calls?
Thanks,
Rob.
On Wed, 25 Jul 2018 23:52:36 +0200, Rob Duncan wrote:
Is it required that all available data has been committed between calls to snd_pcm_ioplug_avail_update() on an IO plugin capture stream?
Yes, the capture data is committed. The snd_pcm_ioplug_hw_ptr_update() should have reported the amount of data the slave PCM can actually transfer. Hence the transfer call thereafter must fulfill the whole requested data.
The capture in ioplug is a sort of mmap emulation. On real hardware, the mmap data is already committed before update_avail is called, update_avail just follows the already committed amount. In the emulation mode, it's other way round; the data is committed at the point hwptr is update, i.e. avail_update is called.
Takashi
If this is not a requirement then there can still be uncommitted data in the mmap after the current appl_ptr. This data will get overwritten by the transfer call.
If it is a requirement then snd_pcm_rate_avail_update() seems to be doing the wrong thing, because it only commits in units of the period size, which means that there could be a partial period left in the mmap.
Can someone clarify what the contract is for these calls?
Thanks,
Rob. _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
At 00:04 on Thu, Jul 26 2018, Takashi wrote:
Is it required that all available data has been committed between calls to snd_pcm_ioplug_avail_update() on an IO plugin capture stream?
Yes, the capture data is committed. The snd_pcm_ioplug_hw_ptr_update() should have reported the amount of data the slave PCM can actually transfer. Hence the transfer call thereafter must fulfill the whole requested data.
But what I see is that snd_pcm_rate_avail_update() commits data in the slave PCM in units of slave->period_size (via snd_pcm_rate_grab_next_period()), which means that if there is a partial period in the slave PCM mmap it will not be committed.
In other words, not all the available data will be commmitted before the next call to snd_pcm_ioplug_avail_update(). This means that the data will be discarded the next time that snd_pcm_ioplug_avail_update() is called.
Rob.
On Thu, 26 Jul 2018 16:19:19 +0200, Rob Duncan wrote:
At 00:04 on Thu, Jul 26 2018, Takashi wrote:
Is it required that all available data has been committed between calls to snd_pcm_ioplug_avail_update() on an IO plugin capture stream?
Yes, the capture data is committed. The snd_pcm_ioplug_hw_ptr_update() should have reported the amount of data the slave PCM can actually transfer. Hence the transfer call thereafter must fulfill the whole requested data.
But what I see is that snd_pcm_rate_avail_update() commits data in the slave PCM in units of slave->period_size (via snd_pcm_rate_grab_next_period()), which means that if there is a partial period in the slave PCM mmap it will not be committed.
In other words, not all the available data will be commmitted before the next call to snd_pcm_ioplug_avail_update(). This means that the data will be discarded the next time that snd_pcm_ioplug_avail_update() is called.
OK, I seem to have misunderstood about what you meant as committed in the context. Yes, if the available is partial, it might be not committed. But I don't understand the next part.
How will it be discarded at the next snd_pcm_ioplug_avail_update()? The data remains on the buffer, and applptr isn't changed.
Takashi
At 07:37 on Thu, Jul 26 2018, Takashi wrote:
OK, I seem to have misunderstood about what you meant as committed in the context. Yes, if the available is partial, it might be not committed. But I don't understand the next part.
How will it be discarded at the next snd_pcm_ioplug_avail_update()? The data remains on the buffer, and applptr isn't changed.
Yes, that's the problem. Because when snd_pcm_ioplug_avail_update() is subsequently called it uses snd_pcm_mmap_begin() to get the offset into the mmap for the destination of the capture transfer operation. This is essentially appl_ptr, which means that the data that has not yet been commited will be overwritten by the transfer.
I guess that the offset could somehow be adjusted to point to after the uncommitted data but I don't see a straightforward way to do that. A scheme along these lines would also have to adjust the size parameter accordingly, of course. This would sometimes mean that we cannot transfer all the available data from the IO plugin. Would that cause any issues?
Rob.
On Thu, 26 Jul 2018 17:02:33 +0200, Rob Duncan wrote:
At 07:37 on Thu, Jul 26 2018, Takashi wrote:
OK, I seem to have misunderstood about what you meant as committed in the context. Yes, if the available is partial, it might be not committed. But I don't understand the next part.
How will it be discarded at the next snd_pcm_ioplug_avail_update()? The data remains on the buffer, and applptr isn't changed.
Yes, that's the problem. Because when snd_pcm_ioplug_avail_update() is subsequently called it uses snd_pcm_mmap_begin() to get the offset into the mmap for the destination of the capture transfer operation. This is essentially appl_ptr, which means that the data that has not yet been commited will be overwritten by the transfer.
OK, point taken. Yeah, if the ioplug driver expects that the transfer happens only once, it's a problem.
I guess that the offset could somehow be adjusted to point to after the uncommitted data but I don't see a straightforward way to do that. A scheme along these lines would also have to adjust the size parameter accordingly, of course. This would sometimes mean that we cannot transfer all the available data from the IO plugin. Would that cause any issues?
Maybe we can track the own applptr (e.g. transfer_ptr or such) and calculate the transfer size from it instead of snd_pcm_mmap_begin(); i.e. write some open codes of alternative snd_pcm_mmap_begin() there.
Then transfer_ptr is updated again in snd_pcm_ioplug_mmap_commit() as well when applptr is updated.
Of course, there is a smarter way, I'd happily take another approach.
thanks,
Takashi
At 08:14 on Thu, Jul 26 2018, Takashi wrote:
Maybe we can track the own applptr (e.g. transfer_ptr or such) and calculate the transfer size from it instead of snd_pcm_mmap_begin(); i.e. write some open codes of alternative snd_pcm_mmap_begin() there.
Then transfer_ptr is updated again in snd_pcm_ioplug_mmap_commit() as well when applptr is updated.
Of course, there is a smarter way, I'd happily take another approach.
Thanks for the hint. I'm not sure I 100% understand your suggestion, though. Unfortunately I don't think I'll have an opportunity to try this for a few weeks.
Rob.
On Mon, 30 Jul 2018 20:55:45 +0200, Rob Duncan wrote:
At 08:14 on Thu, Jul 26 2018, Takashi wrote:
Maybe we can track the own applptr (e.g. transfer_ptr or such) and calculate the transfer size from it instead of snd_pcm_mmap_begin(); i.e. write some open codes of alternative snd_pcm_mmap_begin() there.
Then transfer_ptr is updated again in snd_pcm_ioplug_mmap_commit() as well when applptr is updated.
Of course, there is a smarter way, I'd happily take another approach.
Thanks for the hint. I'm not sure I 100% understand your suggestion, though.
Well, something like below (totally untested).
Takashi
--- --- a/src/pcm/pcm_ioplug.c +++ b/src/pcm/pcm_ioplug.c @@ -44,6 +44,7 @@ typedef struct snd_pcm_ioplug_priv { struct snd_ext_parm params[SND_PCM_IOPLUG_HW_PARAMS]; snd_pcm_uframes_t last_hw; snd_pcm_uframes_t avail_max; + snd_pcm_uframes_t transfer_offset; snd_htimestamp_t trigger_tstamp; } ioplug_priv_t;
@@ -154,6 +155,7 @@ static int snd_pcm_ioplug_reset(snd_pcm_t *pcm) io->data->hw_ptr = 0; io->last_hw = 0; io->avail_max = 0; + io->transfer_offset = 0; return 0; }
@@ -595,7 +597,10 @@ static snd_pcm_sframes_t snd_pcm_ioplug_rewindable(snd_pcm_t *pcm)
static snd_pcm_sframes_t snd_pcm_ioplug_rewind(snd_pcm_t *pcm, snd_pcm_uframes_t frames) { + ioplug_priv_t *io = pcm->private_data; + snd_pcm_mmap_appl_backward(pcm, frames); + io->transfer_offset = io->data->appl_ptr; return frames; }
@@ -606,7 +611,10 @@ static snd_pcm_sframes_t snd_pcm_ioplug_forwardable(snd_pcm_t *pcm)
static snd_pcm_sframes_t snd_pcm_ioplug_forward(snd_pcm_t *pcm, snd_pcm_uframes_t frames) { + ioplug_priv_t *io = pcm->private_data; + snd_pcm_mmap_appl_forward(pcm, frames); + io->transfer_offset = io->data->appl_ptr; return frames; }
@@ -723,10 +731,16 @@ static snd_pcm_sframes_t snd_pcm_ioplug_avail_update(snd_pcm_t *pcm) pcm->access != SND_PCM_ACCESS_RW_NONINTERLEAVED) { if (io->data->callback->transfer) { const snd_pcm_channel_area_t *areas; - snd_pcm_uframes_t offset, size = UINT_MAX; + snd_pcm_uframes_t offset, size; snd_pcm_sframes_t result;
- __snd_pcm_mmap_begin(pcm, &areas, &offset, &size); + areas = snd_pcm_mmap_areas(pcm); + if (!areas) + return -EBADFD; + offset = io->transfer_offset; + size = pcm->buffer_size - offset; + if (avail < size) + size = avail; result = io->data->callback->transfer(io->data, areas, offset, size); if (result < 0) return result; @@ -741,6 +755,9 @@ static snd_pcm_sframes_t snd_pcm_ioplug_avail_update(snd_pcm_t *pcm) if (result < 0) return result; } + + io->transfer_offset += avail; + io->transfer_offset %= pcm->buffer_size; } } if (avail > io->avail_max)
participants (2)
-
Rob Duncan -
Takashi Iwai