Skip to content

Commit

Permalink
Remove audio player thread
Browse files Browse the repository at this point in the history
The current code spawns a dedicated thread to handle playing sound effects.

This is unnecessary as the `simpleaudio` API already is non-blocking, i.e. calling `wave_obj.play()`
returns immediately while the sound plays in the background (see https://simpleaudio.readthedocs.io/en/latest/capabilities.html#asynchronous-interface). At the worst,
loading the sound file, which is synchronous, could block the calling thread for a few milliseconds
but that happens only once and is unlikely to be an issue.

The `AudioWorker.play` method does have a `blocking` argument that is supposed to support
blocking the caller until the sound has finished playing, but this is never used in the current codebase.

But more conclusively, it turns out that due to an oversight, practically no code currently executes
on the audio player thread. The `AudioWorker` object is assigned to a new thread, but because the
`AudioPlayer.play` method calls `__worker.play` without going through a signal, the `AudioWorker.play`
method actually executes on the thread that called `AudioPlayer.play`. In practice this is the UI thread,
because `audio.play` is called through a signal on `SignalReceiver`, which lives on that thread. Thus,
the time-sensitive engine thread is already isolated from any slowness in reading or playing audio.

This commit reduces the `AudioPlayer` class to a plain Python object whose only responsibilities
are to load and play the application's sound effects. It also simplifies the file loading logic by
removing the direct call to the `wave` library in favor of the `from_wave_file` helper method
offered by `simpleaudio`, which in practice uses the exact same call to the `wave` library.

A notable benefit of removing this thread is that the application no longer crashes with an `abort`
call upon exit (issue #680), caused by improper termination of the audio thread.
  • Loading branch information
jonathanperret authored and dl1com committed Jul 25, 2024
1 parent 9d051f6 commit f3361f3
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 51 deletions.
1 change: 0 additions & 1 deletion requirements.build.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ pyserial==3.5
sliplib==0.6.2
bitarray==2.9.2
simpleaudio==1.0.4
wave==0.0.2
numpy==1.26.4
charset-normalizer<3.0.0
requests==2.31.0
62 changes: 16 additions & 46 deletions src/main/python/main/ayab/audio.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,73 +20,43 @@
"""Standalone audio player."""

from __future__ import annotations
import logging
from os import path

import simpleaudio as sa
import wave

from PySide6.QtCore import QObject, QThread
from typing import TYPE_CHECKING, Optional
from typing import TYPE_CHECKING

if TYPE_CHECKING:
from .ayab import GuiMain


class AudioWorker(QObject):
def __init__(self, parent: GuiMain):
super().__init__()
self.__dir = parent.app_context.get_resource("assets")
self.__prefs = parent.prefs
class AudioPlayer:
def __init__(self, gui: GuiMain):
self.__dir = gui.app_context.get_resource("assets")
self.__prefs = gui.prefs
self.__cache: dict[str, sa.WaveObject] = {}

def play(self, sound: str, blocking: bool = False) -> None:
"""Play audio and wait until finished."""
# thread remains open in quiet mode but sound does not play
def play(self, sound: str) -> None:
"""Play audio."""
if self.__prefs.value("quiet_mode"):
return
# else
wave_obj = self.__wave(sound)
if wave_obj is None:
return
# else
play_obj = wave_obj.play()
if blocking:
# wait until sound has finished before returning
play_obj.wait_done()
wave_obj.play()

def __wave(self, sound: str) -> Optional[sa.WaveObject]:
def __wave(self, sound: str) -> sa.WaveObject | None:
"""Get and cache audio."""
if sound not in self.__cache:
self.__cache[sound] = self.__load_wave(sound)
wave_object = self.__load_wave(sound)
if wave_object is None:
return None
self.__cache[sound] = wave_object
return self.__cache[sound]

def __load_wave(self, sound: str) -> Optional[sa.WaveObject]:
def __load_wave(self, sound: str) -> sa.WaveObject | None:
"""Get audio from file."""
filename = sound + ".wav"
try:
wave_read = wave.open(path.join(self.__dir, filename), "rb")
except FileNotFoundError:
logging.warning("File " + filename + " not found.")
return None
except OSError:
logging.warning("Error loading " + filename + ".")
return None
else:
return sa.WaveObject.from_wave_read(wave_read)


class AudioPlayer(QThread):
"""Audio controller in its own thread."""

def __init__(self, parent: GuiMain):
super().__init__(parent)
self.__worker = AudioWorker(parent)
self.__worker.moveToThread(self)
self.start()

def __del__(self) -> None:
self.wait()

def play(self, sound: str) -> None:
self.__worker.play(sound, blocking=False)
filename = path.join(self.__dir, sound + ".wav")
return sa.WaveObject.from_wave_file(filename)
6 changes: 2 additions & 4 deletions src/main/python/main/ayab/signal_receiver.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
# https://github.com/AllYarnsAreBeautiful/ayab-desktop

from __future__ import annotations
from PySide6.QtCore import QObject, Signal, Qt
from PySide6.QtCore import QObject, Signal
from .engine.status import Status
from .engine.options import Alignment
from .engine.engine_fsm import Operation
Expand Down Expand Up @@ -73,9 +73,7 @@ def activate_signals(self, parent: GuiMain) -> None:
# self.statusbar_updater.connect(parent.statusbar.update)
self.blocking_popup_displayer.connect(display_blocking_popup)
self.popup_displayer.connect(display_blocking_popup)
self.audio_player.connect(
parent.audio.play, type=Qt.ConnectionType.BlockingQueuedConnection
)
self.audio_player.connect(parent.audio.play)
self.needles_updater.connect(parent.scene.update_needles)
self.alignment_updater.connect(parent.scene.update_alignment)
self.image_resizer.connect(parent.set_image_dimensions)
Expand Down

0 comments on commit f3361f3

Please sign in to comment.