diff mbox

alsa-lib: update linked hw_ptr and appl_ptr

Message ID 1480069505-16668-1-git-send-email-sutar.mounesh@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

sutar.mounesh@gmail.com Nov. 25, 2016, 10:25 a.m. UTC
From: Andreas Pape <apape@de.adit-jv.com>

Plugin file provides no private hw_ptr and appl_ptr but instead links them to the slave pcm.
If the slave pcm itself changes its hw_ptr or app_prt this needs to be done in file plugin, too.
Plugin 'plug' is such a candidate changing the hw_ptr and app_ptr in hw_params call dependent on the automatically inserted plugins.
ALSA unfortunately has no support for automatically updating chained pointers.
A notification on pointer change seems to be prepared inside the snd_pcm_set_ptr() routine via
rbptr->changed(), but it is not (yet) implemented so that we need to care for it manually.

Signed-off-by: Andreas Pape <apape@de.adit-jv.com>
---
 src/pcm/pcm_file.c |    8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Takashi Iwai Nov. 28, 2016, 7:16 p.m. UTC | #1
On Fri, 25 Nov 2016 11:25:05 +0100,
sutar.mounesh@gmail.com wrote:
> 
> From: Andreas Pape <apape@de.adit-jv.com>
> 
> Plugin file provides no private hw_ptr and appl_ptr but instead links them to the slave pcm.
> If the slave pcm itself changes its hw_ptr or app_prt this needs to be done in file plugin, too.
> Plugin 'plug' is such a candidate changing the hw_ptr and app_ptr in hw_params call dependent on the automatically inserted plugins.
> ALSA unfortunately has no support for automatically updating chained pointers.
> A notification on pointer change seems to be prepared inside the snd_pcm_set_ptr() routine via
> rbptr->changed(), but it is not (yet) implemented so that we need to care for it manually.
> 
> Signed-off-by: Andreas Pape <apape@de.adit-jv.com>

Judging from the description, isn't it a generic problem, not specific
to file plugin?


thanks,

Takashi

> ---
>  src/pcm/pcm_file.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/pcm/pcm_file.c b/src/pcm/pcm_file.c
> index fec76eb..1e3d35b 100644
> --- a/src/pcm/pcm_file.c
> +++ b/src/pcm/pcm_file.c
> @@ -634,6 +634,14 @@ static int snd_pcm_file_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t * params)
>  			return err;
>  		}
>  	}
> +
> +	/* pointer may have changed - e.g if plug is used. */
> +	snd_pcm_unlink_hw_ptr(pcm, file->gen.slave);
> +	snd_pcm_unlink_appl_ptr(pcm, file->gen.slave);
> +
> +	snd_pcm_link_hw_ptr(pcm, file->gen.slave);
> +	snd_pcm_link_appl_ptr(pcm, file->gen.slave);
> +
>  	return 0;
>  }
>  
> -- 
> 1.7.9.5
>
Sutar, Mounesh Dec. 8, 2016, 10:56 a.m. UTC | #2
Hi Takashi,
This is the update I got from Andreas:

"This is not a generic problem, but can be observed in plugins which use manual linking method.
The plugins such as PLUG, FILE, METER, MULTI and HOOK use manual linkage.
PLUG is the 'bad guy' which might change the linking in hw_param operation.
METER and MULTI might need to be fixed as well.
HOOKS does not use transfer/processing of frames, so seems harmless (but not 100% sure- linkage should be updated for completeness) 

The current fix is provided for FILE plugin ONLY"

Regards,
Mounesh


-----Original Message-----
From: Takashi Iwai [mailto:tiwai@suse.de] 
Sent: 29 November 2016 00:46
To: sutar.mounesh@gmail.com
Cc: alsa-devel@alsa-project.org; Sutar, Mounesh <Mounesh_Sutar@mentor.com>; Andreas Pape <apape@de.adit-jv.com>
Subject: Re: [PATCH] alsa-lib: update linked hw_ptr and appl_ptr

On Fri, 25 Nov 2016 11:25:05 +0100,
sutar.mounesh@gmail.com wrote:
> 
> From: Andreas Pape <apape@de.adit-jv.com>
> 
> Plugin file provides no private hw_ptr and appl_ptr but instead links them to the slave pcm.
> If the slave pcm itself changes its hw_ptr or app_prt this needs to be done in file plugin, too.
> Plugin 'plug' is such a candidate changing the hw_ptr and app_ptr in hw_params call dependent on the automatically inserted plugins.
> ALSA unfortunately has no support for automatically updating chained pointers.
> A notification on pointer change seems to be prepared inside the 
> snd_pcm_set_ptr() routine via
> rbptr->changed(), but it is not (yet) implemented so that we need to care for it manually.
> 
> Signed-off-by: Andreas Pape <apape@de.adit-jv.com>

Judging from the description, isn't it a generic problem, not specific to file plugin?


thanks,

Takashi

> ---
>  src/pcm/pcm_file.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/pcm/pcm_file.c b/src/pcm/pcm_file.c index 
> fec76eb..1e3d35b 100644
> --- a/src/pcm/pcm_file.c
> +++ b/src/pcm/pcm_file.c
> @@ -634,6 +634,14 @@ static int snd_pcm_file_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t * params)
>  			return err;
>  		}
>  	}
> +
> +	/* pointer may have changed - e.g if plug is used. */
> +	snd_pcm_unlink_hw_ptr(pcm, file->gen.slave);
> +	snd_pcm_unlink_appl_ptr(pcm, file->gen.slave);
> +
> +	snd_pcm_link_hw_ptr(pcm, file->gen.slave);
> +	snd_pcm_link_appl_ptr(pcm, file->gen.slave);
> +
>  	return 0;
>  }
>  
> --
> 1.7.9.5
>
Takashi Iwai Dec. 14, 2016, 2:45 p.m. UTC | #3
On Thu, 08 Dec 2016 11:56:12 +0100,
Sutar, Mounesh wrote:
> 
> Hi Takashi,
> This is the update I got from Andreas:
> 
> "This is not a generic problem, but can be observed in plugins which use manual linking method.
> The plugins such as PLUG, FILE, METER, MULTI and HOOK use manual linkage.
> PLUG is the 'bad guy' which might change the linking in hw_param operation.
> METER and MULTI might need to be fixed as well.
> HOOKS does not use transfer/processing of frames, so seems harmless (but not 100% sure- linkage should be updated for completeness) 
> 
> The current fix is provided for FILE plugin ONLY"

OK, now I applied the patch as is.  The change callback handling would
be good if we really have, but let's cover the bug quickly for now.


thanks,

Takashi

> 
> Regards,
> Mounesh
> 
> 
> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de] 
> Sent: 29 November 2016 00:46
> To: sutar.mounesh@gmail.com
> Cc: alsa-devel@alsa-project.org; Sutar, Mounesh <Mounesh_Sutar@mentor.com>; Andreas Pape <apape@de.adit-jv.com>
> Subject: Re: [PATCH] alsa-lib: update linked hw_ptr and appl_ptr
> 
> On Fri, 25 Nov 2016 11:25:05 +0100,
> sutar.mounesh@gmail.com wrote:
> > 
> > From: Andreas Pape <apape@de.adit-jv.com>
> > 
> > Plugin file provides no private hw_ptr and appl_ptr but instead links them to the slave pcm.
> > If the slave pcm itself changes its hw_ptr or app_prt this needs to be done in file plugin, too.
> > Plugin 'plug' is such a candidate changing the hw_ptr and app_ptr in hw_params call dependent on the automatically inserted plugins.
> > ALSA unfortunately has no support for automatically updating chained pointers.
> > A notification on pointer change seems to be prepared inside the 
> > snd_pcm_set_ptr() routine via
> > rbptr->changed(), but it is not (yet) implemented so that we need to care for it manually.
> > 
> > Signed-off-by: Andreas Pape <apape@de.adit-jv.com>
> 
> Judging from the description, isn't it a generic problem, not specific to file plugin?
> 
> 
> thanks,
> 
> Takashi
> 
> > ---
> >  src/pcm/pcm_file.c |    8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/src/pcm/pcm_file.c b/src/pcm/pcm_file.c index 
> > fec76eb..1e3d35b 100644
> > --- a/src/pcm/pcm_file.c
> > +++ b/src/pcm/pcm_file.c
> > @@ -634,6 +634,14 @@ static int snd_pcm_file_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t * params)
> >  			return err;
> >  		}
> >  	}
> > +
> > +	/* pointer may have changed - e.g if plug is used. */
> > +	snd_pcm_unlink_hw_ptr(pcm, file->gen.slave);
> > +	snd_pcm_unlink_appl_ptr(pcm, file->gen.slave);
> > +
> > +	snd_pcm_link_hw_ptr(pcm, file->gen.slave);
> > +	snd_pcm_link_appl_ptr(pcm, file->gen.slave);
> > +
> >  	return 0;
> >  }
> >  
> > --
> > 1.7.9.5
> > 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
diff mbox

Patch

diff --git a/src/pcm/pcm_file.c b/src/pcm/pcm_file.c
index fec76eb..1e3d35b 100644
--- a/src/pcm/pcm_file.c
+++ b/src/pcm/pcm_file.c
@@ -634,6 +634,14 @@  static int snd_pcm_file_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t * params)
 			return err;
 		}
 	}
+
+	/* pointer may have changed - e.g if plug is used. */
+	snd_pcm_unlink_hw_ptr(pcm, file->gen.slave);
+	snd_pcm_unlink_appl_ptr(pcm, file->gen.slave);
+
+	snd_pcm_link_hw_ptr(pcm, file->gen.slave);
+	snd_pcm_link_appl_ptr(pcm, file->gen.slave);
+
 	return 0;
 }