#Max volume media player

1 messages · Page 1 of 1 (latest)

errant oyster
#

@scarlet barn I want to figure out a way to do Max volume on media player. I remember you worked on this. Do you have a write up where it got stuck

scarlet barn
#

Sounds like a 1-minute task for a team of LLM agents?

#

The initial idea was to enforce max volume in the entity model https://github.com/home-assistant/architecture/discussions/1012.
For my own failure to implement it, the problem was related to how it's very tricky do something which works across the board considering how much freedom we give to integrations and generally don't enforce anything at all in entity models.
This is a big issue with all our base entities IMO.
So, it just became a maze of edge cases.
As an example, the local volume might be stale. We could work around that by always polling for the volume before executing any action to change volume, that however met resistance from integration owners because they worried their integration would be less performant because of the extra polls.

#

This was discussed, and to some extent documented, on services which I don't have access to anymore as you know.

#

Looking at it now, I'd suggest to raise the second approach from the dead but add a feature flag which allows integrations to declare that they honor max volume. With that approach integrations where it's relevant (big amplifiers) could opt in, and those where its' irrelevant (most media player integrations to be fair) don't need to worry about it.
I seem to remember you didn't agree with the feature flag though, because it should just work.

errant oyster
#

you're right it's only for standalone amplifiers

#

but those are the ones that need the most automation 🙂 (pick right source etc)

errant oyster
#

ok, so the edge cases is around integrations natively implementing volume up/down while also having a max volume set.

#

I wonder if we can make it work like this:
We support max volume only if:

  • entity does not override volume up/down methods
  • entity supports volume_set
#

I also think that VOLUME_SET should imply VOLUME_STEP supported feature btw, but that's a different concern.

scarlet barn
#

support max volume only if:

  • entity does not override volume up/down methods
  • entity supports volume_set
    Yes, it could work, but it would then be very random which media players it works with. If you look at the code base, it's really popular to implement async_volume_down / async_volume_up
#

I still think the best solution would be to resurrect the PR above, but with a feature flag for integrations to opt in. I think the flag is needed, because frontend needs to know which media players support max volume without calculating it based on state attributes or whatever

#

We could then build on that to make it automagic in some select cases

#

But the automagic parts don't need to be there for MVP IMO

errant oyster
#

I would say that for most integrations, we should be able to kill the volume up and down methods, no ? To make it work with this.

#

I don't think it would be very random, and we could print a message saying: This integration does not support max volume

#

It would only be for the integrations that implement VOLUME_STEP but not VOLUME_SET

#

creating an analaysis of the code base

#

Created OVERVIEW.md with the full analysis. Here's the high-level
summary:

77 integrations provide media player entities with volume features. Key
findings:

Integrations with BOTH VOLUME_SET + VOLUME_STEP that override
volume_up/down: ~40 total

  • ~30 use native device step commands (smartthings, philips_js,
    control4, denonavr, onkyo, etc.) - these should keep their overrides
    since the native command is better than read-modify-write
  • ~10 manually compute current + delta (group, bluesound, mpd,
    monoprice, aquostv, frontier_silicon, clementine, ws66i, nad TCP, demo)
  • these could remove their overrides by setting _attr_volume_step and
    letting the base class handle it

Integrations with ONLY VOLUME_STEP (no VOLUME_SET): 9 (epson, xbox,
xiaomi_tv, harman_kardon_avr, hdmi_cec, androidtv_remote, mediaroom,
roku, lookin) - these must keep overrides since the base class requires
VOLUME_SET to auto-step

Interesting anomalies:

  • sonos, denon, devialet, cmus, sisyphus override volume_up/down but
    don't declare VOLUME_STEP - the UI won't show step buttons even though
    the methods work
  • dunehd overrides volume_up/down but declares NEITHER volume feature -
    truly unreachable code
#

It's a mess 😅

#

btw, besides receivers, I think that my sonos 5 also could benefit from a max volume limit, just because it's so god damn loud

errant oyster
#

I think that we can also make the implementation work as follows:

  • if no max volume set: do what we do today
  • if max volume set, intercept calls to volume up and block them if they exceed current volume
errant oyster
#

And put it on the agenda to discuss 😄

errant oyster
#

I turned that branch into PRs

gaunt cobalt
#

FYI the Sonos speakers have a volume limit option in settings. (sorry for off-topic)

scarlet barn
#

The bot also pointed that out, but the PR was merged without addressing the comment (by fixing it, or by saying we don't care).

#

BTW, I'm not sure if you saw it, but back when I was working on this I also opened - and then reverted - PRs removing overridden volume up/down methods.

#

I don't see the point of doing that again before the path forward is clear.

errant oyster
#

Is it good though to fetch the volume 🤔 we can also update local data after a call to set_volume

pure umbra
errant oyster
#

Why?

pure umbra
#
  1. It doesn't check whether volume_step is set by the integration. If it's not, then it would fall back to default, which is incorrect.
  2. It doesn't use the native command for volume up/down, instead always using volume set, which is suboptimal.
  3. It doesn't track the desired volume, always using the current volume known to the integration, which is likely stale in push-based integrations.
#

So the implementation either needs to get a lot more sophisticated, or leave out the handling of volume up/down completely.