Conversation
Pehrsons
left a comment
Pehrsons
left a comment
There was a problem hiding this comment.
Some minor issues to fix before merging, but in general LGTM.
| function startEqualizer(stream) { | ||
| const audioCtx = new AudioContext(); | ||
| const analyser = audioCtx.createAnalyser(); | ||
| audioCtx.createMediaStreamSource(stream).connect(analyser); |
There was a problem hiding this comment.
There's a MediaStreamTrackSource now. The MediaStreamSource is obsolete really, since it locks onto the first track and basically works as the track source anyway.
| muted.type = "checkbox"; | ||
| muted.checked = true; | ||
| muted.onclick = () => { video.muted = muted.checked; }; | ||
| const equalizer = document.createElement("canvas"); |
There was a problem hiding this comment.
This is a visualizer, not an equalizer. An equalizer is used to change the gain of certain frequencies, in order to "equalize" them.
Applies throughout.
| } | ||
| controls.appendChild(camOn.parentNode); | ||
| camOn.onclick = () => { videoTrack.enabled = camOn.checked; }; | ||
| camOn.onclick(); |
There was a problem hiding this comment.
can't the checked state change through other means than click? keyboard navigation and selecting it with space is what I'm thinking. The "change" event looks like the right thing. Also applies to micOn and elementMuted.
| for (let y of data) { | ||
| y = equalizer.height - (y / 128) * equalizer.height / 4; | ||
| let c = Math.floor((x*255)/equalizer.width); | ||
| canvasCtx.fillStyle = "rgb("+c+",0,"+(255-x)+")"; |
There was a problem hiding this comment.
This looks like it could be psychedelic. Do you have a screenshot?
| canvasCtx.fillRect(x, y, 2, equalizer.height - y) | ||
| x++; | ||
| } | ||
| analyser.getByteTimeDomainData(data); |
There was a problem hiding this comment.
This data is never used.
|
|
||
| let interval = setInterval(() => { | ||
| if (stream.getAudioTracks()[0].readyState != "live") { | ||
| clearInterval(interval); |
There was a problem hiding this comment.
Or use the ended event on the track to clear the interval. Also close the AudioContext at that point please.
| clearInterval(interval); | ||
| return; | ||
| } | ||
| canvasCtx.fillStyle = "#ffffff"; |
There was a problem hiding this comment.
clearRect shouldn't need fillStyle, only fillRect, no?
| canvasCtx.lineWidth = 5; | ||
| canvasCtx.beginPath(); | ||
| canvasCtx.stroke(); | ||
| }, 1000 * equalizer.width / audioCtx.sampleRate); |
There was a problem hiding this comment.
wait what, why is the width of the canvas part of the equation?
| let c = Math.floor((x*255)/equalizer.width); | ||
| canvasCtx.fillStyle = "rgb("+c+",0,"+(255-x)+")"; | ||
| canvasCtx.fillRect(x, y, 2, equalizer.height - y) | ||
| x++; |
There was a problem hiding this comment.
Have you made sure x doesn't overshoot the canvas width? That would be unnecessary. You can control how many frequency buckets you get from the analyser with the fftSize attribute.
2 participants
Address feedback in #32 (comment)