Only add gain node and AudioContext when normalization enabled #6142
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Co-authored-by: Dmitry Lyzo <56478732+dmitrylyzo@users.noreply.github.com>
|
Cloudflare Pages deployment
|
There was a problem hiding this comment.
I wanted to comment this yesterday, but wanted to actually verify it, and I can confirm now that this doesn't fix the issue.
The problem start as soon as the AudioContext is connected to the media element, no other nodes are necessary. In Vue for instance, we use WebAudio for analyzing the audio waveform, but we don't apply any gain (or any other node that modifies the output), yet the error still happens.
In order to fix the underlying issue, WebAudio must be completely suspended.
|
But what this PR actually does is not to crate the |
gnattu
changed the title
There was a problem hiding this comment.
@gnattu My fault: I assumed audio normalization was off by default, but actually after checking the settings it is on by default. I also just checked the PR's diff and not the entire file, thinking an audio context was attached somewhere else and that the addGainElement just added the gain node.
Actually, shall we turn off audio normalization by default? This issue affects all Android devices with Chromium-based browsers (so also the app).
Although that's probably for another PR.
Currently, we create a Web Audio session and connect to it regardless of the normalization status, hoping it will work. However, it turns out that some devices exhibit buggy behavior when the Web Audio session is connected to a gain node, causing distorted audio playback. This PR makes that the gain element is only created when normalization is not off. The downside is that users must manually stop the currently playing song and start a new one to apply the state change between on and off. By doing this, users experiencing bugged playback can turn off the normalization so that the playback behavior would be the same as 10.8 which should be fine.
We do need this workaround because:
Changes
Issues
See jellyfin/jellyfin#11614, hopefully will workaround that.