summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTanu Kaskinen <tanuk@iki.fi>2012-11-07 14:52:37 (GMT)
committerDavid Henningsson <david.henningsson@canonical.com>2012-11-08 14:38:32 (GMT)
commitdfd44036b54d65314664622ff93dfd18eee03c7b (patch)
treebdb875ea796d5f21588603175cd3950a5cf1ae4f
parent38c650dca96aff9240ddeb5528206ba43bb58187 (diff)
downloadpulseaudio-dfd44036b54d65314664622ff93dfd18eee03c7b.tar.gz
pulseaudio-dfd44036b54d65314664622ff93dfd18eee03c7b.tar.xz
pulse: Fix hole handling in pa_stream_peek().
Previously, if there was a hole in a recording stream, pa_stream_peek() would crash. Holes could be handled silently inside pa_stream_peek() by generating silence (wouldn't work for compressed streams, though) or by skipping any holes. However, I think it's better to let the caller decide how the holes should be handled, so in case of holes, pa_stream_peek() will return NULL data pointer and the length of the hole in the nbytes argument. This change is technically an interface break, because previously the documentation didn't mention the possibility of holes that need special handling. However, since holes caused crashing anyway in the past, it's not a regression if applications keep misbehaving due to not handing holes properly. Some words about when holes can appear in recording streams: I think it would be reasonable behavior if overruns due to the application reading data too slowly would cause holes. Currently that's not the case - overruns will just cause audio to be skipped. But the point is that this might change some day. I'm not sure how holes can occur with the current code, but as the linked bug shows, they can happen. It's most likely due to recording from a monitor source where the thing being monitored has holes in its playback stream. BugLink: http://bugs.launchpad.net/bugs/1058200
-rw-r--r--src/pulse/stream.c20
-rw-r--r--src/pulse/stream.h20
2 files changed, 31 insertions, 9 deletions
diff --git a/src/pulse/stream.c b/src/pulse/stream.c
index 2b6d306..f692d37 100644
--- a/src/pulse/stream.c
+++ b/src/pulse/stream.c
@@ -1593,9 +1593,17 @@ int pa_stream_peek(pa_stream *s, const void **data, size_t *length) {
if (!s->peek_memchunk.memblock) {
if (pa_memblockq_peek(s->record_memblockq, &s->peek_memchunk) < 0) {
+ /* record_memblockq is empty. */
*data = NULL;
*length = 0;
return 0;
+
+ } else if (!s->peek_memchunk.memblock) {
+ /* record_memblockq isn't empty, but it doesn't have any data at
+ * the current read index. */
+ *data = NULL;
+ *length = s->peek_memchunk.length;
+ return 0;
}
s->peek_data = pa_memblock_acquire(s->peek_memchunk.memblock);
@@ -1614,7 +1622,7 @@ int pa_stream_drop(pa_stream *s) {
PA_CHECK_VALIDITY(s->context, !pa_detect_fork(), PA_ERR_FORKED);
PA_CHECK_VALIDITY(s->context, s->state == PA_STREAM_READY, PA_ERR_BADSTATE);
PA_CHECK_VALIDITY(s->context, s->direction == PA_STREAM_RECORD, PA_ERR_BADSTATE);
- PA_CHECK_VALIDITY(s->context, s->peek_memchunk.memblock, PA_ERR_BADSTATE);
+ PA_CHECK_VALIDITY(s->context, s->peek_memchunk.length > 0, PA_ERR_BADSTATE);
pa_memblockq_drop(s->record_memblockq, s->peek_memchunk.length);
@@ -1622,9 +1630,13 @@ int pa_stream_drop(pa_stream *s) {
if (s->timing_info_valid && !s->timing_info.read_index_corrupt)
s->timing_info.read_index += (int64_t) s->peek_memchunk.length;
- pa_assert(s->peek_data);
- pa_memblock_release(s->peek_memchunk.memblock);
- pa_memblock_unref(s->peek_memchunk.memblock);
+ if (s->peek_memchunk.memblock) {
+ pa_assert(s->peek_data);
+ s->peek_data = NULL;
+ pa_memblock_release(s->peek_memchunk.memblock);
+ pa_memblock_unref(s->peek_memchunk.memblock);
+ }
+
pa_memchunk_reset(&s->peek_memchunk);
return 0;
diff --git a/src/pulse/stream.h b/src/pulse/stream.h
index b4464fa..a6785ec 100644
--- a/src/pulse/stream.h
+++ b/src/pulse/stream.h
@@ -534,11 +534,21 @@ int pa_stream_write(
pa_seek_mode_t seek /**< Seek mode, must be PA_SEEK_RELATIVE for upload streams */);
/** Read the next fragment from the buffer (for recording streams).
- * \a data will point to the actual data and \a nbytes will contain the size
- * of the data in bytes (which can be less or more than a complete
- * fragment). Use pa_stream_drop() to actually remove the data from
- * the buffer. If no data is available this will return a NULL
- * pointer. */
+ * If there is data at the current read index, \a data will point to
+ * the actual data and \a nbytes will contain the size of the data in
+ * bytes (which can be less or more than a complete fragment).
+ *
+ * If there is no data at the current read index, it means that either
+ * the buffer is empty or it contains a hole (that is, the write index
+ * is ahead of the read index but there's no data where the read index
+ * points at). If the buffer is empty, \a data will be NULL and
+ * \a nbytes will be 0. If there is a hole, \a data will be NULL and
+ * \a nbytes will contain the length of the hole.
+ *
+ * Use pa_stream_drop() to actually remove the data from the buffer
+ * and move the read index forward. pa_stream_drop() should not be
+ * called if the buffer is empty, but it should be called if there is
+ * a hole. */
int pa_stream_peek(
pa_stream *p /**< The stream to use */,
const void **data /**< Pointer to pointer that will point to data */,