From 98a7c62d7a4f716d90af6d78e18d1a3b10bc54b3 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 2 Jan 2018 22:03:38 +0100 Subject: [PATCH] player/Thread: don't send silence if decoder is slow The output plugin shall decide whether to insert silence or do nothing at all. The ALSA output plugin has already implemented this. Inserting silence is not necessary or helpful for some plugins, and may even hurt them (e.g. "recorder"). --- src/MusicChunk.hxx | 8 ----- src/output/Source.cxx | 3 +- src/player/Thread.cxx | 80 ++----------------------------------------- 3 files changed, 4 insertions(+), 87 deletions(-) diff --git a/src/MusicChunk.hxx b/src/MusicChunk.hxx index b1a2d682b..7f3382896 100644 --- a/src/MusicChunk.hxx +++ b/src/MusicChunk.hxx @@ -79,14 +79,6 @@ struct MusicChunkInfo { */ ReplayGainInfo replay_gain_info; - /** - * A magic value for #replay_gain_serial which omits updating - * the #ReplayGainFilter. This is used by "silence" chunks - * (see PlayerThread::SendSilence()) so they don't affect the - * replay gain. - */ - static constexpr unsigned IGNORE_REPLAY_GAIN = ~0u; - /** * A serial number for checking if replay gain info has * changed since the last chunk. The magic value 0 indicates diff --git a/src/output/Source.cxx b/src/output/Source.cxx index fdaff77f2..cfcc60f10 100644 --- a/src/output/Source.cxx +++ b/src/output/Source.cxx @@ -140,8 +140,7 @@ AudioOutputSource::GetChunkData(const MusicChunk &chunk, replay_gain_filter_set_mode(*current_replay_gain_filter, replay_gain_mode); - if (chunk.replay_gain_serial != *replay_gain_serial_p && - chunk.replay_gain_serial != MusicChunk::IGNORE_REPLAY_GAIN) { + if (chunk.replay_gain_serial != *replay_gain_serial_p) { replay_gain_filter_set_info(*current_replay_gain_filter, chunk.replay_gain_serial != 0 ? &chunk.replay_gain_info diff --git a/src/player/Thread.cxx b/src/player/Thread.cxx index 17a1d1339..e68c2e412 100644 --- a/src/player/Thread.cxx +++ b/src/player/Thread.cxx @@ -26,13 +26,11 @@ #include "MusicPipe.hxx" #include "MusicBuffer.hxx" #include "MusicChunk.hxx" -#include "pcm/Silence.hxx" #include "DetachedSong.hxx" #include "CrossFade.hxx" #include "Control.hxx" #include "tag/Tag.hxx" #include "Idle.hxx" -#include "system/PeriodClock.hxx" #include "util/Domain.hxx" #include "thread/Name.hxx" #include "Log.hxx" @@ -157,8 +155,6 @@ class Player { */ SongTime pending_seek; - PeriodClock throttle_silence_log; - public: Player(PlayerControl &_pc, DecoderControl &_dc, MusicBuffer &_buffer) noexcept @@ -305,17 +301,6 @@ private: */ bool PlayNextChunk() noexcept; - /** - * Sends a chunk of silence to the audio outputs. This is - * called when there is not enough decoded data in the pipe - * yet, to prevent underruns in the hardware buffers. - * - * The player lock is not held. - * - * @return false on error - */ - bool SendSilence() noexcept; - unsigned UnlockCheckOutputs() noexcept { const ScopeUnlock unlock(pc.mutex); return pc.outputs.CheckPipe(); @@ -550,48 +535,6 @@ Player::CheckDecoderStartup() noexcept } } -bool -Player::SendSilence() noexcept -{ - assert(output_open); - assert(play_audio_format.IsDefined()); - - MusicChunk *chunk = buffer.Allocate(); - if (chunk == nullptr) { - /* this is non-fatal, because this means that the - decoder has filled to buffer completely meanwhile; - by ignoring the error, we work around this race - condition */ - LogDebug(player_domain, "Failed to allocate silence buffer"); - return true; - } - -#ifndef NDEBUG - chunk->audio_format = play_audio_format; -#endif - - const size_t frame_size = play_audio_format.GetFrameSize(); - /* this formula ensures that we don't send - partial frames */ - unsigned num_frames = sizeof(chunk->data) / frame_size; - - chunk->bit_rate = 0; - chunk->time = SignedSongTime::Negative(); /* undefined time stamp */ - chunk->length = num_frames * frame_size; - chunk->replay_gain_serial = MusicChunk::IGNORE_REPLAY_GAIN; - PcmSilence({chunk->data, chunk->length}, play_audio_format.format); - - try { - pc.outputs.Play(chunk); - } catch (...) { - LogError(std::current_exception()); - buffer.Return(chunk); - return false; - } - - return true; -} - bool Player::SeekDecoder(SongTime seek_time) noexcept { @@ -973,11 +916,8 @@ Player::SongBorder() noexcept FormatDefault(player_domain, "played \"%s\"", song->GetURI()); - throttle_silence_log.Reset(); - ReplacePipe(dc.pipe); - pc.outputs.SongBorder(); } @@ -1017,15 +957,6 @@ Player::Run() noexcept !dc.IsIdle()) { /* not enough decoded buffer space yet */ - { - const ScopeUnlock unlock(pc.mutex); - if (!paused && output_open && - pc.outputs.CheckPipe() < 4 && - !SendSilence()) - break; - } - - /* XXX race condition: check decoder again */ dc.WaitForDecoder(); continue; } else { @@ -1115,15 +1046,10 @@ Player::Run() noexcept } } else if (output_open) { /* the decoder is too busy and hasn't provided - new PCM data in time: send silence (if the - output pipe is empty) */ - const ScopeUnlock unlock(pc.mutex); + new PCM data in time: wait for the + decoder */ - if (throttle_silence_log.CheckUpdate(std::chrono::seconds(5))) - FormatWarning(player_domain, "Decoder is too slow; playing silence to avoid xrun"); - - if (!SendSilence()) - break; + dc.WaitForDecoder(); } }