[PATCH 0/3] pcm_file report EIO when fail to write to debug file
Previously, I introduced a patch that may return EPIPE error when pcm_file plugin fails to write to output file for some reason. The purpose of reporting the error was to evade assert in code to allow API user error handling and recovery, or more clarity in debugging. Failing to write to a file usually means bad file descriptor, running out of free memory, insufficient file permissions or in case file is a pipe there may be other reasons preventing the write operation. All these are not easily recoverable by restaring playback as EPIPE error which is used to signal XRUN might suggest. Therefore, use EIO to signal a different kind of error. write_wav_header function is refactored a bit for more detailed error handling in case of failed write operation. Adam Miartus (3): pcm_file: use EIO instead of EPIPE when failing to write output file pcm_file: improve error checking in write_wav_header function pcm_file: in case of failed write clear file buffer variables src/pcm/pcm_file.c | 54 +++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 41 insertions(+), 13 deletions(-) -- 2.7.4
EPIPE is defined as XRUN which is not entirely correct in this condition failing to write to a file in pcm_file plugin can not be simply recovered by a retry as user of the api might be led to believe when receiving EPIPE use EIO instead to indicate a different kid of error that may not be recoverable by retry Signed-off-by: Adam Miartus <amiartus@de.adit-jv.com> Reviewed-by: Timo Wischer <twischer@de.adit-jv.com> --- src/pcm/pcm_file.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/pcm/pcm_file.c b/src/pcm/pcm_file.c index ab47da8..d83f97a 100644 --- a/src/pcm/pcm_file.c +++ b/src/pcm/pcm_file.c @@ -575,7 +575,7 @@ static snd_pcm_sframes_t snd_pcm_file_writei(snd_pcm_t *pcm, const void *buffer, __snd_pcm_lock(pcm); if (snd_pcm_file_add_frames(pcm, areas, 0, n) < 0) { __snd_pcm_unlock(pcm); - return -EPIPE; + return -EIO; } __snd_pcm_unlock(pcm); } @@ -593,7 +593,7 @@ static snd_pcm_sframes_t snd_pcm_file_writen(snd_pcm_t *pcm, void **bufs, snd_pc __snd_pcm_lock(pcm); if (snd_pcm_file_add_frames(pcm, areas, 0, n) < 0) { __snd_pcm_unlock(pcm); - return -EPIPE; + return -EIO; } __snd_pcm_unlock(pcm); } @@ -616,7 +616,7 @@ static snd_pcm_sframes_t snd_pcm_file_readi(snd_pcm_t *pcm, void *buffer, snd_pc __snd_pcm_lock(pcm); if (snd_pcm_file_add_frames(pcm, areas, 0, frames) < 0) { __snd_pcm_unlock(pcm); - return -EPIPE; + return -EIO; } __snd_pcm_unlock(pcm); @@ -640,7 +640,7 @@ static snd_pcm_sframes_t snd_pcm_file_readn(snd_pcm_t *pcm, void **bufs, snd_pcm __snd_pcm_lock(pcm); if (snd_pcm_file_add_frames(pcm, areas, 0, frames) < 0) { __snd_pcm_unlock(pcm); - return -EPIPE; + return -EIO; } __snd_pcm_unlock(pcm); @@ -666,7 +666,7 @@ static snd_pcm_sframes_t snd_pcm_file_mmap_commit(snd_pcm_t *pcm, result = snd_pcm_mmap_commit(file->gen.slave, ofs, siz); if (result > 0) { if (snd_pcm_file_add_frames(pcm, areas, ofs, result) < 0) - return -EPIPE; + return -EIO; } } return result; -- 2.7.4
previously errno would be returned even for cases where it may have not been populated, for example one of the write functions failing, or writing only partial buffer, now progress through write operations separately and report errno when appropriate Signed-off-by: Adam Miartus <amiartus@de.adit-jv.com> Reviewed-by: Timo Wischer <twischer@de.adit-jv.com> --- src/pcm/pcm_file.c | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/src/pcm/pcm_file.c b/src/pcm/pcm_file.c index d83f97a..f800e51 100644 --- a/src/pcm/pcm_file.c +++ b/src/pcm/pcm_file.c @@ -327,6 +327,8 @@ static void setup_wav_header(snd_pcm_t *pcm, struct wav_fmt *fmt) static int write_wav_header(snd_pcm_t *pcm) { snd_pcm_file_t *file = pcm->private_data; + ssize_t res; + static const char header[] = { 'R', 'I', 'F', 'F', 0x24, 0, 0, 0, @@ -341,15 +343,36 @@ static int write_wav_header(snd_pcm_t *pcm) setup_wav_header(pcm, &file->wav_header); - if (write(file->fd, header, sizeof(header)) != sizeof(header) || - write(file->fd, &file->wav_header, sizeof(file->wav_header)) != - sizeof(file->wav_header) || - write(file->fd, header2, sizeof(header2)) != sizeof(header2)) { - int err = errno; - SYSERR("%s write header failed, file data may be corrupt", file->fname); - return -err; + res = write(file->fd, header, sizeof(header)); + if (res != sizeof(header)) { + goto write_error; + } + + res = write(file->fd, &file->wav_header, sizeof(file->wav_header)); + if (res != sizeof(file->wav_header)) { + goto write_error; } + + res = write(file->fd, header2, sizeof(header2)); + if (res != sizeof(header2)) { + goto write_error; + } + return 0; + +write_error: + // print real errno if available and return EIO, reason for this is to block possible + // EPIPE in case file->fd is a pipe. EPIPE from file->fd conflicts with EPIPE from + // playback stream which should be used to signal XRUN on playback device + if (res < 0) { + SYSERR("%s write header failed, file data may be corrupt", file->fname); + } else { + SNDERR("%s write header incomplete, file data may be corrupt", file->fname); + } + + memset(&file->wav_header, 0, sizeof(struct wav_fmt)); + + return -EIO; } /* fix up the length fields in WAV header */ -- 2.7.4
previously, in case of failed write to output file, error is returned from snd_pcm_writei/read APIs, user could run pcm_drain as fallback and encounter an assert, since drain would try to write remaining file buffer to a file if failed to write to output file in first place, it makes sense to clear current internal pmc_file file buffer variables Signed-off-by: Adam Miartus <amiartus@de.adit-jv.com> Reviewed-by: Timo Wischer <twischer@de.adit-jv.com> --- src/pcm/pcm_file.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/pcm/pcm_file.c b/src/pcm/pcm_file.c index f800e51..bf6e064 100644 --- a/src/pcm/pcm_file.c +++ b/src/pcm/pcm_file.c @@ -414,8 +414,11 @@ static int snd_pcm_file_write_bytes(snd_pcm_t *pcm, size_t bytes) if (file->format == SND_PCM_FILE_FORMAT_WAV && !file->wav_header.fmt) { err = write_wav_header(pcm); - if (err < 0) + if (err < 0) { + file->wbuf_used_bytes = 0; + file->file_ptr_bytes = 0; return err; + } } while (bytes > 0) { @@ -426,6 +429,8 @@ static int snd_pcm_file_write_bytes(snd_pcm_t *pcm, size_t bytes) err = write(file->fd, file->wbuf + file->file_ptr_bytes, n); if (err < 0) { err = -errno; + file->wbuf_used_bytes = 0; + file->file_ptr_bytes = 0; SYSERR("%s write failed, file data may be corrupt", file->fname); return err; } -- 2.7.4
participants (1)
-
Adam Miartus