Message ID | 1520007674-14618-4-git-send-email-twischer@de.adit-jv.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hello Takashi, Now I have compiled and tested the patchies successfully. Some minor adaption where required. I have incorporated all your findings. > Please try to format the text a bit nicer. These line breaks appear > like you're writing a poem :)i Done. > You don't need to put inline unless it's really mandatory. Done. > Hm, this whole stuff would fit better in ioplug code itself instead of > open-coding in each plugin. Maybe providing a new helper function? I am providing a new helper function called snd_pcm_ioplug_hw_avail() and I am reusing internally already existing functions. > Ditto. Basically the whole procedure here is some common pattern, so > it wouldn't be too bad to have it in ioplug side. I have provided a new area copy function which takes care about the buffer wrap around. This function is called snd_pcm_areas_copy_wrap(). This reduces the complexity of the JACK IO plug a lot. I have also compared the new implementation with other ALSA IO plugins. I could not found more simuilarities because not all plugins are creating a snd_pcm_channel_area_t for the DMA/HW buffer. Therefore functions like snd_pcm_areas_silence() and snd_pcm_areas_copy() cannot be used. For example the pulse IO plug is also not using snd_pcm_channel_area_t for the pulse buffer. I hope you are fine with these changes, now. If not it would be great if you could give me a feedback in time. Best regards Timo
On Tue, 13 Mar 2018 09:34:41 +0100, <twischer@de.adit-jv.com> wrote: > > > Hello Takashi, > > Now I have compiled and tested the patchies successfully. > Some minor adaption where required. > I have incorporated all your findings. > > > > Please try to format the text a bit nicer. These line breaks appear > > like you're writing a poem :)i > > Done. > > > > You don't need to put inline unless it's really mandatory. > > Done. > > > > Hm, this whole stuff would fit better in ioplug code itself instead of > > open-coding in each plugin. Maybe providing a new helper function? > > I am providing a new helper function called snd_pcm_ioplug_hw_avail() > and I am reusing internally already existing functions. > > > > Ditto. Basically the whole procedure here is some common pattern, so > > it wouldn't be too bad to have it in ioplug side. > > I have provided a new area copy function which takes care about the buffer wrap around. > This function is called snd_pcm_areas_copy_wrap(). > This reduces the complexity of the JACK IO plug a lot. > I have also compared the new implementation with other ALSA IO plugins. > I could not found more simuilarities because not all plugins are creating a snd_pcm_channel_area_t for the DMA/HW buffer. > Therefore functions like snd_pcm_areas_silence() and snd_pcm_areas_copy() cannot be used. > For example the pulse IO plug is also not using snd_pcm_channel_area_t for the pulse buffer. > > I hope you are fine with these changes, now. > If not it would be great if you could give me a feedback in time. Thanks, now applied both patches to alsa-lib and the patch to jack plugin. For alsa-plugins, we'll need the update of configure.ac to check the latest alsa-lib version, but let's do it later at bumping actually the version. Takashi
Hello Takashi,
> Thanks, now applied both patches to alsa-lib and the patch to jack plugin.
I fear you missed the ALSA lib changes [1], right?
[1] http://git.alsa-project.org/?p=alsa-lib.git;a=summary
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
On Thu, 15 Mar 2018 16:09:41 +0100, Wischer, Timo (ADITG/ESB) wrote: > > Hello Takashi, > > > Thanks, now applied both patches to alsa-lib and the patch to jack plugin. > > I fear you missed the ALSA lib changes [1], right? > > [1] http://git.alsa-project.org/?p=alsa-lib.git;a=summary Sorry, forgot to push. Now pushed out. thanks, Takashi
diff --git a/jack/pcm_jack.c b/jack/pcm_jack.c index c547cbb..18425e9 100644 --- a/jack/pcm_jack.c +++ b/jack/pcm_jack.c @@ -54,6 +54,7 @@ typedef struct { static int snd_pcm_jack_stop(snd_pcm_ioplug_t *io); + static int pcm_poll_block_check(snd_pcm_ioplug_t *io) { static char buf[32]; @@ -144,7 +145,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; @@ -154,40 +154,43 @@ 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); + if (io->state == SND_PCM_STATE_RUNNING) { + const snd_pcm_channel_area_t *areas = snd_pcm_ioplug_mmap_areas(io); + + snd_pcm_uframes_t hw_avail = snd_pcm_ioplug_hw_avail(io, hw_ptr, + io->appl_ptr); - 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 (hw_avail > 0) { + snd_pcm_uframes_t frames = nframes - xfer; + snd_pcm_uframes_t offset = hw_ptr % io->buffer_size; - 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); + snd_pcm_areas_copy_wrap(jack->areas, xfer, io->buffer_size, areas, offset, io->buffer_size, io->channels, frames, io->format); else - snd_pcm_area_copy(&areas[channel], offset, &jack->areas[channel], xfer, frames, io->format); + snd_pcm_areas_copy_wrap(areas, offset, io->buffer_size, jack->areas, xfer, io->buffer_size, io->channels, 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 frames = nframes - xfer; + snd_pcm_areas_silence(jack->areas, io->channels, xfer, frames, io->format); + } + } + pcm_poll_unblock_check(io); /* unblock socket for polling if needed */ return 0;