Fix: web audio example #3968
Fix: web audio example #3968
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
examples/webaudio.jsOops! Something went wrong! :( ESLint: 9.17.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by WalkthroughThe pull request introduces modifications to the Web Audio example in Changes
Sequence DiagramsequenceDiagram
participant User
participant WaveSurfer
participant AudioContext
participant Equalizer
User->>WaveSurfer: Initialize with URL
WaveSurfer->>AudioContext: Create Media Source
WaveSurfer->>Equalizer: Setup Filter Bands
User->>Equalizer: Adjust Slider Gains
Equalizer->>AudioContext: Modify Frequency Gains
Assessment against linked issues
Possibly related PRs
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Deploying wavesurfer-js with
|
| Latest commit: |
3e8ed03
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5b9f1c34.wavesurfer-js.pages.dev |
| Branch Preview URL: | https://webaudio.wavesurfer-js.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
examples/webaudio.js (3)
17-17: Potential confusion in click handler naming.
Callingwavesurfer.playPause()inside a'click'event might confuse new users expecting a separate “play” method. Consider renaming the event or clarifying it in comments.
36-40: Consider a named function or comment for clarity.
Chaining filters via.reduceis concise. A small comment explaining the chaining logic may improve maintainability.
52-66: Random initialization of slider values.
Using random defaults is fine for demos, but might introduce unpredictable behaviors for testing or production. Keep this in mind if consistent start values become necessary.src/plugins/timeline.ts (1)
254-254: Straightforward timeline offset logic.
(i + this.options.timeOffset) * pxPerSecis concise and clear. Make sure to note in documentation thattimeOffsetshifts the timeline's labeling rather than the audio playback.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
examples/webaudio.js(1 hunks)src/plugins/regions.ts(2 hunks)src/plugins/timeline.ts(3 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
examples/webaudio.js
[error] 48-48: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (9)
examples/webaudio.js (5)
5-6: Good introduction of eqBands.
Defining the equalizer bands upfront clarifies the audio processing pipeline.
13-14: Confirm the media file path.
Ensure that '/examples/audio/audio.wav' is correctly spelled and exists in your deployment environment.
19-31: Delay audio context creation until user gesture.
Creating the AudioContext inside once('play') is recommended for modern browser autoplay policies. Great approach!
33-34: Validate autoplay policies for createMediaElementSource.
Some browsers require explicit user gestures prior to creating media nodes. This seems fine since it’s inside the 'play' listener, but keep an eye on older or niche browsers.
42-43: Well-structured output routing.
Connecting the last filter node to audioContext.destination is a standard approach. Looks good!
src/plugins/timeline.ts (2)
28-28: Optional timeOffset property is a neat addition.
It gives developers more flexibility in labeling the timeline beyond the default start time.
39-39: Default value for timeOffset.
Defining a default of 0 is good practice to ensure consistent calculations even if not set by the user.
src/plugins/regions.ts (2)
98-102: Constructor parameter formatting.
Splitting params, totalDuration, and numberOfChannels across lines reads well. No functional changes needed.
270-270: Explicit condition for drag.
Using if (this.drag) this.emit('update-end') is clearer than coupling checks with other logic. Nice improvement.
| sliders.forEach((slider, i) => { | ||
| const filter = filters[i] | ||
| filter.gain.value = slider.value | ||
| slider.oninput = (e) => (filter.gain.value = e.target.value) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Avoid assignment inside arrow function return.
Static analysis flagged line 48 as confusing because of the inline assignment:
slider.oninput = (e) => (filter.gain.value = e.target.value)Move the assignment into a block for improved readability:
- slider.oninput = (e) => (filter.gain.value = e.target.value)
+ slider.oninput = (e) => {
+ filter.gain.value = e.target.value
+ }🧰 Tools
🪛 Biome (1.9.4)
[error] 48-48: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
Short description
Resolves #3821
Implementation details
The Web Audio example is updated to initialize AudioContext on wavesurfer play.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
timeOffsetproperty in the timeline plugin, ensuring a default value is set.Chores