[alsa-devel] [PATCH 0/2] v2 pcm_file propagate write error instead of an assert
patch possible case whenre file write causes assert in pcm_file plugin, by reporting error to user and printing a message
user has a chance to recover error state, or capture correct logs before assert happens, and may have an idea how to recover
Adam Miartus (2): pcm_file: do not disrupt playback on output file write fail pcm_file: report write output file error to api user
src/pcm/pcm_file.c | 61 ++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 43 insertions(+), 18 deletions(-)
previously playback could be interrupted by snd_pcm_file_add_frames: assert(file->wbuf_used_bytes < file->wbuf_size_bytes)
in case snd_pcm_file_write_bytes fails to write full amount of bytes to file, variable wbuf_used_bytes would not be fully decremented by requested amount of bytes function was called with
for the assert to trigger, multiple write fails need to happen, so that wbuf_used_bytes overflows wbuf_size_bytes,
this patch will allow application to report error code to api user who might have an idea how to recover, before assert is triggered, also reporting error along with the print out message might give user a better idea of what is going on, where previously reason for mentioned assert was not immediately clear
Signed-off-by: Adam Miartus amiartus@de.adit-jv.com Reviewed-by: Timo Wischer twischer@de.adit-jv.com --- src/pcm/pcm_file.c | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/src/pcm/pcm_file.c b/src/pcm/pcm_file.c index 1ef80b5..0a8d98d 100644 --- a/src/pcm/pcm_file.c +++ b/src/pcm/pcm_file.c @@ -346,7 +346,7 @@ static int write_wav_header(snd_pcm_t *pcm) sizeof(file->wav_header) || write(file->fd, header2, sizeof(header2)) != sizeof(header2)) { int err = errno; - SYSERR("Write error.\n"); + SYSERR("%s write header failed, file data may be corrupt", file->fname); return -err; } return 0; @@ -381,27 +381,29 @@ static void fixup_wav_header(snd_pcm_t *pcm)
-static void snd_pcm_file_write_bytes(snd_pcm_t *pcm, size_t bytes) +/* return error code in case write failed */ +static int snd_pcm_file_write_bytes(snd_pcm_t *pcm, size_t bytes) { snd_pcm_file_t *file = pcm->private_data; + snd_pcm_sframes_t err = 0; assert(bytes <= file->wbuf_used_bytes);
if (file->format == SND_PCM_FILE_FORMAT_WAV && !file->wav_header.fmt) { - if (write_wav_header(pcm) < 0) - return; + err = write_wav_header(pcm); + if (err < 0) + return err; }
while (bytes > 0) { - snd_pcm_sframes_t err; size_t n = bytes; size_t cont = file->wbuf_size_bytes - file->file_ptr_bytes; if (n > cont) n = cont; err = write(file->fd, file->wbuf + file->file_ptr_bytes, n); if (err < 0) { - SYSERR("write failed"); - break; + SYSERR("%s write failed, file data may be corrupt", file->fname); + return err; } bytes -= err; file->wbuf_used_bytes -= err; @@ -412,15 +414,17 @@ static void snd_pcm_file_write_bytes(snd_pcm_t *pcm, size_t bytes) if ((snd_pcm_uframes_t)err != n) break; } + return 0; }
-static void snd_pcm_file_add_frames(snd_pcm_t *pcm, - const snd_pcm_channel_area_t *areas, - snd_pcm_uframes_t offset, - snd_pcm_uframes_t frames) +static int snd_pcm_file_add_frames(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; while (frames > 0) { + int err = 0; snd_pcm_uframes_t n = frames; snd_pcm_uframes_t cont = file->wbuf_size - file->appl_ptr; snd_pcm_uframes_t avail = file->wbuf_size - snd_pcm_bytes_to_frames(pcm, file->wbuf_used_bytes); @@ -437,10 +441,14 @@ static void snd_pcm_file_add_frames(snd_pcm_t *pcm, if (file->appl_ptr == file->wbuf_size) file->appl_ptr = 0; file->wbuf_used_bytes += snd_pcm_frames_to_bytes(pcm, n); - if (file->wbuf_used_bytes > file->buffer_bytes) - snd_pcm_file_write_bytes(pcm, file->wbuf_used_bytes - file->buffer_bytes); + if (file->wbuf_used_bytes > file->buffer_bytes) { + err = snd_pcm_file_write_bytes(pcm, file->wbuf_used_bytes - file->buffer_bytes); + if (err < 0) + return err; + } assert(file->wbuf_used_bytes < file->wbuf_size_bytes); } + return 0; }
static int snd_pcm_file_close(snd_pcm_t *pcm)
when writing to output file fails, api user is notified and can handle recovery
Signed-off-by: Adam Miartus amiartus@de.adit-jv.com Reviewed-by: Timo Wischer twischer@de.adit-jv.com --- src/pcm/pcm_file.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-)
diff --git a/src/pcm/pcm_file.c b/src/pcm/pcm_file.c index 0a8d98d..ca8e0c8 100644 --- a/src/pcm/pcm_file.c +++ b/src/pcm/pcm_file.c @@ -572,7 +572,10 @@ static snd_pcm_sframes_t snd_pcm_file_writei(snd_pcm_t *pcm, const void *buffer, if (n > 0) { snd_pcm_areas_from_buf(pcm, areas, (void*) buffer); __snd_pcm_lock(pcm); - snd_pcm_file_add_frames(pcm, areas, 0, n); + if (snd_pcm_file_add_frames(pcm, areas, 0, n) < 0) { + __snd_pcm_unlock(pcm); + return -EPIPE; + } __snd_pcm_unlock(pcm); } return n; @@ -587,7 +590,10 @@ static snd_pcm_sframes_t snd_pcm_file_writen(snd_pcm_t *pcm, void **bufs, snd_pc if (n > 0) { snd_pcm_areas_from_bufs(pcm, areas, bufs); __snd_pcm_lock(pcm); - snd_pcm_file_add_frames(pcm, areas, 0, n); + if (snd_pcm_file_add_frames(pcm, areas, 0, n) < 0) { + __snd_pcm_unlock(pcm); + return -EPIPE; + } __snd_pcm_unlock(pcm); } return n; @@ -608,6 +614,11 @@ static snd_pcm_sframes_t snd_pcm_file_readi(snd_pcm_t *pcm, void *buffer, snd_pc snd_pcm_file_areas_read_infile(pcm, areas, 0, frames); __snd_pcm_lock(pcm); snd_pcm_file_add_frames(pcm, areas, 0, frames); + if (snd_pcm_file_add_frames(pcm, areas, 0, frames) < 0) { + __snd_pcm_unlock(pcm); + return -EPIPE; + } + __snd_pcm_unlock(pcm);
return frames; @@ -627,7 +638,11 @@ static snd_pcm_sframes_t snd_pcm_file_readn(snd_pcm_t *pcm, void **bufs, snd_pcm snd_pcm_areas_from_bufs(pcm, areas, bufs); snd_pcm_file_areas_read_infile(pcm, areas, 0, frames); __snd_pcm_lock(pcm); - snd_pcm_file_add_frames(pcm, areas, 0, frames); + if (snd_pcm_file_add_frames(pcm, areas, 0, frames) < 0) { + __snd_pcm_unlock(pcm); + return -EPIPE; + } + __snd_pcm_unlock(pcm);
return frames; @@ -649,8 +664,10 @@ static snd_pcm_sframes_t snd_pcm_file_mmap_commit(snd_pcm_t *pcm, if (result >= 0) { assert(ofs == offset && siz == size); result = snd_pcm_mmap_commit(file->gen.slave, ofs, siz); - if (result > 0) - snd_pcm_file_add_frames(pcm, areas, ofs, result); + if (result > 0) { + if (snd_pcm_file_add_frames(pcm, areas, ofs, result) < 0) + return -EPIPE; + } } return result; }
On Wed, 12 Jun 2019 08:48:26 +0200, Adam Miartus wrote:
patch possible case whenre file write causes assert in pcm_file plugin, by reporting error to user and printing a message
user has a chance to recover error state, or capture correct logs before assert happens, and may have an idea how to recover
Adam Miartus (2): pcm_file: do not disrupt playback on output file write fail pcm_file: report write output file error to api user
Applied both patches. Thanks.
Takashi
On Wed, 12 Jun 2019 08:48:26 +0200, Adam Miartus wrote:
patch possible case whenre file write causes assert in pcm_file plugin, by reporting error to user and printing a message
user has a chance to recover error state, or capture correct logs before assert happens, and may have an idea how to recover
Adam Miartus (2): pcm_file: do not disrupt playback on output file write fail pcm_file: report write output file error to api user
The patches have been already merged. Any reason for resend?
thanks,
Takashi
-----Original Message----- From: Takashi Iwai tiwai@suse.de Sent: Dienstag, 18. Juni 2019 08:31 To: Miartus, Adam (Arion Recruitment; ADITG/ESM) <amiartus@de.adit- jv.com> Cc: alsa-devel@alsa-project.org Subject: Re: [ALSA patch] [PATCH 0/2] v2 pcm_file propagate write error instead of an assert
On Wed, 12 Jun 2019 08:48:26 +0200, Adam Miartus wrote:
patch possible case whenre file write causes assert in pcm_file plugin, by reporting error to user and printing a message
user has a chance to recover error state, or capture correct logs before assert happens, and may have an idea how to recover
Adam Miartus (2): pcm_file: do not disrupt playback on output file write fail pcm_file: report write output file error to api user
The patches have been already merged. Any reason for resend?
thanks,
Takashi
This is request v2, Im not aware I sent it two times. If so, sorry for spam.
participants (3)
-
Adam Miartus -
Miartus, Adam (Arion Recruitment; ADITG/ESM)
-
Takashi Iwai