[-,JACK,PCM,plugin] jack: Do not Xrun the ALSA buffer
diff mbox

Message ID B0FB33DC1499054591F62C0EF1E013D7684FB83A@HI2EXCH01.adit-jv.com
State New
Headers show

Commit Message

Timo Wischer Feb. 26, 2018, 8:14 a.m. UTC
Hello all,

Due to merged [1] (see [2]) [3] can be merged without conflicts, now.

@Takashi: Thanks for merging [2].

Please give a feedback if there is anything which has to be changed.

The biggest change in snd_pcm_jack_process_cb() occurs 
due to the movement of the snd_pcm_area_silence() call to the end of this function.
This is required to fill the buffer with silence if there is not enough audio data available for JACK.

[1] http://mailman.alsa-project.org/pipermail/alsa-devel/2018-January/130986.html
[2] http://git.alsa-project.org/?p=alsa-plugins.git;a=commit;h=3e6ace6fe045c580dc5490d87eff6b616f7769ef
[3] http://mailman.alsa-project.org/pipermail/alsa-devel/2018-January/130987.html

Best regards

Timo Wischer

Advanced Driver Information Technology GmbH
Engineering Software Base (ADITG/ESB)
Robert-Bosch-Str. 200
31139 Hildesheim
Germany

Tel. +49 5121 49 6938
Fax +49 5121 49 6999
twischer@de.adit-jv.com

ADIT is a joint venture company of Robert Bosch GmbH/Robert Bosch Car Multimedia GmbH and DENSO Corporation
Sitz: Hildesheim, Registergericht: Amtsgericht Hildesheim HRB 3438
Geschäftsführung: Wilhelm Grabow, Ken Yaguchi

Comments

Takashi Iwai Feb. 27, 2018, 8:48 a.m. UTC | #1
On Mon, 26 Feb 2018 09:14:11 +0100,
Wischer, Timo (ADITG/ESB) wrote:
> 
> Hello all,
> 
> Due to merged [1] (see [2]) [3] can be merged without conflicts, now.
> 
> @Takashi: Thanks for merging [2].
> 
> Please give a feedback if there is anything which has to be changed.

Could you simply resubmit the patches?  In your way, the real topic
and the patch is buried deeply.


thanks,

Takashi
Timo Wischer March 1, 2018, 1:14 p.m. UTC | #2
Hello Takashi,

> Could you simply resubmit the patches?  In your way, the real topic
> and the patch is buried deeply.

Hopefully this is what you are requesting!?
Takashi Iwai March 1, 2018, 3:19 p.m. UTC | #3
On Thu, 01 Mar 2018 14:14:04 +0100,
<twischer@de.adit-jv.com> wrote:
> 
> Hello Takashi,
> 
> > Could you simply resubmit the patches?  In your way, the real topic
> > and the patch is buried deeply.
> 
> Hopefully this is what you are requesting!?

Yes, a sort of.  At best, please give a cover letter as [0/N] for a
short explanation of the whole series at the next time.


thanks,

Takashi

Patch
diff mbox

diff --git a/jack/pcm_jack.c b/jack/pcm_jack.c
index c22a5d0..a4f35f4 100644
--- a/jack/pcm_jack.c
+++ b/jack/pcm_jack.c
@@ -54,6 +54,68 @@  typedef struct {

 static int snd_pcm_jack_stop(snd_pcm_ioplug_t *io);

+
+static inline snd_pcm_uframes_t snd_pcm_jack_playback_avail(const snd_pcm_ioplug_t* const io,
+                                                            const snd_pcm_uframes_t hw_ptr,
+                                                            const snd_pcm_uframes_t appl_ptr)
+{
+       const snd_pcm_jack_t* const jack = io->private_data;
+
+       /* cannot use io->hw_ptr without calling snd_pcm_avail_update()
+        * because it is not guranteed that snd_pcm_jack_pointer() was already
+        * called
+        */
+       snd_pcm_sframes_t avail;
+       avail = hw_ptr + io->buffer_size - appl_ptr;
+       if (avail < 0)
+               avail += jack->boundary;
+       else if ((snd_pcm_uframes_t) avail >= jack->boundary)
+               avail -= jack->boundary;
+       return avail;
+}
+
+static inline snd_pcm_uframes_t snd_pcm_jack_capture_avail(const snd_pcm_ioplug_t* const io,
+                                                           const snd_pcm_uframes_t hw_ptr,
+                                                           const snd_pcm_uframes_t appl_ptr)
+{
+       const snd_pcm_jack_t* const jack = io->private_data;
+
+       /* cannot use io->hw_ptr without calling snd_pcm_avail_update()
+        * because it is not guranteed that snd_pcm_jack_pointer() was already
+        * called
+        */
+       snd_pcm_sframes_t avail;
+       avail = hw_ptr - appl_ptr;
+       if (avail < 0)
+               avail += jack->boundary;
+       return avail;
+}
+
+static inline snd_pcm_uframes_t snd_pcm_jack_avail(const snd_pcm_ioplug_t* const io,
+                                                   const snd_pcm_uframes_t hw_ptr,
+                                                   const snd_pcm_uframes_t appl_ptr)
+{
+       return (io->stream == SND_PCM_STREAM_PLAYBACK) ?
+                               snd_pcm_jack_playback_avail(io, hw_ptr, appl_ptr) :
+                               snd_pcm_jack_capture_avail(io, hw_ptr, appl_ptr);
+}
+
+static inline snd_pcm_uframes_t snd_pcm_jack_hw_avail(const snd_pcm_ioplug_t* const io,
+                                                      const snd_pcm_uframes_t hw_ptr,
+                                                      const snd_pcm_uframes_t appl_ptr)
+{
+       /* available data/space which can be transfered by the user application */
+       const snd_pcm_uframes_t user_avail = snd_pcm_jack_avail(io, hw_ptr,
+                                                               appl_ptr);
+
+       if (user_avail > io->buffer_size) {
+               /* there was an Xrun */
+               return 0;
+       }
+       /* available data/space which can be transfered by the DMA */
+       return io->buffer_size - user_avail;
+}
+
 static int pcm_poll_block_check(snd_pcm_ioplug_t *io)
 {
        static char buf[32];
@@ -154,7 +216,6 @@  snd_pcm_jack_process_cb(jack_nframes_t nframes, snd_pcm_ioplug_t *io)
 {
        snd_pcm_jack_t *jack = io->private_data;
        snd_pcm_uframes_t hw_ptr;
-       const snd_pcm_channel_area_t *areas;
        snd_pcm_uframes_t xfer = 0;
        unsigned int channel;

@@ -164,40 +225,57 @@  snd_pcm_jack_process_cb(jack_nframes_t nframes, snd_pcm_ioplug_t *io)
                jack->areas[channel].first = 0;
                jack->areas[channel].step = jack->sample_bits;
        }
-
-       if (io->state != SND_PCM_STATE_RUNNING) {
-               if (io->stream == SND_PCM_STREAM_PLAYBACK) {
-                       for (channel = 0; channel < io->channels; channel++)
-                               snd_pcm_area_silence(&jack->areas[channel], 0, nframes, io->format);
-                       return 0;
-               }
-       }

        hw_ptr = jack->hw_ptr;
-       areas = snd_pcm_ioplug_mmap_areas(io);
-
-       while (xfer < nframes) {
-               snd_pcm_uframes_t frames = nframes - xfer;
-               snd_pcm_uframes_t offset = hw_ptr % io->buffer_size;
-               snd_pcm_uframes_t cont = io->buffer_size - offset;
-
-               if (cont < frames)
-                       frames = cont;
+       if (io->state == SND_PCM_STATE_RUNNING) {
+               const snd_pcm_channel_area_t *areas = snd_pcm_ioplug_mmap_areas(io);
+
+               while (xfer < nframes) {
+                       snd_pcm_uframes_t frames = nframes - xfer;
+                       snd_pcm_uframes_t offset = hw_ptr % io->buffer_size;
+                       snd_pcm_uframes_t cont = io->buffer_size - offset;
+                       snd_pcm_uframes_t hw_avail =
+                                       snd_pcm_jack_hw_avail(io, hw_ptr,
+                                                             io->appl_ptr);
+
+                       /* stop copying if there is no more data available */
+                       if (hw_avail <= 0)
+                               break;
+
+                       /* split the snd_pcm_area_copy() function into two parts
+                        * if the data to copy passes the buffer wrap around
+                        */
+                       if (cont < frames)
+                               frames = cont;
+
+                       if (hw_avail < frames)
+                               frames = hw_avail;
+
+                       for (channel = 0; channel < io->channels; channel++) {
+                               if (io->stream == SND_PCM_STREAM_PLAYBACK)
+                                       snd_pcm_area_copy(&jack->areas[channel], xfer, &areas[channel], offset, frames, io->format);
+                               else
+                                       snd_pcm_area_copy(&areas[channel], offset, &jack->areas[channel], xfer, frames, io->format);
+                       }

-               for (channel = 0; channel < io->channels; channel++) {
-                       if (io->stream == SND_PCM_STREAM_PLAYBACK)
-                               snd_pcm_area_copy(&jack->areas[channel], xfer, &areas[channel], offset, frames, io->format);
-                       else
-                               snd_pcm_area_copy(&areas[channel], offset, &jack->areas[channel], xfer, frames, io->format);
+                       hw_ptr += frames;
+                       if (hw_ptr >= jack->boundary)
+                               hw_ptr -= jack->boundary;
+                       xfer += frames;
                }
-
-               hw_ptr += frames;
-               if (hw_ptr >= jack->boundary)
-                       hw_ptr -= jack->boundary;
-               xfer += frames;
        }
        jack->hw_ptr = hw_ptr;

+       /* check if requested frames were copied */
+       if (xfer < nframes) {
+               /* always fill the not yet written JACK buffer with silence */
+               if (io->stream == SND_PCM_STREAM_PLAYBACK) {
+                       const snd_pcm_uframes_t samples = nframes - xfer;
+                       for (channel = 0; channel < io->channels; channel++)
+                               snd_pcm_area_silence(&jack->areas[channel], xfer, samples, io->format);
+               }
+       }
+
        pcm_poll_unblock_check(io); /* unblock socket for polling if needed */

        return 0;