Implement AlsaMixerObserver with actor model approach

[st8ed]

This PR addresses several issues and proposes a solution based on a customized Pykka actor runtime. I tried to keep it as simple as I could.

Issues:

• Sound devices with multiple poll descriptors are not supported. This is essential because ALSA utilities [1] and particular device providers [2] rely on POLLERR event, which on Linux can be generated either by a socket (not sure about this) or the write end of a pipe, citing man poll 2:

POLLERR
              Error condition (only returned in revents; ignored in
              events).  This bit is also set for a file descriptor
              referring to the write end of a pipe when the read end has
              been closed.

So ALSA devices ideally must provide at least 2 poll descriptors, one for notifications about changes (a read end of some pipe) and another for notifications about device disconnection (a write end of some other pipe, which therefore can't generate read events and leads to use of 2 descriptors).

It's important to mention that poll() behaviour is undefined if FD was closed during polling [3]. On my system a blocking poll() continues to block execution after FD has been closed, so it can lead to an unresponsive state of application if poll() isn't interrupted by something else later.

• Actual event polling is currently implemented as an infinite loop. I'm not sure to what degree, but wasted CPU ticks on context switching can matter for users on battery power. Increasing timeout value could decrease this, but in current state it also leads to unpleasant delays on application shutdown.

• Currently once a sound device has been disconnected, Mopidy-ALSAMixer enters a broken state and requires restart of Mopidy to work again, even though Mopidy itself continues to play music if I plug device back.

• (Related to Pykka) If some actor enters blocking call, it won't process any messages and will block all senders until it finishes this call. For example, if an actor calls simple time.sleep, it will break normal shutdown of whole application, even though under normal circumstances it shuts down due to SIGINT signal caught by Python and raised as an exception. To demonstrate there's simple script [4]. I understand there's no general way no interrupt any kind of blocking call due to Python's nature, but in case of poll there actually is a way, which I exploited in this PR. There's also a working example how to use this new actor runtime [4]. Potentially this approach can be used to implement actual interruptable sleep() call as part of Pykka's interfaces. I haven't thoroughly read Pykka's issue tracker, but i believe it's mentioned somewhere.

Proposal:

The general idea is to use auxiliary pipe to interrupt select.epoll.poll() blocking call. Any message sent to an actor is preceded with a write operation to this pipe, so once Python internal execution context switches to the blocked thread, it will be able to continue and handle received messages, returning back to blocking interruptable sleep thereafter.

I tested this PR on x86_64 with Pipewire backend and aarch64 with bluez-alsa.

[djmattyg007]

It looks like the call to AlsaMixerObserver.start() should be moved to a new method named _start_observer(), to match what you've done for the "stop observer" call.

[st8ed]

It came to my mind, but there's more than one way to implement _start_observer:

def _start_observer(self, exc):
    # Perhaps we should raise an exception is observer is already running?
    self._observer = AlsaMixerObserver.start(exc, self.actor_ref.proxy())

def _start_observer(self, exc):
    # This check is meaningless iif _start_observer()
    # is used only within AlsaMixer class
    if self._observer is None:
        self._observer = AlsaMixerObserver.start(exc, self.actor_ref.proxy())

def _start_observer(self, exc):
    # This is actually restart_observer()
    self._stop_observer()
    self._observer = AlsaMixerObserver.start(exc, self.actor_ref.proxy())

I struggled to decide which one to choose, so being guided by "There should be one -- and preferably only one -- obvious way to do it" I introduced restart_observer() as public method. This way _start_observer() would then need to be implemented as no. 1 from above, which is very repetitive in my opinion and unnecessary because it's internal method.

Does such reasoning make sense or it's still better to explicitly define _start_observer() as in no. 1 only for the sake of consistency?

[djmattyg007]

I still think it's important for consistency. It helps define the One True Way of doing what needs to be done.

[djmattyg007]

I'm curious why this logging has been removed.

[st8ed]

I removed it for consistency (get_volume has no such logging) and because I thought this method (as well as get_volume) may be called repeatedly generating lots of error messages due to the actual error being suppressed.

I'm grateful you bring this up because now I think it's better to do the opposite and add logging to get_volume too, as ALSAAudioError can occur during alsaaudio.Mixer.getvolume, not only during alsaaudio.Mixer.

[djmattyg007]

It isn't super clear exactly what code needs updating (and how) when a new version of pyalsaaudio is released.

Is this code forwards-compatible, or will it break with newer versions of pyalsaaudio?

[st8ed]

Added clarifications. It's compatible with newer versions, I tried to underline it now phrasing it as "it can be removed".

Compatibility follows from the fact that max absolute value of correct file descriptor data type -- signed int32 -- is about a half of unsigned int32 (used in current latest release). So this check essentially becomes just garbage.

[st8ed]

Thank you for commenting. It allowed me to add a few more improvements about logging.

[jodal]

You could set use_daemon_thread = True as a class variable here to make sure this thread will not block the application from exiting.

Ref https://pykka.readthedocs.io/en/stable/runtimes/threading/#pykka.ThreadingActor.use_daemon_thread