From 028e2dbd6aee376dd130f6b41286743b4ddcf428 Mon Sep 17 00:00:00 2001 From: Chris Uthe Date: Thu, 15 Oct 2026 18:12:54 -0500 Subject: [PATCH] Report playback from the null and stdout sinks so the player releases chunks A sink that never calls on_frames_played leaves the player's sync task in initial-sync priming: the codec header stays held, every chunk queues behind it in the inbound ring, the player's quota overflows and the library closes the connection, which the server then reopens in a loop (#82). The null and stdout sinks now report every write as played against a virtual clock that runs ahead of real time by the server's lead, so each chunk is released on arrival. The audio_sink.h note that said reporting was optional for instant sinks is corrected. --- CMakeLists.txt | 1 + src/audio_sink.h | 3 +- src/null_sink.cpp | 28 +++++++++++++++ src/null_sink.h | 7 ++++ tests/null_sink_test.cpp | 75 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 tests/null_sink_test.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 21581a6..ffdefa7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -440,6 +440,7 @@ if(SENDSPIN_CLI_BUILD_TESTS) tests/device_spec_test.cpp tests/discovery_test.cpp tests/hooks_test.cpp + tests/null_sink_test.cpp tests/pcm_ring_test.cpp tests/pcm_volume_test.cpp tests/pipewire_sink_test.cpp diff --git a/src/audio_sink.h b/src/audio_sink.h index 96f569e..9d5f777 100644 --- a/src/audio_sink.h +++ b/src/audio_sink.h @@ -106,7 +106,8 @@ class AudioSink { return SinkCapabilities::permissive(); } - /// Reports frames that reached the DAC, for sync feedback; optional for instant sinks. + /// Reports frames that reached the DAC, for sync feedback; a sink that never reports stalls + /// the player before its first chunk, so an instant sink reports against a virtual clock. /// Assign before start() and never after: backends read it from their audio thread. std::function on_frames_played; diff --git a/src/null_sink.cpp b/src/null_sink.cpp index aeb7475..89bc9ac 100644 --- a/src/null_sink.cpp +++ b/src/null_sink.cpp @@ -20,6 +20,7 @@ #include #include +#include #include namespace sendspin_cli { @@ -34,6 +35,14 @@ namespace { constexpr size_t SILENCE_CHUNK = 4096; constexpr std::array SILENCE{}; +constexpr int64_t US_PER_S = 1000000; + +int64_t now_us() { + return std::chrono::duration_cast( + std::chrono::steady_clock::now().time_since_epoch()) + .count(); +} + /// Rounds `bytes` down to whole frames; a frame size of 0 means no stream, so no alignment. size_t align_to_frame(size_t bytes, size_t bytes_per_frame) { if (bytes_per_frame == 0) { @@ -54,6 +63,7 @@ bool NullAudioSink::configure(uint32_t sample_rate, uint8_t channels, uint8_t bi const size_t bytes_per_frame = static_cast(channels) * (static_cast(bits_per_sample) / 8U); this->bytes_per_frame_.store(bytes_per_frame); + this->sample_rate_.store(sample_rate); cli_log(LogLevel::INFO, "%s: stream %u Hz, %u ch, %u-bit (%zu bytes/frame)", this->name().c_str(), sample_rate, channels, bits_per_sample, bytes_per_frame); @@ -70,6 +80,7 @@ size_t NullAudioSink::write(const uint8_t* data, size_t length, uint32_t /*timeo const bool to_stdout = this->output_ == NullSinkOutput::Stdout && !this->stdout_failed_.load(); if (!to_stdout) { this->total_bytes_.fetch_add(length); + this->report_played(length); return length; } @@ -100,6 +111,7 @@ size_t NullAudioSink::write(const uint8_t* data, size_t length, uint32_t /*timeo } this->total_bytes_.fetch_add(consumed); + this->report_played(consumed); return consumed; } @@ -134,4 +146,20 @@ size_t NullAudioSink::total_bytes() const { return this->total_bytes_.load(); } +void NullAudioSink::report_played(size_t bytes) { + const size_t bytes_per_frame = this->bytes_per_frame_.load(); + const uint32_t rate = this->sample_rate_.load(); + if (bytes_per_frame == 0 || rate == 0 || !this->on_frames_played) { + return; + } + const auto frames = static_cast(bytes / bytes_per_frame); + if (frames == 0) { + return; + } + // Never behind real time, so a stream that paused resumes from now rather than the past. + this->playhead_us_ = std::max(this->playhead_us_, now_us()) + + (static_cast(frames) * US_PER_S) / static_cast(rate); + this->on_frames_played(frames, this->playhead_us_); +} + } // namespace sendspin_cli diff --git a/src/null_sink.h b/src/null_sink.h index 57ee308..35106b2 100644 --- a/src/null_sink.h +++ b/src/null_sink.h @@ -32,6 +32,7 @@ enum class NullSinkOutput { }; /// An AudioSink that needs no device; consumes instantly, and honours mute but not volume. +/// Reports playback against a virtual clock that runs ahead of real time by the server's lead. class NullAudioSink final : public AudioSink { public: explicit NullAudioSink(NullSinkOutput output); @@ -48,10 +49,16 @@ class NullAudioSink final : public AudioSink { size_t total_bytes() const; private: + /// Reports `bytes` as played, finishing on the virtual clock. write()'s thread only. + void report_played(size_t bytes); + NullSinkOutput output_; std::atomic total_bytes_{0}; /// Frame size, so a short stdout write rounds down to a frame boundary. std::atomic bytes_per_frame_{0}; + std::atomic sample_rate_{0}; + /// When the last reported frame finishes, in steady-clock microseconds. + int64_t playhead_us_{0}; /// Latches once stdout goes bad; the sink then discards. std::atomic stdout_failed_{false}; std::atomic volume_{DEFAULT_SINK_VOLUME}; diff --git a/tests/null_sink_test.cpp b/tests/null_sink_test.cpp new file mode 100644 index 0000000..0fc05c3 --- /dev/null +++ b/tests/null_sink_test.cpp @@ -0,0 +1,75 @@ +// Copyright 2026 sendspin-cpp-cli Contributors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +/// NullAudioSink's playback reports, which the player releases each chunk against. + +#include "null_sink.h" + +#include + +#include +#include +#include + +namespace sendspin_cli { +namespace { + +struct Report { + uint32_t frames; + int64_t finish_us; +}; + +/// A discarding sink with every report recorded. +struct Harness { + Harness() { + this->sink.on_frames_played = [this](uint32_t frames, int64_t finish_us) { + this->reports.push_back({frames, finish_us}); + }; + } + + NullAudioSink sink{NullSinkOutput::Discard}; + std::vector reports; +}; + +/// 480 frames of 16-bit stereo: 10 ms at 48 kHz. +const std::vector TEN_MS(480 * 4, 0); + +TEST(NullSink, EveryWriteIsReportedPlayedOnAClockThatKeepsAdvancing) { + Harness harness; + ASSERT_TRUE(harness.sink.configure(48000, 2, 16)); + + EXPECT_EQ(harness.sink.write(TEN_MS.data(), TEN_MS.size(), 20), TEN_MS.size()); + EXPECT_EQ(harness.sink.write(TEN_MS.data(), TEN_MS.size(), 20), TEN_MS.size()); + + ASSERT_EQ(harness.reports.size(), 2U); + EXPECT_EQ(harness.reports[0].frames, 480U); + EXPECT_EQ(harness.reports[1].frames, 480U); + // The second write finishes a full write after the first, however fast the two ran. + EXPECT_GE(harness.reports[1].finish_us - harness.reports[0].finish_us, 10000); +} + +TEST(NullSink, NothingIsReportedWithoutAStreamOrFrames) { + Harness harness; + + EXPECT_EQ(harness.sink.write(TEN_MS.data(), TEN_MS.size(), 20), TEN_MS.size()); + ASSERT_TRUE(harness.sink.configure(48000, 2, 16)); + EXPECT_EQ(harness.sink.write(TEN_MS.data(), 0, 20), 0U); + harness.sink.stop(); + EXPECT_EQ(harness.sink.write(TEN_MS.data(), TEN_MS.size(), 20), TEN_MS.size()); + + EXPECT_TRUE(harness.reports.empty()); +} + +} // namespace +} // namespace sendspin_cli