Mike Kazantsev <[email protected]> writes: > On Fri, 14 Feb 2025 11:03:15 -0500 > Richard Sent <[email protected]> wrote: > >> --- >> 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. > > Yeah, thanks, it looks great. > > Guess I can merge and push it, unless anyone spots any issues in a > couple days?
A scant 4 weeks, 6 days, and 21 hours later: Richard's paperwork is all set. That is indeed "a couple of days" for certain values of "couple". Mike, can you please incorporate these changes into the main git repo? > > I should probably add it to docs and to emms-volume.el as well, > so that it'd be easier to know about and find/use in that case. > > It didn't occur to me at the time, but separate file seem to also make > more sense in context of being a part of emms-volume.el subsystem too. > > >> > "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. > > Sorry, didn't mean it as in "should be changed to work like that" at > all, was just thinking to comment on these volumes, but in retrospect > guess it's easy to read like that. > (as I'm supposed to be a maintainer or something, not just random > person in a thread) > > But with a method-switch it definitely seems more flexible, and won't > need e.g. someone wanting ao-volume directly to patch it later. > > >> 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%. > > Don't think it really matters how it's done, but other way around that > error that comes to mind is how mpv inputs do it - sending something > like "add ao-volume 5" command, which should have mpv increase and cap > the value as-needed. > > Iirc these commands don't return resulting value, so volume will still > need to be fetched for minibuffer message, which seems important to > echo instead of only changing stuff blindly. > > > Guess alternative way to implement this with "add" commands in general > can be to use emms-player-mpv-observe-property on 'volume and 'ao-volume > from emms-player-mpv-event-connect-hook and process those to echo changing > volume anytime via emms-player-mpv-event-functions. > > So that after e.g. "add volume -5" is processed and mpv changes volume, > it'd notify emacs about "volume" property change and it'd be printed in > minibuffer in a similar way. > > One advantage with observe_property would be that changing volume in > mpv externally somehow - e.g. via pavucontrol or pressing hotkeys > and GUI elements in mpv window - will also print info about changes > in emacs, but dunno if it's needed there on those external changes. > > Also this allows to track and change these volumes via emacs-side > variable (as observing property will keep it in sync), but don't think > that's how emms-volume.el does it anyway, and maybe a bit too magical. > > > Don't mean to say that anything needs changes at all, as I don't know > how keeping volumes more in-sync like that would be useful. > It just didn't occur to me earlier, so thought to mention that it can > also be implemented in this other alternative way as well. > > (...guess maybe if there was "display emms volume emoji in mode line" > widget or something like that, to avoid it needing to poll for > volume(s) every N seconds, but then such widget can probably register > its own observer hooks for its own purposes, doesn't really need to be > rely on volume-changing implementation details) > > >> 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. > > It's also been my experience with threads that "put one sync mutex > around as much as possible" tends to be the best way to side-step most > quirks, and usually worth doing just in case, if possible. > > And most obvious emacs docs in particular seem to not guarantee strict > cooperative multitasking behavior (where switching threads only happens > at exact thread-yield calls or mutex/condition waits), saying that it's > "mostly cooperative", so IMO seems best to just make these calls as > sequential/non-concurrent as they can be, to be as future-proof against > hard-to-debug threading quirks as possible. > > >> 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. > > Afaik that's also how mpv hotkeys (and commands, other UI elements) work - > prints some error until ao-stream is actually created somehow after > playback starts, where latter part can be delayed if it's from network > source/fs, > spun-down hdd or whatever. > So seems like a current feature or mpv limitation in general. > (and using e.g. "add ao-volume 1" command won't work around this either) > > > I think if it's useful enough for emms to control this in a stricter > way than how mpv itself does it, then emms-player-mpv-observe-property + > emms-player-mpv-event-functions seem to be one obvious way to do it. > > I.e. observe 'ao-volume, which I think mpv should send immediately when > it's available, and if it doesn't match what's set in emms - adjust it asap. > To avoid emms completely taking control over ao-volume from other mpv inputs, > alternatively maybe it can only be set by emms once, when it first sees that > properly from mpv, and then unobserve or ignore it (leave up to mpv). > > Can see this kind of extra control being useful as an alternative to > PULSE_PROP_media.role=music env var which I've mentioned in an earlier > email (makes pulseaudio store emms-mpv stream volume separately). > > But same as above, it's just how I can think of implementing "this can > be resolved" part, which I've no idea if useful enough or worth doing. -- "Cut your own wood and it will warm you twice"
