Re: [RFC - AAF PCM plugin 5/5] aaf: Implement Capture mode support

["Guedes, Andre" <andre.guedes@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2014 bytes --]

Hi Takashi,

On Tue, 2018-08-21 at 14:17 +0900, Takashi Sakamoto wrote:
> > +static int aaf_mclk_timeout_capture(snd_pcm_aaf_t *aaf)
> > +{
> > +     ssize_t n;
> > +     uint64_t expirations;
> > +
> > +     n = read(aaf->timer_fd, &expirations, sizeof(uint64_t));
> > +     if (n < 0) {
> > +             SNDERR("Failed to read() timer");
> > +             return -errno;
> > +     }
> > +
> > +     if (expirations != 1)
> > +             pr_debug("Missed %llu presentation time(s) ",
> > expirations - 1);
> > +
> > +     while (expirations--) {
> > +             snd_pcm_sframes_t len;
> > +
> > +             aaf_inc_ptr(&aaf->hw_ptr, aaf->frames_per_pkt, aaf-
> > >boundary);
> > +
> > +             len = aaf->hw_virt_ptr - aaf->hw_ptr;
> > +             if (len < 0)
> > +                     len += aaf->boundary;
> > +             if (len > aaf->buffer_size) {
> > +                     /* If the distance between hw virtual pointer
> > and hw
> > +                      * pointer is greater than the buffer size,
> > it means we
> > +                      * had an overrun error so -EPIPE is
> > returned.
> > +                      */
> > +                     return -EPIPE;
> > +             }
> > +
> > +             aaf->mclk_ticks++;
> > +     }
> > +
> > +     return 0;
> > +}
> 
> In your code, -EPIPE can be returned in a call of 
> 'snd_pcm_poll_revents()'. In this case, typical applications follow 
> recovery process below:
> 
> ->snd_pcm_poll_revents()
> ->snd_pcm_prepare()
> ->snd_pcm_start()
> 
> In this case, status of corresponding PCM substream is reset (see 
> 'snd_pcm_ioplug_prepare()'). In my opinion, in this case, backend is 
> expected to stop data streaming, then initialize hardware and
> restart 
> data streaming. In your driver, timer_fd/sk_fd is leaked. It's better
> to
> release them by checking status of PCM substream in .prepare
> callback.

I think you're right. I'll fix it.

Thank you,

Andre

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]