[PATCH v2] * emms-volume-mpv.el: New file.

[Richard Sent]

---
Hi all,

> Personally I have WM/DE hotkeys to tweak volume in audio output for
> that particular mpv instance (*), but yeah, it sounds like a good thing 
> to have bundled with the player backend (to me at least), as it's very
> much tied to its API and provides kinda basic player functionality
> (that probably should've been there from the start).

Understood. For now I'll keep it separate since it's easier for me to
track mentally but we can move it later.

> Haven't tested that code at all though, maybe I'm missing something
> basic about why it's not really an option and wasn't considered?
> (haven't used threads in emacs myself, so it seems quite possible)

There's an easy answer: I didn't know Emacs had threads 😀. I've
reworked the patch using your ipc code as a base. I also threw in some
validation to ensure ipc is not called from the main thread. (As you
mentioned the ipc-req-send handler is run in the main thread, which can
deadlock if ipc is also invoked by the main thread.)

> "sound clipping starts to occur" point can only be measured by mpv and
> its audio processing pipeline, and don't think it exposes that via any
> properties in some very generic way.
> 
> So best way is probably to just let user decide whether they want to
> change "volume" or "ao-volume", kinda like mpv itself does/supports.

I've added emms-volume-mpv-method. This allows for pure "volume"
control, pure "ao-volume" control, and a "smart" mode. The smart mode
tries to avoid clipping by ensuring both "volume" and "ao-volume" are at
100% before raising "volume" above 100%. When lowering volume, it first
drops "volume" to 100%, then lowers "ao-volume" to 0. This should allow
for mostly-system-integrated volume management while still supporting
mpv's software volume boost.

While ao-volume properties above 100% should be possible, I don't think
mpv supports them. Attempting to raise ao-volume above 100% causes this
error:

(error "Failed to run (set_property ao-volume 105.0) with error unsupported 
format for accessing property")

Because ao-volume-max doesn't exist [1], this cap can't be changed. Ergo
I capped ao-volume at 100%.

One neat factoid is that the percentage in ao-volume doesn't correspond
with what I see in pavucontrol unless it's at 0% or 100%. Sounds like
some translation "feature" when using pipewire-pulse, but I don't think
it's our problem [2].

> On "avoid tweaking system volume" - I think usually "ao-volume" ends up
> translating to "volume of mpv audio stream" (with e.g. pulseaudio or
> pipewire on linux), and not supposed to affect volume of anything else
> on the system.
> I.e. not "system volume" as in "single volume knob on speakers".
> 
> But it's probably not universally like that

That matches my understanding [3]. What I meant to say was we shouldn't
solely use "ao-volume" because it doesn't have the "volume of mpv audio
stream" behavior on all systems, only most. So users should still be
able to choose regular ol' "volume". (Which we agree on.)

While developing this patch I found some odd behavior when volume was
changed rapidly. Threads would indefinitely block on
emms-volume-mpv--ipc-sync-check and the volume would suddenly jump to
unrelated values. I think this was because of multiple threads calling
emms-volume-mpv-synchronous-ipc, which interacted with each other in odd
ways.

For example, this was printed to the console when I held '-' in a
playlist (aka emms-volume-lower):

Native volume is 20%
Native volume is 15%
Native volume is 10%
Native volume is 5%
Native volume is 0% [18 times]
Native volume is 95%
Native volume is 90%

I resolved this by making the ipc mutex, condition variable, and reply
local variables instead of global. Thanks to lexical scope each handler
and thread has their own set of variables and this seems to solve the
problem.

I also added a second mutex, emms-volume-mpv--volume-sync, in
emms-volume-mpv-change. This mutex ensures the thread runs to completion
before another thread starts.

e.g. without the volume-sync mutex, the following code only changes the
volume by 1 because each thread changes the volume from 3 to 4
simultaneously:

(dotimes (a 100)
  (emms-volume-mpv-change 1))
;;Native volume is 100% and system volume is 4% [100 times]

while with volume-sync, it works as expected, increasing volume by 100%.

One limitation of the current patch: ao-volume is only exposed while
mpv's audio output is active. With the 'system or 'smart methods, if the
user tries adjusting volume before mpv starts playing, or if the user
plays media without audio, the volume cannot be adjusted. I don't know
if this can be resolved. The initial value for ao-volume may change
depending on what it was when mpv was last run.

[1]: https://github.com/mpv-player/mpv/issues/8819#issuecomment-835352309
[2]: https://github.com/mpv-player/mpv/issues/11065#issuecomment-1368146703
[3]: https://mpv.io/manual/master/#command-interface-ao-volume

 emms-volume-mpv.el | 159 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 159 insertions(+)
 create mode 100644 emms-volume-mpv.el

diff --git a/emms-volume-mpv.el b/emms-volume-mpv.el
new file mode 100644
index 0000000..5f4711f
--- /dev/null
+++ b/emms-volume-mpv.el
@@ -0,0 +1,159 @@
+;;; emms-volume-mpv.el --- Volume function to adjust mpv volume easily  -*- 
lexical-binding: t; -*-
+
+;; FIXME Copyright assignment
+
+;; Author: Richard Sent <richard@freakingpenguin.com>
+
+;; This file is part of EMMS.
+
+;; EMMS is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation; either version 3, or (at your option)
+;; any later version.
+;;
+;; EMMS is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with EMMS; see the file COPYING. If not, write to the
+;; Free Software Foundation, Inc., 51 Franklin St, Fifth Floor,
+;; Boston, MA 02110-1301, USA.
+
+;;; Commentary:
+;;
+;; This file defines a function to raise or lower the volume of mpv.
+;; It can be used stand-alone by passing a process object, though it
+;; is meant for usage with Emms, particularly with emms-volume.el and
+;; emms-player-mpv.el.
+;;
+;; To use add the following to your Emms configuration
+;;     (setq emms-volume-change-function 'emms-volume-mpv-change)
+
+;;; History:
+
+;; January 2025: First release, partly based on emms-volume-pulse.el.
+
+;;; Code:
+
+(defcustom emms-volume-mpv-method 'native
+  "How Emms should attempt to adjust mpv's volume.
+
+If `native', Emms will adjust mpv's volume property. This
+provides the same experience as adjusting the volume slider in
+mpv.
+
+If `system', Emms will adjust mpv's ao-volume property, which
+adjusts the volume using the system audio service, such as
+Pulseaudio. Depending on what audio service is being used this
+may either change the mpv application's volume or global volume.
+
+If `smart', Emms will adjust both mpv's volume and ao-volume
+properties. When raising volume, the native volume will be raised
+to 100. Emms will then switch to adjusting system volume to 100
+before raising the native volume again. When lowering volume,
+Emms will lower the software volume to 100, then lower system
+volume to 0.
+
+Both `system' and `smart' require mpv to expose the ao-volume
+property. This property is only available while mpv audio output
+is active. If audio output is not active, the volume will not be
+changed.
+
+Additionally, the percentage provided by and set for ao-volume
+and thus this module may not match what is reported by the system
+audio program."
+  :type
+  '(choice (const :tag "MPV Volume" native)
+           (const :tag "System Volume" system)
+           (const :tag "Smart" smart)))
+
+(defvar emms-volume-mpv--volume-sync (make-mutex 
"emms-volume-mpv--volume-sync")
+  "Ensure only one volume-change function runs to completion at a
+time.")
+
+(defun emms-volume-mpv-synchronous-ipc (cmd &optional proc)
+  "Run mpv command and get result synchronously for current thread.
+
+This must not be run by the main thread. The handler for
+emms-player-mpv-ipc-req-send runs in the main thread, potentially
+causing a deadlock."
+  (when (eq main-thread (current-thread))
+    (error "This function cannot be invoked by the main thread"))
+  (let* ((emms-volume-mpv--ipc-sync (make-mutex "emms-volume-mpv--ipc-sync"))
+         (emms-volume-mpv--ipc-sync-check (make-condition-variable 
emms-volume-mpv--ipc-sync
+                                                                   
"emms-volume-mpv--ipc-sync-check"))
+         (emms-volume-mpv--ipc-sync-reply nil))
+    (with-mutex emms-volume-mpv--ipc-sync
+      (emms-player-mpv-ipc-req-send
+       cmd
+       #'(lambda (data err)
+           (with-mutex emms-volume-mpv--ipc-sync
+             (setq emms-volume-mpv--ipc-sync-reply (list data err))
+             (condition-notify emms-volume-mpv--ipc-sync-check)))
+       proc)
+      (while (not emms-volume-mpv--ipc-sync-reply)
+        (condition-wait emms-volume-mpv--ipc-sync-check))
+      (cl-multiple-value-bind (data err) emms-volume-mpv--ipc-sync-reply
+        (if err (error "Failed to run %s, %s" cmd err) data)))))
+
+(defun emms-volume-mpv-limit (vol volume-max &optional volume-min)
+  "Limit VOL to the range [0 - volume-max]."
+  (max (min vol volume-max) (or volume-min 0)))
+
+(defun emms-volume-mpv--smart-increment (native-old system-old amount 
native-max)
+  (cond
+   ((< native-old 100)
+    (list (emms-volume-mpv-limit (+ native-old amount) 100) system-old))
+   ((< system-old 100)
+    (list native-old (emms-volume-mpv-limit (+ system-old amount) 100)))
+   (t (list (emms-volume-mpv-limit (+ native-old amount) native-max) 
system-old))))
+
+(defun emms-volume-mpv--smart-decrement (native-old system-old amount 
native-max)
+  (cond
+   ((> native-old 100)
+    (list (emms-volume-mpv-limit (+ native-old amount) native-max 100) 
system-old))
+   (t (list native-old (emms-volume-mpv-limit (+ system-old amount) 100)))))
+
+(defun emms-volume-mpv--smart-change (native-old system-old amount native-max)
+  (if (>= amount 0)
+      (emms-volume-mpv--smart-increment native-old system-old amount 
native-max)
+    (emms-volume-mpv--smart-decrement native-old system-old amount 
native-max)))
+
+;;;###autoload
+(defun emms-volume-mpv-change (amount &optional proc)
+  "Change volume by AMOUNT using mpv process PROC."
+  (unless (or emms-player-mpv-ipc-proc proc)
+    (error "mpv is not currently running"))
+  ;; mpv does not protect against storing volumes > volume-max. We
+  ;; must retrieve volume-max and verify the target volume.
+  (make-thread
+   (lambda ()
+     (with-mutex emms-volume-mpv--volume-sync
+       (with-demoted-errors "Failed to adjust the volume: %s"
+         (let* ((native-max (emms-volume-mpv-synchronous-ipc '(get_property 
volume-max) proc))
+                (native-old (emms-volume-mpv-synchronous-ipc '(get_property 
volume) proc))
+                (system-old (emms-volume-mpv-synchronous-ipc '(get_property 
ao-volume) proc)))
+           (pcase emms-volume-mpv-method
+             ('native (let ((volume (emms-volume-mpv-limit (+ native-old 
amount) native-max)))
+                        (emms-volume-mpv-synchronous-ipc
+                         `(set_property volume ,volume) proc)
+                        (message "Native volume is %d%%" volume)))
+             ('system (let ((volume (emms-volume-mpv-limit (+ system-old 
amount) 100)))
+                        (emms-volume-mpv-synchronous-ipc
+                         `(set_property ao-volume ,volume) proc)
+                        (message "System volume is %d%%" volume)))
+             ('smart (cl-multiple-value-bind (native system)
+                         (emms-volume-mpv--smart-change native-old system-old
+                                                        amount native-max)
+                       (emms-volume-mpv-synchronous-ipc
+                        `(set_property volume ,native) proc)
+                       (emms-volume-mpv-synchronous-ipc
+                        `(set_property ao-volume ,system) proc)
+                       (message "Native volume is %d%% and system volume is 
%d%%"
+                                native system))))))))))
+
+(provide 'emms-volume-mpv)
+
+;;; emms-volume-mpv.el ends here
-- 
2.47.1