On Thu, 05 Jan 2017 14:29:15 +0100, sutar.mounesh@gmail.com wrote:
From: Andreas Pape apape@de.adit-jv.com
If using very short periods, DSHARE/DSNOOP/DMIX may report underruns while in status 'prepared'. This prohibits correct recovery. Now slave xrun conditions for DSHARE/DSNOOP/DMIX are being handled properly.
Signed-off-by: Andreas Pape apape@de.adit-jv.com Signed-off-by: Joshua Frkuska joshua_frkuska@mentor.com Signed-off-by: Mounesh Sutar mounesh_sutar@mentor.com
The codes look mostly good, but minor coding style issues:
+int snd_pcm_direct_slave_recover(snd_pcm_direct_t *direct) +{
- int ret = 0;
The initialization can be dropped.
- if (ret=snd_pcm_direct_semaphore_down(direct, DIRECT_IPC_SEM_CLIENT)) {
Please avoid this style. I guess gcc -Wall would complain, too. Simply write like
ret = xxx(); if (ret < 0) { ....
Ditto for all other parts.
SNDERR("SEMDOWN FAILED with err %d", ret);return ret;- }
- if (snd_pcm_state(direct->spcm) != SND_PCM_STATE_XRUN) {
/*ignore... someone else already did recovery*/
Please put a space between "/*" and the text (also to the close, too).
if (ret = snd_pcm_direct_semaphore_up(direct,DIRECT_IPC_SEM_CLIENT)) {SNDERR("SEMUP FAILED with err %d", ret);}return ret;- }
- ret = snd_pcm_prepare(direct->spcm);
- if (ret < 0) {
SNDERR("recover: unable to prepare slave");if (ret = snd_pcm_direct_semaphore_up(direct,DIRECT_IPC_SEM_CLIENT)) {SNDERR("SEMUP FAILED with err %d", ret);}return ret;
This may end up with the return zero if snd_pcm_direct_semaphore_up() succeeds. Use another variable.
- ret = snd_pcm_start(direct->spcm);
- if (ret < 0) {
SNDERR("recover: unable to start slave");if (ret = snd_pcm_direct_semaphore_up(direct,DIRECT_IPC_SEM_CLIENT)) {SNDERR("SEMUP FAILED with err %d", ret);}return ret;
Ditto.
+/*
- enter xrun state, if slave xrun occured
- @return: 0 - no xrun >0: xrun happened
- */
+int snd_pcm_direct_client_chk_xrun(snd_pcm_direct_t *direct, snd_pcm_t *pcm) +{
- if (direct->shmptr->recoveries != direct->recoveries) {
/* no matter how many xruns we missed -so don't increment but just update to actual counter*/
Please align the comment, /* * blah blah * blah blah */ or /* blah blah * blah blah */ or /* blah blah */
@@ -572,6 +653,10 @@ int snd_pcm_direct_poll_revents(snd_pcm_t *pcm, struct pollfd *pfds, unsigned in } switch (snd_pcm_state(dmix->spcm)) { case SND_PCM_STATE_XRUN:
/*recover slave and update client state to xrunbefore returning POLLERR*/snd_pcm_direct_slave_recover(dmix);snd_pcm_direct_client_chk_xrun(dmix, pcm);
Put a comment here like
/* fallthrough */
to indicate the fall-thru line.
@@ -841,8 +851,10 @@ static snd_pcm_sframes_t snd_pcm_dmix_mmap_commit(snd_pcm_t *pcm, if ((err = snd_pcm_dmix_start_timer(pcm, dmix)) < 0) return err; } else if (dmix->state == SND_PCM_STATE_RUNNING ||
dmix->state == SND_PCM_STATE_DRAINING)snd_pcm_dmix_sync_ptr(pcm);
dmix->state == SND_PCM_STATE_DRAINING) {if (( err = snd_pcm_dmix_sync_ptr(pcm)) < 0)
Avoid unnecessary space after the parenthesis.
return err;- } if (dmix->state == SND_PCM_STATE_RUNNING || dmix->state == SND_PCM_STATE_DRAINING) { /* ok, we commit the changes after the validation of area */
@@ -858,10 +870,13 @@ static snd_pcm_sframes_t snd_pcm_dmix_mmap_commit(snd_pcm_t *pcm, static snd_pcm_sframes_t snd_pcm_dmix_avail_update(snd_pcm_t *pcm) { snd_pcm_direct_t *dmix = pcm->private_data;
int err;
if (dmix->state == SND_PCM_STATE_RUNNING ||
dmix->state == SND_PCM_STATE_DRAINING)snd_pcm_dmix_sync_ptr(pcm);
dmix->state == SND_PCM_STATE_DRAINING) {if (( err = snd_pcm_dmix_sync_ptr(pcm)) < 0)
Ditto.
diff --git a/src/pcm/pcm_dshare.c b/src/pcm/pcm_dshare.c index a1fed5d..ef1e6c1 100644 --- a/src/pcm/pcm_dshare.c +++ b/src/pcm/pcm_dshare.c @@ -162,7 +162,6 @@ static int snd_pcm_dshare_sync_ptr0(snd_pcm_t *pcm, snd_pcm_uframes_t slave_hw_p snd_pcm_direct_t *dshare = pcm->private_data; snd_pcm_uframes_t old_slave_hw_ptr, avail; snd_pcm_sframes_t diff;
- old_slave_hw_ptr = dshare->slave_hw_ptr; dshare->slave_hw_ptr = slave_hw_ptr; diff = slave_hw_ptr - old_slave_hw_ptr;
Don't remove, a blank line after the function declaration is preferred.
thanks,
Takashi