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

feat equalizer #296

Merged
merged 16 commits into from
Mar 31, 2019
Merged

feat equalizer #296

merged 16 commits into from
Mar 31, 2019

Conversation

c-jacquin
Copy link
Collaborator

@c-jacquin c-jacquin commented Mar 18, 2019

I replace react-sound by a react component with same props wrapping an audio tag.
I think next steps are:

  • add an equalizer view ? (or any other ui solution, modal ?)
  • add an equalizer component using semantic-ui range input
  • store equalizer value in redux
  • use the equalizer value in Audio component with AudioContext api
  • replace range input by a chart.js magic thing
  • define presets value for equalizer
  • replace dumb presets with good ones
  • optional visualization

@c-jacquin c-jacquin requested a review from nukeop March 18, 2019 09:39
@nukeop
Copy link
Owner

nukeop commented Mar 18, 2019

Awesome, the sound component itself could even be published as your own package since it exposes the same API as react-sound and could serve other people interested in replacing it.
Remember to remove package-lock so we don't inflate the LOC changed in the commits. I'll test how this works in practice in the evening.

@c-jacquin
Copy link
Collaborator Author

Good idea for the package, will check wich names are available.
Sorry for the lockfile, i will add it in the .gitignore (i know myself i will do it again otherwise)

@nukeop
Copy link
Owner

nukeop commented Mar 18, 2019

The equalizer itself could be created using the same sliders as the volume component, but vertically, or we could do something more fancy using chartjs, like this: https://cdn.guidingtech.com/media/assets/WordPress-Import/2016/04/apple-music-itunes-spotify-eq-equalizer-5.png

@nukeop
Copy link
Owner

nukeop commented Mar 18, 2019

You can also use .npmrc (and commit it if you want) https://codeburst.io/disabling-package-lock-json-6be662f5b97d?gi=c28afc04af04

@c-jacquin
Copy link
Collaborator Author

i love the idea of chart.js
i will first poc with simple vertical slider and then think more about it.

@nukeop
Copy link
Owner

nukeop commented Mar 18, 2019

As for chartjs, I've used it a lot at work, and created a little widget with it here: https://github.com/nukeop/react-ui-cards#cryptocurrency-card

So we could reuse the part of config for those cards to make a fancy equalizer. Not sure if it's possible to drag the points on the chart easily though.

@c-jacquin
Copy link
Collaborator Author

everything is possible with chart.js ;)

@c-jacquin
Copy link
Collaborator Author

Where do you imagine the equalizer in the ui ?

  • its own view ?
  • a popup with a button near the seekbar ?
  • somewhere else ?

@nukeop
Copy link
Owner

nukeop commented Mar 18, 2019

I think it could occupy its own view, with a button to access it somewhere in the settings.

@nukeop
Copy link
Owner

nukeop commented Mar 19, 2019

do you have a screenshot of your work at the moment? can't wait to see it in action

@c-jacquin
Copy link
Collaborator Author

Capture d’écran de 2019-03-19 14-21-00

@c-jacquin
Copy link
Collaborator Author

didnt notice i hav a little size problem i update this in 5 min

@nukeop
Copy link
Owner

nukeop commented Mar 19, 2019

looks really awesome

@c-jacquin
Copy link
Collaborator Author

better like this
Capture d’écran de 2019-03-19 14-24-43

@c-jacquin
Copy link
Collaborator Author

c-jacquin commented Mar 19, 2019

For the presets we could:

  • persist some equalizer configurations
  • add a saveAs feature
  • load the configurations and the selected one at startup and store in redux
  • add a list exactly like the spotify equalizer
  • add a few endpoints in the http api

@c-jacquin
Copy link
Collaborator Author

We can also imagine some sound visualition as bar chart just behind the line chart

@c-jacquin
Copy link
Collaborator Author

c-jacquin commented Mar 19, 2019

And make the equalizer usage optional

@c-jacquin c-jacquin marked this pull request as ready for review March 20, 2019 13:16
@c-jacquin c-jacquin force-pushed the feat.equalizer branch 2 times, most recently from 8088956 to 4513a36 Compare March 20, 2019 13:21
@c-jacquin
Copy link
Collaborator Author

c-jacquin commented Mar 23, 2019

i force push because i rebase locally to resolve conflict before the big mess. sorry for the spam :)

@c-jacquin
Copy link
Collaborator Author

We could make the equalizer customizable by putting the frequencies used by the equalizer (and sound component) in the settings and make them editable.

@c-jacquin
Copy link
Collaborator Author

I have put the Sound component in another module: https://github.com/charjac/react-sound-html5.
docs here => https://charjac.github.io/react-sound-html5/

The last important thing to do is to replace dumb presets by reel ones.
I have no idea where i can find help to do this, if someone has any clue ?

@nukeop
Copy link
Owner

nukeop commented Mar 26, 2019

Awesome, fantastic work, as always.

A good start would be to just copy some presets by eye from iTunes or whatever. If it ends up sounding off, we can correct it with another PR later.

@c-jacquin
Copy link
Collaborator Author

c-jacquin commented Mar 31, 2019

i add :

  • frequencies and some presets from https://webamp.org/.
  • preamp input like winamp or itunes
  • optional visualization tool (not necessary but fun to implement)

I put the equalizer link in the main menu, didnt get how to put it in settings.

@c-jacquin c-jacquin changed the title [wip]feat equalizer feat equalizer Mar 31, 2019
@nukeop
Copy link
Owner

nukeop commented Mar 31, 2019

Ready for review and merging?

@c-jacquin
Copy link
Collaborator Author

yep

@nukeop
Copy link
Owner

nukeop commented Mar 31, 2019

Wow what an incredible contribution. Probably the best feature implemented so far, even among lots of fantastic quality code from all the contributors. I tested this and it all works great. There are only some minor edits to the UI that I'd like to add in the coming days.

@nukeop nukeop merged commit 16c0336 into nukeop:master Mar 31, 2019
nukeop added a commit that referenced this pull request Sep 30, 2019
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