| Message ID | ccd82798-b3be-7a4f-444d-47e8d5408324@IEE.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 17 Nov 2016 09:20:16 +0100, Alan Young wrote: > > snd_pcm_dshare_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 dshare > plugin). > > Note: snd_pcm_dshare_delay() is not updated. Thanks for the patch, but it doesn't look like a proper patch to be applied to the latest git tree. I guess you created a patch on top of the modified tree. Please rebase and resubmit. Also, don't forget to add your sign-off. We prefer having it in alsa-lib code like the kernel code (although it's not strictly needed). Takashi > --- > src/pcm/pcm_dshare.c | 45 ++++++++++++++++++++++++++++----------------- > 1 file changed, 28 insertions(+), 17 deletions(-) > > diff --git a/src/pcm/pcm_dshare.c b/src/pcm/pcm_dshare.c > index c5b3178..9b478a7 100644 > --- a/src/pcm/pcm_dshare.c > +++ b/src/pcm/pcm_dshare.c > @@ -157,23 +157,14 @@ static void snd_pcm_dshare_sync_area(snd_pcm_t *pcm) > /* > * synchronize hardware pointer (hw_ptr) with ours > */ > -static int snd_pcm_dshare_sync_ptr(snd_pcm_t *pcm) > +static int snd_pcm_dshare_sync_ptr0(snd_pcm_t *pcm, snd_pcm_uframes_t slave_hw_ptr) > { > snd_pcm_direct_t *dshare = pcm->private_data; > - snd_pcm_uframes_t slave_hw_ptr, old_slave_hw_ptr, avail; > + snd_pcm_uframes_t old_slave_hw_ptr, avail; > snd_pcm_sframes_t diff; > > - switch (snd_pcm_state(dshare->spcm)) { > - case SND_PCM_STATE_DISCONNECTED: > - dshare->state = SNDRV_PCM_STATE_DISCONNECTED; > - return -ENODEV; > - default: > - break; > - } > - if (dshare->slowptr) > - snd_pcm_hwsync(dshare->spcm); > old_slave_hw_ptr = dshare->slave_hw_ptr; > - slave_hw_ptr = dshare->slave_hw_ptr = *dshare->spcm->hw.ptr; > + dshare->slave_hw_ptr = slave_hw_ptr; > diff = slave_hw_ptr - old_slave_hw_ptr; > if (diff == 0) /* fast path */ > return 0; > @@ -207,6 +198,24 @@ static int snd_pcm_dshare_sync_ptr(snd_pcm_t *pcm) > return 0; > } > +static int snd_pcm_dshare_sync_ptr(snd_pcm_t *pcm) > +{ > + snd_pcm_direct_t *dshare = pcm->private_data; > + > + switch (snd_pcm_state(dshare->spcm)) { > + case SND_PCM_STATE_DISCONNECTED: > + dshare->state = SNDRV_PCM_STATE_DISCONNECTED; > + return -ENODEV; > + default: > + break; > + } > + > + if (dshare->slowptr) > + snd_pcm_hwsync(dshare->spcm); > + > + return snd_pcm_dshare_sync_ptr0(pcm, *dshare->spcm->hw.ptr); > +} > + > /* > * plugin implementation > */ > @@ -215,22 +224,24 @@ static int snd_pcm_dshare_status(snd_pcm_t *pcm, snd_pcm_status_t * status) > { > snd_pcm_direct_t *dshare = pcm->private_data; > + memset(status, 0, sizeof(*status)); > + snd_pcm_status(dshare->spcm, status); > + > switch (dshare->state) { > case SNDRV_PCM_STATE_DRAINING: > case SNDRV_PCM_STATE_RUNNING: > - snd_pcm_dshare_sync_ptr(pcm); > + snd_pcm_dshare_sync_ptr0(pcm, status->hw_ptr); > + status->delay += snd_pcm_mmap_playback_delay(pcm) > + + status->avail - dshare->spcm->buffer_size; > break; > default: > break; > } > - memset(status, 0, sizeof(*status)); > - snd_pcm_status(dshare->spcm, status); > - status->state = snd_pcm_state(dshare->spcm); > + > status->trigger_tstamp = dshare->trigger_tstamp; > status->avail = snd_pcm_mmap_playback_avail(pcm); > status->avail_max = status->avail > dshare->avail_max ? status->avail : dshare->avail_max; > dshare->avail_max = 0; > - status->delay = snd_pcm_mmap_playback_delay(pcm); > return 0; > } > -- > 2.5.5 >
On 17/11/16 10:31, Takashi Iwai wrote: > On Thu, 17 Nov 2016 09:20:16 +0100, > Alan Young wrote: >> snd_pcm_dshare_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 dshare >> plugin). >> >> Note: snd_pcm_dshare_delay() is not updated. > Thanks for the patch, but it doesn't look like a proper patch to be > applied to the latest git tree. I guess you created a patch on top of > the modified tree. > > Please rebase and resubmit. > > Also, don't forget to add your sign-off. We prefer having it in > alsa-lib code like the kernel code (although it's not strictly > needed). I can add a signoff but first I need to understand what is wrong with the patch. I did a git pull of master from git://git.alsa-project.org/alsa-lib.git, committed my patch on top (previous commit a668a94238d: "mixer: Don't install smixer modules unless python is enabled") and did a git format-patch to generate the supplied patch. What did I do wrong?
On Thu, 17 Nov 2016 15:18:04 +0100, Alan Young wrote: > > On 17/11/16 10:31, Takashi Iwai wrote: > > On Thu, 17 Nov 2016 09:20:16 +0100, > > Alan Young wrote: > >> snd_pcm_dshare_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 dshare > >> plugin). > >> > >> Note: snd_pcm_dshare_delay() is not updated. > > Thanks for the patch, but it doesn't look like a proper patch to be > > applied to the latest git tree. I guess you created a patch on top of > > the modified tree. > > > > Please rebase and resubmit. > > > > Also, don't forget to add your sign-off. We prefer having it in > > alsa-lib code like the kernel code (although it's not strictly > > needed). > > I can add a signoff but first I need to understand what is wrong with > the patch. > > I did a git pull of master from > git://git.alsa-project.org/alsa-lib.git, committed my patch on top > (previous commit a668a94238d: "mixer: Don't install smixer modules > unless python is enabled") and did a git format-patch to generate the > supplied patch. What did I do wrong? Try a clean alsa-lib.git checkout and apply your patch manually there. Then you'll see what I meant. Takashi
diff --git a/src/pcm/pcm_dshare.c b/src/pcm/pcm_dshare.c index c5b3178..9b478a7 100644 --- a/src/pcm/pcm_dshare.c +++ b/src/pcm/pcm_dshare.c @@ -157,23 +157,14 @@ static void snd_pcm_dshare_sync_area(snd_pcm_t *pcm) /* * synchronize hardware pointer (hw_ptr) with ours */ -static int snd_pcm_dshare_sync_ptr(snd_pcm_t *pcm) +static int snd_pcm_dshare_sync_ptr0(snd_pcm_t *pcm, snd_pcm_uframes_t slave_hw_ptr) { snd_pcm_direct_t *dshare = pcm->private_data; - snd_pcm_uframes_t slave_hw_ptr, old_slave_hw_ptr, avail; + snd_pcm_uframes_t old_slave_hw_ptr, avail; snd_pcm_sframes_t diff; - switch (snd_pcm_state(dshare->spcm)) { - case SND_PCM_STATE_DISCONNECTED: - dshare->state = SNDRV_PCM_STATE_DISCONNECTED; - return -ENODEV; - default: - break; - } - if (dshare->slowptr) - snd_pcm_hwsync(dshare->spcm); old_slave_hw_ptr = dshare->slave_hw_ptr; - slave_hw_ptr = dshare->slave_hw_ptr = *dshare->spcm->hw.ptr; + dshare->slave_hw_ptr = slave_hw_ptr; diff = slave_hw_ptr - old_slave_hw_ptr; if (diff == 0) /* fast path */ return 0; @@ -207,6 +198,24 @@ static int snd_pcm_dshare_sync_ptr(snd_pcm_t *pcm) return 0; } +static int snd_pcm_dshare_sync_ptr(snd_pcm_t *pcm) +{ + snd_pcm_direct_t *dshare = pcm->private_data; + + switch (snd_pcm_state(dshare->spcm)) { + case SND_PCM_STATE_DISCONNECTED: + dshare->state = SNDRV_PCM_STATE_DISCONNECTED; + return -ENODEV; + default: + break; + } + + if (dshare->slowptr) + snd_pcm_hwsync(dshare->spcm); + + return snd_pcm_dshare_sync_ptr0(pcm, *dshare->spcm->hw.ptr); +} + /* * plugin implementation */ @@ -215,22 +224,24 @@ static int snd_pcm_dshare_status(snd_pcm_t *pcm, snd_pcm_status_t * status) { snd_pcm_direct_t *dshare = pcm->private_data; + memset(status, 0, sizeof(*status)); + snd_pcm_status(dshare->spcm, status); + switch (dshare->state) { case SNDRV_PCM_STATE_DRAINING: case SNDRV_PCM_STATE_RUNNING: - snd_pcm_dshare_sync_ptr(pcm); + snd_pcm_dshare_sync_ptr0(pcm, status->hw_ptr); + status->delay += snd_pcm_mmap_playback_delay(pcm) + + status->avail - dshare->spcm->buffer_size; break; default: break; } - memset(status, 0, sizeof(*status)); - snd_pcm_status(dshare->spcm, status); - status->state = snd_pcm_state(dshare->spcm); + status->trigger_tstamp = dshare->trigger_tstamp; status->avail = snd_pcm_mmap_playback_avail(pcm); status->avail_max = status->avail > dshare->avail_max ? status->avail : dshare->avail_max; dshare->avail_max = 0; - status->delay = snd_pcm_mmap_playback_delay(pcm); return 0; }