[alsa-devel] [PATCH 6/9] rate: handle negative values from snd_pcm_mmap_playback_hw_avail

[Takashi Iwai]

At Mon, 15 Sep 2014 16:03:57 +0600,
Alexander E. Patrakov wrote:
> 
> 15.09.2014 14:49, Takashi Iwai пишет:
> > At Sun, 14 Sep 2014 00:30:18 +0600,
> > Alexander E. Patrakov wrote:
> >>
> >> Such negative returns are possible during an underrun if xrun detection
> >> is disabled.
> >>
> >> So, don't store the result in an unsigned variable (where it will
> >> overflow), and postpone the trigger in such case, too.
> >>
> >> Signed-off-by: Alexander E. Patrakov <patrakov at gmail.com>
> >> ---
> >>
> >> The patch is only compile-tested and the second hunk may well be wrong.
> >>
> >> There are also similar issues in pcm_share.c, but, as I don't completely
> >> understand the code there and cannot test that plugin at all due to
> >> unrelated crashes, there will be no patch from me.
> >
> > In general, hw_avail must not be negative before starting the stream.
> > If it is, then it means most likely the driver problem, so we should
> > return error immediately instead.
> 
> Thanks for the review. Would -EBADFD be a correct error code, or do you 
> want something different? or maybe even an assert?

I'd take either EPIPE or EBADFD.
An assert would be an overkill, IMO.


Takashi

> 
> >
> >
> > Takashi
> >
> >>
> >>   src/pcm/pcm_rate.c | 4 ++--
> >>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/src/pcm/pcm_rate.c b/src/pcm/pcm_rate.c
> >> index b436a8e..736d558 100644
> >> --- a/src/pcm/pcm_rate.c
> >> +++ b/src/pcm/pcm_rate.c
> >> @@ -1058,7 +1058,7 @@ static snd_pcm_state_t snd_pcm_rate_state(snd_pcm_t *pcm)
> >>   static int snd_pcm_rate_start(snd_pcm_t *pcm)
> >>   {
> >>   	snd_pcm_rate_t *rate = pcm->private_data;
> >> -	snd_pcm_uframes_t avail;
> >> +	snd_pcm_sframes_t avail;
> >>   		
> >>   	if (pcm->stream == SND_PCM_STREAM_CAPTURE)
> >>   		return snd_pcm_start(rate->gen.slave);
> >> @@ -1069,7 +1069,7 @@ static int snd_pcm_rate_start(snd_pcm_t *pcm)
> >>   	gettimestamp(&rate->trigger_tstamp, pcm->tstamp_type);
> >>
> >>   	avail = snd_pcm_mmap_playback_hw_avail(rate->gen.slave);
> >> -	if (avail == 0) {
> >> +	if (avail <= 0) {
> >>   		/* postpone the trigger since we have no data committed yet */
> >>   		rate->start_pending = 1;
> >>   		return 0;
> >> --
> >> 2.1.0
> >>
> 
> 
> -- 
> Alexander E. Patrakov
>