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

Replaygain support for Web Player #243

Merged
merged 7 commits into from
Sep 22, 2023
Merged

Conversation

kgarner7
Copy link
Collaborator

@kgarner7 kgarner7 commented Sep 11, 2023

This PR adds ReplayGain support for web player. Notes:

  • uses WebAudio (which is not supported in all browsers)
  • WebAudio and MediaSession to not behave well, so image/title display may not behave well Github Issue
  • PLEASE test this in Safari, when I added this to Navidrome there was an issue on MacOS This should be good now (I have a personal deployment of it which behaves properly now), but still worth checking
  • While this PR does support setting the sampling rate, I couldn't figure out how to cleanly support configuring that without a refresh
  • It might also be nice to toggle RG from the full-screen player, but the menu is getting kinda full, so I'm going to refrain from adding anything more there

@vercel
Copy link

vercel bot commented Sep 11, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
feishin ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 21, 2023 6:31pm

@kgarner7
Copy link
Collaborator Author

A sample rate that can be configured without a refresh is unlikely (at least with createMediaElementSource) for a number of reasons:

  • to change the sample rate, you would need to delete and create a new audio context
  • this would then require connecting the audio element to this new context
  • However, per https://github.com/WebAudio/web-audio-api/issues/1202, you cannot disconnect a media source from source node. This means that the one you first created is the one you're stuck with
  • HOWEVER, you cannot attach nodes created from one context to another context.

It might be possible to do through the more roundabout way, but I'm not sure that makes sense.

@kgarner7 kgarner7 marked this pull request as ready for review September 12, 2023 04:51
@jeffvli jeffvli merged commit 65f28bb into jeffvli:development Sep 22, 2023
2 checks passed
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

Successfully merging this pull request may close these issues.

2 participants