-
-
Notifications
You must be signed in to change notification settings - Fork 225
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
Add music visualiser #2043
Add music visualiser #2043
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides the individual comments:
- There are some code smells reported by SonarCloud. Although checks are failing in master, run
npm run typecheck
andnpm run lint
to verify there are no new errors - Rename all instances of
visualiser
tovisualizer
.
<div ref="musicVisualiser" /> | ||
</template> | ||
|
||
<script lang="ts"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use composition API with script setup like we do everywhere else.
With Composition API you don't need the undefined either, since the full script section is garbage collected when the component is unmounted.
export default { | ||
mounted(): void { | ||
visualiser = new AudioMotionAnalyzer(this.$refs.musicVisualiser, { | ||
source: mediaElementRef._value, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't exist. Do you have the recommended extensions installed in VSCode? This error should appear to you straightaway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Studio code did not show any errors on my end so I checked the recommended extensions and only some of them auto installed. I installed them manually and it started showing some errors, it's fixed now (hopefully).
<script lang="ts"> | ||
export default { | ||
data(): object { | ||
return { | ||
isVisualising: false | ||
}; | ||
} | ||
}; | ||
</script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why???
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know that variable refs could be defined like this const isVisualizing = ref(false);
until I looked at the composition API docs.
Damn, I was kind of hoping I was going to get away with using British English :P |
if (mediaElementRef.value) { | ||
const audioCtx = new AudioContext(); | ||
|
||
audioCtx | ||
.createMediaElementSource(mediaElementRef.value) | ||
.connect(audioCtx.destination); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we need this at mount to connect to the visualizer instead of onBeforeUnmount? Why this on unmount?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you instantiate the visualizer it looks like it inserts itself between the audio source and the speakers automatically (which is why it's not in onMount), so when you disconnect the inputs and outputs it also ends up disconnecting the speakers. So I added those lines to reconnect the audio source directly to the speakers when you disable the visualizer.
3de8f67
to
87df773
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
87df773
to
6270b8d
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Cloudflare Pages deployment
|
This adds a music visualiser option when you fullscreen the music player which looks cool as heck.
Since the discussion I have changed the colours, decreased the number of bars, and increased the smoothing value and sample size to make it look better on 60hz monitors. I still recommend playing around with the parameters though.
Link to discussion
Example Screenshot:
Also sorry about accidentally closing the previous pull request, I clicked the sync fork button, which didn't do what I thought it would and in trying to fix it I made it even worse by reverting to a previous branch and deleting the old one which closed the pull request. I'm still quite rubbish at this git stuff.
Link to previous pull request