Svoboda | Graniru | BBC Russia | Golosameriki | Facebook
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migration guide for major breaking changes due to audio backend rework #1175

Open
fladd opened this issue Jul 19, 2024 · 4 comments
Open

Migration guide for major breaking changes due to audio backend rework #1175

fladd opened this issue Jul 19, 2024 · 4 comments

Comments

@fladd
Copy link

fladd commented Jul 19, 2024

As described here, the major breaking audio changes that have been introduced in 2.0.12 have effects on other software that relies on Pyglet for audio playback (in my case https://github.com/zipped-album/zap).

Due to the lack of documentation of these changes, it is not straight-forward to make already existing (now broken) code - which worked fine in Pyglet 1 and Pyglet 2 (until 2.0.12) - work again.

Assuming that 2.0.10 will not be maintained in parallel in the future, a migration guide, which explains the changes in detail and gives step-by-step instructions on what parts need adaptation, would help with migrating existing software to 2.0.12 and beyond.

@caffeinepills
Copy link
Collaborator

caffeinepills commented Jul 20, 2024

Thanks for the report.

Can you provide information to help us with your issue? Including the information specified in the bug report form, if you can share the output of the python -m pyglet.info command. If this is not possible, please provide the OS information, Python information, Codec (If ffmpeg, which version), audio driver being used (PulseAudio, OpenAL, etc), and if possible an example of audio having issues (or format).

EDIT: I am unable to run your code on my machine, nor do I have an example of a Zip album. However from initial glance, it looks like you are overriding the FFmpegSource. This was not meant to be overridden in such a complete manner as it was done here.

I can tell this overridden class was based on a much older version. For example, the temporary file behavior was replaced in version 1.5.2, in 2020. I would probably start with getting your FFmpegSource that you overrode up to date: https://github.com/pyglet/pyglet/blob/v2.0.16/pyglet/media/codecs/ffmpeg.py#L498. What I would do now is just test whether or not the default FFmpegSource works properly in your application. (If there is functionality not present in the default FFmpeg source, we can see about adding it provided we have examples to test and it doesn't break existing usage.)

As far as seeking goes, I do see you are double seeking here: https://github.com/zipped-album/zap/blob/14c82fc391e5b411b6d29a7656e59bd6e7658c6a/zap/player.py#L372-L375 Could you try a single seek if you get playback working?

@fladd
Copy link
Author

fladd commented Jul 20, 2024

@caffeinepills Thanks for getting back to me.

I indeed had to override the __init__ of FFmpegSource for two reasons:

  1. The original did not support 24bit FLAC files (it does specify 24 bits, but as far as I could tell, files when loaded in Pyglet were 32bit padded, so I added that and it worked)
  2. I needed to implement dithering, because OpenAL does not support 24 bits, so I had to resample, but I wanted to do this properly (i.e with dithering and noise shaping)

I actually just had a look at my customized __init__ yesterday and it seemed pretty up to date. I only had to change the part that used asbytes_filename, as this does not seem to be supported since 2.0.12 anymore.

The temp file loading I do is because I play files directly from a ZIP file, and for this to work, I apparently need to first unzip the file I want to play into a temporary file on disk.

I am getting somewhere, though! With one simple change (apparently the DirectSoundbuffer_size and OpenALideal_buffer_size have been unified to _buffered_data_ideal_size across all drivers in 2.0.12), I can now get my AudioPlayer class to work! I have audio and it seeks correctly it appears.

However, I still cannot get the more important GaplessAudioPlayer class (which inherits from the former) to work. This uses SourceGroup (and my additions to it) in order to allow for gapless playback between tracks. Have there been any changes to how SourceGroups work, and especially in how they report time?

If you could maybe have a look at that part (starting from line 389) and in particular how I seek and how I update this player (every 100ms), that would be great! Maybe you spot the issue immediately, given that you are familiar with the internal changes in 2.0.12.

Btw., for actually loading a ZippedAlbum, you could download any free album from Bandcamp in FLAC format, for instance this (random pick; click on "Buy album" and under "name your price" put in 0).

@Square789
Copy link
Contributor

The changes are very wide, but were supposed to have no effect on users.
Of course there has been a critical oversight: The new PreciseStreamingSource only sets audio/video format on initialization. You are using a SourceGroup and since they are special - their audio_format getting set only when a source is added - their true format will always be shadowed by the wrapping PreciseStreamingSource's None and players won't start an audio player.
Working on a PR for that.

zap suffers from two other issues, but both of those are related to internal access, so fair game for a minor version change:
a) The player's audio buffer size, which is not exposed is accessed. For size of buffered data, the new attribute name unified across backends is _buffered_data_ideal_size, although still not public.
b) The private timer of players is modified once a source changes, which will then cause very audible and ugly sample repetition as the player tries to resync.
The fix for that should be recording a value somewhere in your program's space to then subtract from other calculations involving player.time.

This is a hack capable of fixing the primary issue, although if possible it's probably preferrable to wait on pyglet 2.0.11 until (presumably) a fix enters 2.0.17.

try:
    from pyglet.media.codecs import PreciseStreamingSource

    def _saf(s, f): s._source.audio_format = f
    def _svf(s, f): s._source.video_format = f
    def _sin(s, i): s._source.info = i
    PreciseStreamingSource.audio_format = property(lambda s: s._source.audio_format, _saf)
    PreciseStreamingSource.video_format = property(lambda s: s._source.video_format, _svf)
    PreciseStreamingSource.info = property(lambda s: s._source.info, _sin)
    PreciseStreamingSource.duration = property(lambda s: s._source.duration)
except ImportError:
    pass

@fladd
Copy link
Author

fladd commented Jul 24, 2024

@Square789

Thanks for the snippet. Adding this to the beginning of Zap's player.py gives back audio playback, however, as you predicted, it now stutters (but see below).

Concerning the two issues you mention:
a) Well, until now I used buffer_size (DirectSound) and ideal_buffer_size (OpenAL), which are both not explicitly private (unlike the new unified _buffered_data_ideal_size.
b) I indeed modify the internal timer of the player, as this was seemingly the only way to make gapless playback work correctly (as the time of the player depends on how many sources are queued in the source group). I followed your advise to work with an offset after a source changes (apparently after seeking once though, the player's timer resets to reflect the correct time from the start of the currently playing source, and the offset is not needed anymore). This seems to restore non-stuttering playback.

Square789 added a commit to Square789/pyglet that referenced this issue Jul 27, 2024
benmoran56 pushed a commit that referenced this issue Jul 29, 2024
* Remove `PreciseStreamingSource`; Move back audio buffering into players.
Square789 added a commit to Square789/pyglet that referenced this issue Aug 3, 2024
* Remove `PreciseStreamingSource`; Move back audio buffering into players.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants