On Thu, 17 Nov 2016 18:12:57 +0100,
Alan Young wrote:
>
> On 17/11/16 15:24, Alan Young wrote:
> >
> > Similar to recent dshare patch.
> >
> Update with sign-off
Sorry for the late follow up, I've been drug by other things for too
long.
> >From 6f93ee59846d2c0058ef702c1fa68d723bfb14f6 Mon Sep 17 00:00:00 2001
> From: Alan Young <consult.awy@xxxxxxxxx>
> Date: Tue, 14 Jun 2016 10:15:01 +0100
> Subject: [PATCH] pcm_rate: Do not discard slave reported delay in status
> result.
>
> snd_pcm_rate_status() gets the underlying status from the slave PCM.
> This may contain a delay value that includes elements such as codec and
> other transfer delays. Use this as the base for the returned delay
> value, adjusted for any frames buffered locally (within the rate
> plugin).
>
> Also update snd_pcm_rate_delay() similarly.
>
> Signed-off-by: Alan Young <consult.awy@xxxxxxxxx>
> ---
> src/pcm/pcm_rate.c | 46 ++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 40 insertions(+), 6 deletions(-)
>
> diff --git a/src/pcm/pcm_rate.c b/src/pcm/pcm_rate.c
> index 6184def..15383ae 100644
> --- a/src/pcm/pcm_rate.c
> +++ b/src/pcm/pcm_rate.c
> @@ -559,10 +559,9 @@ snd_pcm_rate_read_areas1(snd_pcm_t *pcm,
> pcm->channels, rate);
> }
>
> -static inline void snd_pcm_rate_sync_hwptr(snd_pcm_t *pcm)
> +static inline void snd_pcm_rate_sync_hwptr0(snd_pcm_t *pcm, snd_pcm_uframes_t slave_hw_ptr)
> {
> snd_pcm_rate_t *rate = pcm->private_data;
> - snd_pcm_uframes_t slave_hw_ptr = *rate->gen.slave->hw.ptr;
>
> if (pcm->stream != SND_PCM_STREAM_PLAYBACK)
> return;
> @@ -576,6 +575,12 @@ static inline void snd_pcm_rate_sync_hwptr(snd_pcm_t *pcm)
> rate->hw_ptr %= pcm->boundary;
> }
>
> +static inline void snd_pcm_rate_sync_hwptr(snd_pcm_t *pcm)
> +{
> + snd_pcm_rate_t *rate = pcm->private_data;
> + snd_pcm_rate_sync_hwptr0(pcm, *rate->gen.slave->hw.ptr);
> +}
> +
> static int snd_pcm_rate_hwsync(snd_pcm_t *pcm)
> {
> snd_pcm_rate_t *rate = pcm->private_data;
> @@ -586,10 +591,37 @@ static int snd_pcm_rate_hwsync(snd_pcm_t *pcm)
> return 0;
> }
>
> +static snd_pcm_uframes_t snd_pcm_rate_playback_internal_delay(snd_pcm_t *pcm)
> +{
> + snd_pcm_rate_t *rate = pcm->private_data;
> +
> + if (rate->appl_ptr < rate->last_commit_ptr) {
> + return rate->appl_ptr - rate->last_commit_ptr + pcm->boundary;
> + } else {
> + return rate->appl_ptr - rate->last_commit_ptr;
> + }
> +}
> +
> static int snd_pcm_rate_delay(snd_pcm_t *pcm, snd_pcm_sframes_t *delayp)
> {
> + snd_pcm_rate_t *rate = pcm->private_data;
> + snd_pcm_sframes_t slave_delay;
> + int err;
> +
> snd_pcm_rate_hwsync(pcm);
> - *delayp = snd_pcm_mmap_hw_avail(pcm);
> +
> + err = snd_pcm_delay(rate->gen.slave, &slave_delay);
> + if (err < 0) {
> + return err;
> + }
> +
> + if (pcm->stream == SND_PCM_STREAM_PLAYBACK) {
> + *delayp = rate->ops.input_frames(rate->obj, slave_delay)
> + + snd_pcm_rate_playback_internal_delay(pcm);
> + } else {
> + *delayp = rate->ops.output_frames(rate->obj, slave_delay)
> + + snd_pcm_mmap_capture_hw_avail(pcm);
> + }
Hmm, I'm not 100% sure through a quick review whether it's the correct
calculation. Maybe it's worth to give more comments either in the
code or in the changelog.
> return 0;
> }
>
> @@ -1083,15 +1115,17 @@ static int snd_pcm_rate_status(snd_pcm_t *pcm, snd_pcm_status_t * status)
> status->state = SND_PCM_STATE_RUNNING;
> status->trigger_tstamp = rate->trigger_tstamp;
> }
> - snd_pcm_rate_sync_hwptr(pcm);
> + snd_pcm_rate_sync_hwptr0(rate, status->hw_ptr);
This can't work.
I can fix it in my side, but OTOH, this made me wonder how you tested
the patch...
> status->appl_ptr = *pcm->appl.ptr;
> status->hw_ptr = *pcm->hw.ptr;
> if (pcm->stream == SND_PCM_STREAM_PLAYBACK) {
> - status->delay = snd_pcm_mmap_playback_hw_avail(pcm);
> + status->delay = rate->ops.input_frames(rate->obj, status->delay)
> + + snd_pcm_rate_playback_internal_delay(pcm);
> status->avail = snd_pcm_mmap_playback_avail(pcm);
> status->avail_max = rate->ops.input_frames(rate->obj, status->avail_max);
> } else {
> - status->delay = snd_pcm_mmap_capture_hw_avail(pcm);
> + status->delay = rate->ops.output_frames(rate->obj, status->delay)
> + + snd_pcm_mmap_capture_hw_avail(pcm);
> status->avail = snd_pcm_mmap_capture_avail(pcm);
> status->avail_max = rate->ops.output_frames(rate->obj, status->avail_max);
Why only playback needs the special handling while the capture
doesn't? Again, some comments would be helpful for better
understanding your changes.
thanks,
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
