YTMD/youtube-music
1
0
Fork 0
Code Issues 445 Pull requests 34 Projects Releases 87 Packages Wiki Activity Actions

Add custom equalizer plugin with presets (bass booster, rock...) and the ability to add your own presets via a config.json file #2831

Open
ZDAVanO wants to merge 6 commits from ZDAVanO/master into master
pull from: ZDAVanO/master
merge into: YTMD:master
Conversation 11 Commits 6 Files changed 3 +655 -76
ZDAVanO commented 2025-01-05 12:13:44 +00:00 (Migrated from github.com)
Copy link

This PR adds a new Custom Equalizer plugin for EQ settings with a large selection of default presets, and the ability to make your own flexible presets with detailed settings and any number of BiquadFilter. There is no UI, custom presets are added via a config.json file.

image

This PR adds a new Custom Equalizer plugin for EQ settings with a large selection of default presets, and the ability to make your own flexible presets with detailed settings and any number of BiquadFilter. There is no UI, custom presets are added via a config.json file. ![image](https://github.com/user-attachments/assets/e00bee5b-b247-49f3-b854-4203dc8afd42)
arjix commented 2025-01-06 01:00:36 +00:00
Owner
Copy link

I'd recommend you split this into multiple smaller files, one for menu, one for renderer and one for the presets would be a good way of splitting this.

I'd recommend you split this into multiple smaller files, one for menu, one for renderer and one for the presets would be a good way of splitting this.
ZDAVanO commented 2025-01-06 09:38:58 +00:00 (Migrated from github.com)
Copy link

The code is quite small, so I think splitting it into multiple files might be unnecessary. Also, I haven't fully figured out how to do it properly yet.

The code is quite small, so I think splitting it into multiple files might be unnecessary. Also, I haven't fully figured out how to do it properly yet.
JellyBrick commented 2025-01-10 03:21:51 +00:00 (Migrated from github.com)
Copy link

It seems like there is some overlap in functionality with the equalizer plugin that is already included in this application.

It seems like there is some overlap in functionality with the equalizer plugin that is already included in this application.
ZDAVanO commented 2025-01-10 09:27:14 +00:00 (Migrated from github.com)
Copy link

Yes, but I don't quite understand why to use the already included plugin, since it connects all the filters to the audioSource, which does not allow you to add many filters, only 1. There is also no choice of standard presets, or the ability to add your own.

Yes, but I don't quite understand why to use the already included plugin, since it connects all the filters to the audioSource, which does not allow you to add many filters, only 1. There is also no choice of standard presets, or the ability to add your own.
arjix commented 2025-01-10 10:09:45 +00:00
Owner
Copy link

JellyBrick meant that your plugin could be merged with the old plugin, essentially updating the old plugin instead of introducing a new one

JellyBrick meant that your plugin could be merged with the old plugin, essentially updating the old plugin instead of introducing a new one
👍 1
arjix commented 2025-01-10 13:44:27 +00:00
Owner
Copy link

Nice!

Nice!
MulverineX commented 2025-03-01 21:06:18 +00:00 (Migrated from github.com)
Copy link

A mode for this that would be really nice is per-video EQ; instead of having to switch your presets manually every time a song plays, you can set it once and it'll keep track via videoid

A mode for this that would be really nice is per-video EQ; instead of having to switch your presets manually every time a song plays, you can set it once and it'll keep track via videoid
copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed 2025-03-28 00:35:39 +00:00
copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment
Copy link

Pull Request Overview

This PR adds a custom equalizer plugin that supports default and user-defined presets through a config.json file. Key changes include:

  • Removal of the old filters and presets configuration and menu setup.
  • Introduction of helper functions to create, connect, and clear biquad filters.
  • Updating the menu structure to support custom presets and configuration editing.
Files not reviewed (1)
  • src/i18n/resources/en.json: Language not supported
Comments suppressed due to low confidence (2)

src/plugins/equalizer/index.ts:60

  • [nitpick] Consider renaming 'allPresetsD' to 'allPresets' for clarity and consistency.
const allPresetsD = config.customPresets.concat(defaultPresets);

src/plugins/equalizer/index.ts:73

  • Using 'includes' to check for the preset in customPresets may lead to unexpected behavior if the preset objects are not the same reference. Consider comparing preset names instead.
label: config.customPresets.includes(preset) ? preset.name : t(`plugins.equalizer.menu.presets.list.${preset.name}`),
## Pull Request Overview This PR adds a custom equalizer plugin that supports default and user-defined presets through a config.json file. Key changes include: - Removal of the old filters and presets configuration and menu setup. - Introduction of helper functions to create, connect, and clear biquad filters. - Updating the menu structure to support custom presets and configuration editing. <details> <summary>Files not reviewed (1)</summary> * **src/i18n/resources/en.json**: Language not supported </details> <details> <summary>Comments suppressed due to low confidence (2)</summary> **src/plugins/equalizer/index.ts:60** * [nitpick] Consider renaming 'allPresetsD' to 'allPresets' for clarity and consistency. ``` const allPresetsD = config.customPresets.concat(defaultPresets); ``` **src/plugins/equalizer/index.ts:73** * Using 'includes' to check for the preset in customPresets may lead to unexpected behavior if the preset objects are not the same reference. Consider comparing preset names instead. ``` label: config.customPresets.includes(preset) ? preset.name : t(`plugins.equalizer.menu.presets.list.${preset.name}`), ``` </details>
Destroy666x commented 2025-04-27 20:04:23 +00:00 (Migrated from github.com)
Copy link

A mode for this that would be really nice is per-video EQ; instead of having to switch your presets manually every time a song plays, you can set it once and it'll keep track via videoid

Yea, but I think it's for another PR, would be nice to get this one merged first, as the progress compared to current EQ is already huge

> A mode for this that would be really nice is per-video EQ; instead of having to switch your presets manually every time a song plays, you can set it once and it'll keep track via videoid Yea, but I think it's for another PR, would be nice to get this one merged first, as the progress compared to current EQ is already huge
❤️ 2
arjix commented 2025-07-08 13:10:10 +00:00
Owner
Copy link

why was this closed?
all it needed was a rebase...

just because a PR is stale, doesn't mean it has to be recreated

why was this closed? all it needed was a rebase... just because a PR is stale, doesn't mean it has to be recreated
👍 4
arjix commented 2025-07-08 13:45:35 +00:00
Owner
Copy link

what does re-PR even mean?
the existing PR was fine as is, do you just want to be a co-author for doing nothing?

what does re-PR even mean? the existing PR was fine as is, do you just want to be a co-author for doing nothing?
This pull request has changes conflicting with the target branch.
  • src/plugins/equalizer/index.ts
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin ZDAVanO/master:ZDAVanO/master
git switch ZDAVanO/master

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch master
git merge --no-ff ZDAVanO/master
git switch ZDAVanO/master
git rebase master
git switch master
git merge --ff-only ZDAVanO/master
git switch ZDAVanO/master
git rebase master
git switch master
git merge --no-ff ZDAVanO/master
git switch master
git merge --squash ZDAVanO/master
git switch master
git merge --ff-only ZDAVanO/master
git switch master
git merge ZDAVanO/master
git push origin master
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
awaiting-reply

Awaiting reply

  
breaking changes

   
bug

Something isn't working

  
cannot-reproduce

Bug cannot be reproduced

  
dependencies

Pull requests that update a dependency file

  
documentation

   
duplicate

This issue or pull request already exists

  
electron-issue

It's an Electron or Electron related dependencies issue (not YTM-Desktop issue)

  
enhancement

New feature or request

  
fix-available

A fix to the issue is available in a new version

  
good first issue

Good for newcomers

  
help wanted

Extra attention is needed

  
invalid

This doesn't seem right

  
javascript

Pull requests that update javascript code

  
need more information

Need more information about the issue

  
need rebase

This plugin need rebase

  
official-youtube-music-issue

It's an YouTube's issue (not YTM-Desktop issue)

  
plugin request

An issue/discussion requesting for a new plugin or new functionality to an existing plugin.

  
question

Further information is requested

  
release

Release

  
security

   
stale

   
Status: blocked

There are something to do before this

  
typo

   
wontfix

This will not be worked on

  
ytmd-issue

An issue/discussion about ytm desktop, unrelated to th-ch/youtube-music

No labels
awaiting-reply
breaking changes
bug
cannot-reproduce
dependencies
documentation
duplicate
electron-issue
enhancement
fix-available
good first issue
help wanted
invalid
javascript
need more information
need rebase
official-youtube-music-issue
plugin request
question
release
security
stale
Status: blocked
typo
wontfix
ytmd-issue
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: YTMD/youtube-music#2831
No description provided.
Delete branch "ZDAVanO/master"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?