May 2019 - Patch - mailman.alsa-project.org

[PATCH - alsa-lib 2/4] pcm_file: use snd_pcm_file_areas_read_infile for readi
by Miartus, Adam (Arion Recruitment; ADITG/ESM) 20 May '19

20 May '19

From: Adam Miartus <amiartus(a)de.adit-jv.com> use previously introduced helper function, this commit unifies behavior of readi and readn corner case behavior of readi is changed by this commit, previously, in case 0 bytes were red from file (EOF), frames = 0 was returned, signaling api user as if no data was red from slave, after the patch, amount of frames red from slave with data red from slave stored in buffer is returned when EOF is reached Signed-off-by: Adam Miartus <amiartus(a)de.adit-jv.com> Reviewed-by: Timo Wischer <twischer(a)de.adit-jv.com> diff --git a/src/pcm/pcm_file.c b/src/pcm/pcm_file.c index 7998b64..29ab2fb 100644 --- a/src/pcm/pcm_file.c +++ b/src/pcm/pcm_file.c @@ -577,22 +577,21 @@ static snd_pcm_sframes_t snd_pcm_file_readi(snd_pcm_t *pcm, void *buffer, snd_pc { snd_pcm_file_t *file = pcm->private_data; snd_pcm_channel_area_t areas[pcm->channels]; - snd_pcm_sframes_t n; + snd_pcm_sframes_t frames; + + __snd_pcm_lock(pcm); + + frames = _snd_pcm_readi(file->gen.slave, buffer, size); + if (frames <= 0) + return frames; - n = _snd_pcm_readi(file->gen.slave, buffer, size); - if (n <= 0) - return n; - if (file->ifd >= 0) { - __snd_pcm_lock(pcm); - n = read(file->ifd, buffer, n * pcm->frame_bits / 8); - __snd_pcm_unlock(pcm); - if (n < 0) - return n; - n = n * 8 / pcm->frame_bits; - } snd_pcm_areas_from_buf(pcm, areas, buffer); - snd_pcm_file_add_frames(pcm, areas, 0, n); - return n; + snd_pcm_file_areas_read_infile(pcm, areas, 0, frames); + snd_pcm_file_add_frames(pcm, areas, 0, frames); + + __snd_pcm_unlock(pcm); + + return frames; } /* locking */ -- 2.7.4

1 0

FW: [PATCH - alsa-lib 1/4] pcm_file: add support for infile reading in non interleaved mode
by Miartus, Adam (Arion Recruitment; ADITG/ESM) 20 May '19

20 May '19

From: Adam Miartus <amiartus(a)de.adit-jv.com> add helper function to copy input file data to buffer mapped by areas, in case of an error, do not fill the areas, allowing device read buffer to be provided to api caller previously unused rbuf variable is reused for this purpose Signed-off-by: Adam Miartus <amiartus(a)de.adit-jv.com> Reviewed-by: Timo Wischer <twischer(a)de.adit-jv.com> diff --git a/src/pcm/pcm_file.c b/src/pcm/pcm_file.c index 3a19cef..7998b64 100644 --- a/src/pcm/pcm_file.c +++ b/src/pcm/pcm_file.c @@ -77,6 +77,7 @@ typedef struct { snd_pcm_uframes_t appl_ptr; snd_pcm_uframes_t file_ptr_bytes; snd_pcm_uframes_t wbuf_size; + snd_pcm_uframes_t rbuf_size; size_t wbuf_size_bytes; size_t wbuf_used_bytes; char *wbuf; @@ -266,6 +267,37 @@ static int snd_pcm_file_open_output_file(snd_pcm_file_t *file) return 0; } +/* fill areas with data from input file, return bytes red */ static int +snd_pcm_file_areas_read_infile(snd_pcm_t *pcm, const snd_pcm_channel_area_t *areas, + snd_pcm_uframes_t offset, snd_pcm_uframes_t frames) { + snd_pcm_file_t *file = pcm->private_data; + snd_pcm_channel_area_t areas_if[pcm->channels]; + ssize_t bytes; + + if (file->ifd < 0) + return -EBADF; + + if (file->rbuf == NULL) + return -ENOMEM; + + if (file->rbuf_size < frames) { + SYSERR("requested more frames than pcm buffer"); + return -ENOMEM; + } + + bytes = read(file->ifd, file->rbuf, snd_pcm_frames_to_bytes(pcm, frames)); + if (bytes < 0) { + SYSERR("read from file failed, error: %d", bytes); + return bytes; + } + + snd_pcm_areas_from_buf(pcm, areas_if, file->rbuf); + snd_pcm_areas_copy(areas, offset, areas_if, 0, pcm->channels, +snd_pcm_bytes_to_frames(pcm, bytes), pcm->format); + + return bytes; +} + static void setup_wav_header(snd_pcm_t *pcm, struct wav_fmt *fmt) { fmt->fmt = TO_LE16(0x01); @@ -568,19 +600,19 @@ static snd_pcm_sframes_t snd_pcm_file_readn(snd_pcm_t *pcm, void **bufs, snd_pcm { snd_pcm_file_t *file = pcm->private_data; snd_pcm_channel_area_t areas[pcm->channels]; - snd_pcm_sframes_t n; + snd_pcm_sframes_t frames; - if (file->ifd >= 0) { - SNDERR("DEBUG: Noninterleaved read not yet implemented.\n"); - return 0; /* TODO: Noninterleaved read */ - } + __snd_pcm_lock(pcm); + frames = _snd_pcm_readn(file->gen.slave, bufs, size); + if (frames <= 0) + return frames; - n = _snd_pcm_readn(file->gen.slave, bufs, size); - if (n > 0) { - snd_pcm_areas_from_bufs(pcm, areas, bufs); - snd_pcm_file_add_frames(pcm, areas, 0, n); - } - return n; + snd_pcm_areas_from_bufs(pcm, areas, bufs); + snd_pcm_file_areas_read_infile(pcm, areas, 0, frames); + snd_pcm_file_add_frames(pcm, areas, 0, frames); + + __snd_pcm_unlock(pcm); + return frames; } static snd_pcm_sframes_t snd_pcm_file_mmap_commit(snd_pcm_t *pcm, @@ -609,9 +641,11 @@ static int snd_pcm_file_hw_free(snd_pcm_t *pcm) free(file->wbuf); free(file->wbuf_areas); free(file->final_fname); + free(file->rbuf); file->wbuf = NULL; file->wbuf_areas = NULL; file->final_fname = NULL; + file->rbuf = NULL; return snd_pcm_hw_free(file->gen.slave); } @@ -638,6 +672,15 @@ static int snd_pcm_file_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t * params) snd_pcm_file_hw_free(pcm); return -ENOMEM; } + assert(!file->rbuf); + file->rbuf_size = slave->buffer_size; + file->rbuf_size_bytes = snd_pcm_frames_to_bytes(slave, file->rbuf_size); + file->rbuf_used_bytes = 0; + file->rbuf = malloc(file->rbuf_size_bytes); + if (file->rbuf == NULL) { + snd_pcm_file_hw_free(pcm); + return -ENOMEM; + } file->appl_ptr = file->file_ptr_bytes = 0; for (channel = 0; channel < slave->channels; ++channel) { snd_pcm_channel_area_t *a = &file->wbuf_areas[channel]; -- 2.7.4

1 0

[PATCH v2 0/6]
by vanitha.channaiah＠in.bosch.com 15 May '19

15 May '19

- Updated the patches by incorporating review comments from Takashi Iwai-san [PATCH 1/5] pcm: direct: Add generic hw_ptr_alignment function for dmix, dshare and dsnoop [Takashi Iwai:] > The patch description needs rephrasing. What actually this does is to move the code from pcm_dmix.c to pcm_direct.c > and its header so that the helper code can be re-used by other direct-PCM plugins. - Commit message is rephrased as suggested. - New commit: [PATCH v2 1/6] pcm: direct: Add generic hw_ptr_alignment function for [PATCH 2/5] pcm: dshare: Added "hw_ptr_alignment" option in configuration for alignment of slave pointers [Takashi Iwai:] > Again, this patch description is too ambiguous. > And, if it enables the hw_ptr_alignment option, update the documentation as well. - Commit message is explained in detail for the changes done. - Documentation updated. - New commit: [PATCH v2 2/6] pcm: dshare: Added "hw_ptr_alignment" option in [PATCH 3/5] pcm: dsnoop: Added "hw_ptr_alignment" option in configuration for slave pointer alignment [Takashi Iwai:] > Ditto as patch 2, the description is too ambiguous, and the update of documentation is missing. > It's not good to change the helper function semantics out of sudden, even without any description. - Commit message is explained in detail for the changes done. - Documentation updated. - Divided the patch with commit ("pcm: dsnoop: Add hw_ptr_alignment option in configuration") into additional patch commit ("pcm: direct: Round up of slave_app_ptr pointer if buffer") - Usecase scenario is described for the changes done in helper function. - New commit: [PATCH v2 3/6] pcm: dsnoop: Added "hw_ptr_alignment" option in [PATCH v2 4/6] pcm: direct: Round up of slave_app_ptr pointer if buffer [PATCH 4/5] pcm: restructuring sw params function [Takashi Iwai:] > I see no reason to do that. Please describe. - Commit message is explained in detail why reformating was done. - New commit: [PATCH v2 5/6] pcm: restructuring sw params function [PATCH 5/5] pcm: Update pcm->avail_min with needed_slave_avail_min, after reading unaligned frames [Takashi Iwai:] > This kind of changes in the core code should be avoided as much as possible, especially if it's only relevant with the specific plugins. > Sorry, this isn't convincing enough. If this is a MUST, please clarify better. - Commit message is explained in detail with the generic usecase and specific use case we came across. - New commit: [PATCH v2 6/6] pcm: Update pcm->avail_min with needed_slave_avail_min,

1 6

[PATCH - maemo fixes 1/1] maemo plugin has two crashes I was able to see in a valgrind log from another user:
by stian.skjelstad＠gmail.com 02 May '19

02 May '19

From: Stian Skjelstad <stian.skjelstad(a)gmail.com> * maximum write size was calculated in words (16bit), but checked against byte-size length. This causes memcpy later to overflow the buffer (normally by up to 12KB). * remove a double free (by marking free'd data with NULL) * mmap returns MMAP_FAILED on error, not NULL I suspect that this plugin/driver might have other issues aswell, since I am unable to find any logic for checking DSP buffer status, and no implementation for odelay reporting. Author: Stian Skjelstad <stian.skjelstad(a)gmail.com> Signed-off-by: Stian Skjelstad <stian.skjelstad(a)gmail.com> diff --git a/maemo/alsa-dsp.c b/maemo/alsa-dsp.c index 7e04f6a..3307384 100644 --- a/maemo/alsa-dsp.c +++ b/maemo/alsa-dsp.c @@ -135,18 +135,18 @@ static snd_pcm_sframes_t alsa_dsp_transfer(snd_pcm_ioplug_t * io, snd_pcm_alsa_dsp_t *alsa_dsp = io->private_data; DENTER(); char *buf; - int words; + int bytes, words; ssize_t result; - words = size * alsa_dsp->bytes_per_frame; - words /= 2; - DPRINT("***** Info: words %d size %lu bpf: %d\n", words, size, - alsa_dsp->bytes_per_frame); - if (words > alsa_dsp->dsp_protocol->mmap_buffer_size) { - DERROR("Requested too much data transfer (playing only %d)\n", - alsa_dsp->dsp_protocol->mmap_buffer_size); - words = alsa_dsp->dsp_protocol->mmap_buffer_size; + bytes = size * alsa_dsp->bytes_per_frame; + DPRINT("***** Info: samples %lu * bpf %d => bytes %d\n", + size, alsa_dsp->bytes_per_frame, bytes); + if (bytes > alsa_dsp->dsp_protocol->mmap_buffer_size) { + DERROR("Requested too much data transfer (requested %d, playing only %d)\n", + bytes, alsa_dsp->dsp_protocol->mmap_buffer_size); + bytes = alsa_dsp->dsp_protocol->mmap_buffer_size; } + words = bytes / 2; if (alsa_dsp->dsp_protocol->state != STATE_PLAYING) { DPRINT("I did nothing - No start sent\n"); alsa_dsp_start(io); diff --git a/maemo/dsp-ctl.c b/maemo/dsp-ctl.c index 5bcda36..ac05942 100644 --- a/maemo/dsp-ctl.c +++ b/maemo/dsp-ctl.c @@ -93,6 +93,7 @@ static void dsp_ctl_close(snd_ctl_ext_t * ext) snd_ctl_dsp_t *dsp_ctl = ext->private_data; DENTER(); free(dsp_ctl->controls); + dsp_ctl->controls = NULL; free_control_list(&(dsp_ctl->playback_devices)); free_control_list(&(dsp_ctl->recording_devices)); // free(dsp_ctl); diff --git a/maemo/dsp-protocol.c b/maemo/dsp-protocol.c index af67251..32193d3 100644 --- a/maemo/dsp-protocol.c +++ b/maemo/dsp-protocol.c @@ -194,7 +194,7 @@ int dsp_protocol_open_node(dsp_protocol_t * dsp_protocol, const char *device) mmap((void *)0, dsp_protocol->mmap_buffer_size, PROT_READ | PROT_WRITE, MAP_SHARED, dsp_protocol->fd, 0); - if (dsp_protocol->mmap_buffer == NULL) { + if (dsp_protocol->mmap_buffer == MAP_FAILED) { report_dsp_protocol("Cannot mmap data buffer", dsp_protocol); ret = -ENOMEM; goto unlock; -- 2.17.1

1 0