SID plugin stutters when plays sid files with PipeWire Output plugin

[lucacremonesi]

Describe the bug When you play any SID file using the PipeWire Output plugin, the audio stutters every second. If you switch to the PulseAudio Output plugin the SID playing is smooth with no stuttering.

Steps to reproduce

• Select the PipeWire Output plugin.
• Play any SID file using the SID player Input plugin.
• The audio stutters every second.

Expected behavior SID audio should be played without stuttering.

Additional information

• Audacious 4.3.1 in Qt mode
• Operating system: Arch Linux
• Desktop environment: KDE Plasma 6.0.5
• pipewire compiled with libpipewire 1.0.7

[mschwendt]

Confirmed with Fedora 40 and 4.4-beta1.

This is interesting considering that the input plugin is "output plugin agnostic", i.e. it uses Audacious' audio output API and doesn't distinguish between e.g. Pipewire and PulseAudio itself.

[jlindgren90]

It is probably a buffer size issue. Some of the emulator plugins produce audio in large chunks, then do CPU-heavy processing without producing any more audio for a while, then produce another large chunk. If the output buffer isn't large enough, then you get underruns.

[lucacremonesi]

I tried all the other available Output plugins (PulseAudio, ALSA, SDL and Jack) and only the PipeWire Output plugin presents this issue. I think the problem is strictly caused by the PipeWire Output plugin.

[radioactiveman]

The buffer size is indeed much smaller than in other output plugins.

Sizes for a 44100 Hz stereo file:

  pipewire:  15056
pulseaudio: 176000
      jack:  44100
       sdl:  88200

pipewire:

m_stride = FMT_SIZEOF(m_aud_format) * m_channels; // 4 * 2 = 8
m_frames = aud::clamp<int>(64, ceilf(2048 * m_rate / 48000.0f), 8192); // 2048 * 44100 / 48000 = 1882
m_buffer_size = m_frames * m_stride; // 8 * 1882 = 15056

pulseaudio:

int buffer_ms = aud_get_int("output_buffer_size"); // 500
size_t buffer_size = pa_usec_to_bytes ((pa_usec_t) 1000 * buffer_ms, & ss);
// (((time * rate) / PA_USEC_PER_SEC)) * (size_table[spec->format] * spec->channels)
// 1000 * 500 * 44100 / 1000000 * 4 * 2 = 176400

jack:

int buffer_time = aud_get_int("output_buffer_size"); // 500
int buffer_size = aud::rescale (buffer_time, 1000, rate) * channels; // 22050 * 2 = 44100

sdl:

int buffer_ms = aud_get_int ("output_buffer_size"); // 500
int buffer_size = 2 * chan * aud::rescale (buffer_ms, 1000, rate); // 2 * 2 * 22050 = 88200

@jlindgren90: I don't know what buffer size would be good for PipeWire. Should we use the same value and calculation as for PulseAudio (see patch below)? Are the values even comparable or did I compare apples with oranges? :smiley:

diff --git a/src/pipewire/pipewire.cc b/src/pipewire/pipewire.cc
index f1d58800e..1fbce6deb 100644
--- a/src/pipewire/pipewire.cc
+++ b/src/pipewire/pipewire.cc
@@ -343,9 +343,11 @@ bool PipeWireOutput::init_core()
         return false;
     }

+    int buffer_size = aud_get_int("output_buffer_size");
+
     m_stride = FMT_SIZEOF(m_aud_format) * m_channels;
     m_frames = aud::clamp<int>(64, ceilf(2048 * m_rate / 48000.0f), 8192);
-    m_buffer_size = m_frames * m_stride;
+    m_buffer_size = buffer_size * m_rate / 1000 * m_stride;
     m_buffer = new unsigned char[m_buffer_size];

     return true;

[jlindgren90]

@radioactiveman the buffer size should indeed respect the "output_buffer_size" setting. That is in milliseconds, so you need to convert to audio frames first based on the sample rate.

It looks like the m_frames calculation should be updated, and m_buffer_size still calculated from that as before. I think this is correct (not tested, I don't have pipewire installed):

diff --git a/src/pipewire/pipewire.cc b/src/pipewire/pipewire.cc
index f1d58800e..f7d017679 100644
--- a/src/pipewire/pipewire.cc
+++ b/src/pipewire/pipewire.cc
@@ -344,7 +344,7 @@ bool PipeWireOutput::init_core()
     }

     m_stride = FMT_SIZEOF(m_aud_format) * m_channels;
-    m_frames = aud::clamp<int>(64, ceilf(2048 * m_rate / 48000.0f), 8192);
+    m_frames = aud_get_int("output_buffer_size") * m_rate / 1000;
     m_buffer_size = m_frames * m_stride;
     m_buffer = new unsigned char[m_buffer_size];

[jlindgren90]

By the way, these are all about correct for the default 500ms buffer size, but there is some unit confusion:

pulseaudio: 176000 bytes ~= 4 bytes/sample * 2 channels * 44100 Hz * 0.5s
      jack:  44100 *samples* (not bytes) = 2 channels * 44100 Hz * 0.5s
       sdl:  88200 bytes = 2 bytes/sample * 2 channels * 44100 Hz * 0.5s

[mschwendt]

@jlindgren90 While that change fixes the stuttering, it breaks the main window's spectrum analyzer in the bottom right corner, which slows down so much it isn't in sync with the audio anymore.

[jlindgren90]

Probably the delay calculation in the pipewire plugin is not correct.

[lucacremonesi]

I performed several tests modifying the "Buffer size" and the "Bit depth" settings from the UI and the stuttering issue was always present using the PipeWire Output plugin. Even using a Buffer size of 10,000 ms (ten seconds) did not solve the problem. On the other hand, when I set the Buffer size to 100 ms using the PulseAudio Output plugin, the SID playing stuttered.

It seems the Buffer size setting is completely ignored by the PipeWire Output plugin.

[jlindgren90]

It seems the Buffer size setting is completely ignored by the PipeWire Output plugin.

Yes, this is what we are discussing how to fix.

[radioactiveman]

@mschwendt: Can you please try the patch below? It fixes the stuttering for me without impact on the visualizations.

@jlindgren90: Are you fine with including this fix as workaround for 4.4? For 4.4.1 we can look for a proper solution.

diff --git a/src/pipewire/pipewire.cc b/src/pipewire/pipewire.cc
index f1d58800e..597fb5c08 100644
--- a/src/pipewire/pipewire.cc
+++ b/src/pipewire/pipewire.cc
@@ -343,9 +343,11 @@ bool PipeWireOutput::init_core()
         return false;
     }

+    // TODO: Respect buffer size setting from libaudcore ("output_buffer_size")
+    // See also: #1401
     m_stride = FMT_SIZEOF(m_aud_format) * m_channels;
     m_frames = aud::clamp<int>(64, ceilf(2048 * m_rate / 48000.0f), 8192);
-    m_buffer_size = m_frames * m_stride;
+    m_buffer_size = 4 * m_frames * m_stride;
     m_buffer = new unsigned char[m_buffer_size];

     return true;

[jlindgren90]

I don't think changing m_buffer_size without changing m_frames is correct - it looks like they are supposed to be in sync. But I did not write this code so maybe I am missing something.

[mschwendt]

Here it also breaks the main spectrum analyzer in the aforementioned way. For all input plugins.

[radioactiveman]

Yes, I spoke too soon, sorry. 4 times the size affects the visualizations. And with 2 times it is probably the same, but less noticeable.

Since there is another workaround (use the PulseAudio output plugin), this issue should be no blocker for Audacious 4.4.

[radioactiveman]

I don't think changing m_buffer_size without changing m_frames is correct - it looks like they are supposed to be in sync. But I did not write this code so maybe I am missing something.

Hi @trialuser02, Could you clarify this question please as Qmmp developer and author of its PipeWire plugin? And what was your idea behind the m_frames calculation?

m_frames = aud::clamp<int>(64, ceilf(2048 * m_rate / 48000.0f), 8192);

P.S. https://sourceforge.net/p/qmmp-dev/code/11337/ needs a version check (see https://github.com/audacious-media-player/audacious-plugins/commit/932e1bcb23957a283d4d6c25643cee91f54567fd) or let CMake check for version 0.3.33.

[trialuser02]

Hi @radioactiveman Nothing special. 2048 is the optimal buffer size that worked for me. Unfortunately, upstream does not provide any information about proper way to calculate this value.

[mschwendt]

What's so ununsual here is that compared with 30 years ago, SID emulation doesn't create high load anymore. Sure, the modern libsidplay versions are "cycle-based" under the hood as to offer realtime emulation capabilities. But within the Audacious input plugin it's still only a "fill this buffer with samples" call as far as I know, and that will be done quickly enough.

When PulseAudio was new, there were plenty of buffering issues, too, compared with ALSA output.

[maris-ab]

Corrected this Bug see commit - 1ab42bf Now Pipewire will use buffer time configuration from Buffer Size - output_buffer_size. Replaced memmove with ringbuf as found optimal how it is working in alsa plugin - Thanks John for it. Corrected drain() by putting buffer processing in the loop to wait for all buffer being played. Corrected get_delay by using timer as did not found in PW library function to get exact buffer position being played. There is known exact buffer position only on the beginning of on_process call, and get_delay uses time difference between known time and the time when get_delay is executed. This corrected visualizations. I am developing WASAPI plugin for audacious now and the same problem is in Exclusive mode using callback. pipewire.cc in my repository contains additional code to work with DSD and FLOAT64 format therefore it can not be used in main audacious-plugins directly but diff 1ab42bf is absolutely correct. Will send file to radioactiveman maybe he can review code and make Pull-request.