lxqt / pavucontrol-qt

A Pulseaudio mixer in Qt (port of pavucontrol)
https://lxqt.github.io
GNU General Public License v2.0
110 stars 29 forks source link

Latency offset field is limited #258

Closed cjvth closed 2 months ago

cjvth commented 1 year ago
Expected Behavior

If you connect bluetooth headphones and try to change the latency offset, you have no limits. Just like in original pavucontrol

Current Behavior

The number box accepts values from 0 to 99

Possible Solution

Remove the box max and min value or increase them

Steps to Reproduce (for bugs)
  1. Connect bluetooth headphones
  2. Open pavucontrol-qt
  3. Go to Output devices > the headphones > Show additional settings > Latency offset
  4. Try to set value that is higher than 100 or less than 0
Context

I needed a to set a negative offset after some changes in Pipewire.

System Information
RJVB commented 2 months ago

This patch gives the offset spinners the same range as they have in pavucontrol:

diff --git a/src/devicewidget.ui b/src/devicewidget.ui
index 3b5f781..5b8ca1e 100644
--- a/src/devicewidget.ui
+++ b/src/devicewidget.ui
@@ -244,6 +244,12 @@
            <property name="suffix">
             <string> ms</string>
            </property>
+           <property name="minimum">
+            <number>-2000</number>
+           </property>
+           <property name="maximum">
+            <number>5000</number>
+           </property>
           </widget>
          </item>
         </layout>
tsujan commented 2 months ago

I know nothing of the code of PulseAudio, but if that spin-box shows something in milliseconds, these lines seem suspicious to me:

https://github.com/lxqt/pavucontrol-qt/blob/6502a29663b7561faeadb768e751121c146ee4a0/src/devicewidget.cc#L150

https://github.com/lxqt/pavucontrol-qt/blob/6502a29663b7561faeadb768e751121c146ee4a0/src/devicewidget.cc#L177

@palinek?

RJVB commented 2 months ago

On Tuesday July 30 2024 16:27:59 tsujan wrote:

With my patch, pavucontrol-qt latency spinboxes show the values I configured via pavucontrol (gtk).

I know nothing of the code of PulseAudio, but if that spin-box shows something in milliseconds, these lines seem suspicious to me:

The spinboxes show milliseconds indeed. Pulseaudio uses its own type for latency offsets: pa_usec_t. Usec must stand for microseconds.

The interesting thing here is that it's an unsigned type and so the question is how negative values (configured in the spinners) are handled.

tsujan commented 2 months ago

Pulseaudio uses its own type for latency offsets: pa_usec_t.

In that case, those lines are correct. Thanks!

The interesting thing here is that it's an unsigned type and so....

It's used in pa_context_set_port_latency_offset() and pa_card_port_info, which take int64_t.

My problem was that PulseAudio's documentation was written so poorly.

tsujan commented 2 months ago

@RJVB Would you make a PR for the limits of that spin-box? The current limits are practically useless.

RJVB commented 2 months ago

Would you make a PR for the limits of that spin-box? The current limits are practically useless.

I could, but it's not high on my bucket list.

Plus I do not yet have Qt6 on any of my systems so "formally" I can only do PRs against the non-existing Qt5 branch ;)

tsujan commented 2 months ago

Done in https://github.com/lxqt/pavucontrol-qt/pull/280

RJVB commented 2 months ago

On Thursday August 01 2024 00:07:23 tsujan wrote:

Closed #258 as completed via 3cf4a5ef7b4d297d8f7284f73039f6df77daef8f.

Thanks. I meant to post a LGTM on the PR but got called away, sorry.