output/alsa: check ring buffer space before writing to it

Pass only the amount of data to PcmExport::Export() when its full
output fits into the ring buffer.  Using only a part of the
PcmExport::Export() result may cause data corruption because
PcmExport's internal state may contain partial blocks which would need
to be rolled back when only some of its output data was used.

As a side effect, this fixes an assertion failure because
PcmExport::CalcInputSize() considered partial block data and could
cause Play() to return a number larger than the "size" parameter.
This commit is contained in:
Max Kellermann 2019-07-04 12:34:39 +02:00
parent b12fc3c60d
commit 0a32634d8f

View File

@ -104,12 +104,10 @@ class AlsaOutput final
/** the libasound PCM device handle */ /** the libasound PCM device handle */
snd_pcm_t *pcm; snd_pcm_t *pcm;
#ifndef NDEBUG
/** /**
* The size of one audio frame passed to method play(). * The size of one audio frame passed to method play().
*/ */
size_t in_frame_size; size_t in_frame_size;
#endif
/** /**
* The size of one audio frame passed to libasound. * The size of one audio frame passed to libasound.
@ -298,6 +296,17 @@ private:
return true; return true;
} }
/**
* Wait until there is some space available in the ring buffer.
*
* Caller must not lock the mutex.
*
* Throws on error.
*
* @return the number of frames available for writing
*/
size_t LockWaitWriteAvailable();
int Recover(int err) noexcept; int Recover(int err) noexcept;
/** /**
@ -716,9 +725,7 @@ AlsaOutput::Open(AudioFormat &audio_format)
audio_format.channels, audio_format.channels,
params); params);
#ifndef NDEBUG
in_frame_size = audio_format.GetFrameSize(); in_frame_size = audio_format.GetFrameSize();
#endif
out_frame_size = pcm_export->GetOutputFrameSize(); out_frame_size = pcm_export->GetOutputFrameSize();
drain = false; drain = false;
@ -939,14 +946,10 @@ AlsaOutput::Close() noexcept
} }
size_t size_t
AlsaOutput::Play(const void *chunk, size_t size) AlsaOutput::LockWaitWriteAvailable()
{ {
assert(size > 0); const size_t out_block_size = pcm_export->GetOutputBlockSize();
assert(size % in_frame_size == 0); const size_t min_available = 2 * out_block_size;
const auto e = pcm_export->Export({chunk, size});
if (e.empty())
return size;
std::unique_lock<Mutex> lock(mutex); std::unique_lock<Mutex> lock(mutex);
@ -954,10 +957,15 @@ AlsaOutput::Play(const void *chunk, size_t size)
if (error) if (error)
std::rethrow_exception(error); std::rethrow_exception(error);
size_t bytes_written = ring_buffer->push((const uint8_t *)e.data, size_t write_available = ring_buffer->write_available();
e.size); if (write_available >= min_available) {
if (bytes_written > 0) /* reserve room for one extra block, just in
return pcm_export->CalcInputSize(bytes_written); case PcmExport::Export() has some partial
block data in its internal buffer */
write_available -= out_block_size;
return write_available / out_frame_size;
}
/* now that the ring_buffer is full, we can activate /* now that the ring_buffer is full, we can activate
the socket handlers to trigger the first the socket handlers to trigger the first
@ -975,6 +983,29 @@ AlsaOutput::Play(const void *chunk, size_t size)
} }
} }
size_t
AlsaOutput::Play(const void *chunk, size_t size)
{
assert(size > 0);
assert(size % in_frame_size == 0);
const size_t max_frames = LockWaitWriteAvailable();
const size_t max_size = max_frames * in_frame_size;
if (size > max_size)
size = max_size;
const auto e = pcm_export->Export({chunk, size});
if (e.empty())
return size;
size_t bytes_written = ring_buffer->push((const uint8_t *)e.data,
e.size);
assert(bytes_written == e.size);
(void)bytes_written;
return size;
}
std::chrono::steady_clock::duration std::chrono::steady_clock::duration
AlsaOutput::PrepareSockets() noexcept AlsaOutput::PrepareSockets() noexcept
{ {