feat(plugin): audio stream#4396
Conversation
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as spam.
This comment was marked as spam.
|
@coderabbitai review |
This comment was marked as spam.
This comment was marked as spam.
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/plugins/audio-stream/backend.ts`:
- Around line 40-84: The GET '/stream' route handler in the Hono app definition
has multiple Prettier formatting violations. Run Prettier on this code block to
automatically fix the formatting issues. This will ensure the code meets the
project's ESLint and Prettier formatting standards for the route handler,
including proper indentation, line lengths, and spacing throughout the handler
definition.
- Around line 126-133: The `stop()` method in the audio-stream backend creates a
Promise that only resolves when the server's close callback is invoked, but if
`this.server` is undefined, the callback never fires and the Promise hangs
indefinitely. Modify the `stop()` method to check if `this.server` exists before
calling `close()` on it; if the server is undefined, resolve the Promise
immediately without waiting for the callback, otherwise proceed with the
existing logic of passing the resolve callback to the `close()` method.
- Around line 41-50: The `/stream` endpoint route handler is missing CORS
headers required for cross-origin browser access. Add the
`Access-Control-Allow-Origin` header to the response in the GET `/stream` route
handler (where ctx.header calls are made for Content-Type and
Transfer-Encoding). Set the header to allow cross-origin requests from browsers,
typically using a value like '*' for public access or a specific origin if
needed. This should be added alongside the existing ctx.header calls that set
Content-Type and Transfer-Encoding.
- Around line 56-59: The `icy-audio-info` header value in the audio stream
backend is hardcoding `ice-channels=2` which incorrectly reports stereo
regardless of the actual channel configuration. Replace the hardcoded value `2`
in the `ice-channels=2` portion of the template literal with the configured
channel count from the config object (likely `config.channels`), so the header
accurately reflects whether mono or stereo is selected.
In `@src/plugins/audio-stream/BroadcastStream.ts`:
- Around line 26-33: The write method in BroadcastStream unconditionally
enqueues chunks to all subscribers, which can cause memory buildup when slow
clients fall behind. Before calling controller.enqueue(chunk) in the write
method, check the controller's desiredSize property. If desiredSize is less than
or equal to zero, indicating a backed-up queue, remove the controller from the
subscribers set instead of enqueueing. Only enqueue the chunk to controllers
with desiredSize greater than zero to prevent unbounded buffering on slow
clients.
In `@src/plugins/audio-stream/menu.ts`:
- Around line 1-12: Fix the import order and prettier formatting violations in
the file by organizing imports following the project's convention: external
dependencies first, then internal imports from `@/` paths, then type imports, with
each group sorted alphabetically. Run prettier on the file to auto-correct
formatting issues. The violations are reported at multiple locations throughout
the file (lines 1-12, 65-66, 80-81, 95-98, 110-111), so ensure all import
statements are properly reorganized and formatted consistently to pass static
analysis checks.
- Around line 55-57: The setConfig calls are spreading the entire config object
and then overriding the changed field, which can leak unwanted fields like
enabled across the config boundary. Instead of using the spread operator pattern
with config or currentConfig, pass only the specific field being updated to
setConfig. This pattern needs to be fixed at all five locations in the menu.ts
file where setConfig is called: the port update around line 56, the
currentConfig port update around line 74, the currentConfig name update around
line 89, the currentConfig volume update around line 104, and the currentConfig
pitch update around line 119. Replace each instance with a call to setConfig
that includes only the changed key-value pair.
In `@src/plugins/audio-stream/ogg-opus.ts`:
- Line 121: The return statement containing the this.rewrite method call with
bitwise operations has a Prettier formatting violation. Run Prettier's
auto-formatter on the file or manually reformat the return statement with the
bitwise AND and OR operations to comply with Prettier's line length and
formatting rules for the expression involving HEADER_TYPE_BOS and
HEADER_TYPE_EOS constants.
In `@src/plugins/audio-stream/renderer.ts`:
- Around line 174-182: The isStreaming flag is being set to true twice: once
inside the .then() block after successful worklet initialization (correct
placement) and once synchronously immediately after the .catch() handler before
the async operation completes. Remove the synchronous this.isStreaming = true
assignment that appears after the .catch() block to ensure the flag only becomes
true after the addModule promise successfully resolves, preventing other code
from thinking streaming is active when the worklet setup hasn't actually
completed yet.
- Around line 76-81: The startStreaming method accepts a bufferSize parameter
but no call sites pass this.config.bufferSize to it, so configuration changes to
buffer size have no effect. Find all invocations of the startStreaming method
throughout the file and add this.config.bufferSize as the bufferSize argument.
Additionally, update the restart logic within the onConfigChange method (around
line 271) to also pass this.config.bufferSize when restarting the stream,
ensuring that buffer size changes from the menu are properly applied.
In `@src/plugins/audio-stream/StreamProcessor.js`:
- Around line 27-30: The output passthrough logic in the condition block
checking outputs[0] and inputs[0] assumes that if outputs[0] exists as an array,
its channel indices will be accessible. However, outputs[0] can be an empty
array with no channels, causing outputs[0][0] and outputs[0][1] to be undefined.
Before calling .set() on outputs[0][0], add a check to verify that this channel
actually exists in the array. Similarly, the existing check for inputs[0][1]
should have a corresponding check to verify outputs[0][1] exists before calling
.set() on it. Add these existence checks for both output channels to prevent
TypeError crashes when the worklet attempts to call .set() on undefined values.
In `@src/plugins/audio-stream/test-gui.html`:
- Around line 469-510: The entire parsing logic in the while loop starting at
line 469 expects a custom wire format with JSON metadata followed by PCM data,
but the backend now sends Ogg/Opus encoded audio. Remove the JSON metadata
parsing logic (the metadata length reading, TextDecoder, and JSON.parse
sections) and either replace the stream processing to use the browser's native
<audio> element with the Ogg stream URL directly, or implement Ogg page parsing
and WebCodecs AudioDecoder to decode the Opus audio. The audioConfig
initialization based on parsed metadata will need to be restructured
accordingly.
- Around line 779-805: The reader variable obtained from
response.body.getReader() inside the connect() function is scoped locally and
cannot be cancelled from disconnect(), causing the fetch read loop to continue
running even after disconnect is called and potentially causing errors when it
accesses nullified audioContext or analyserNode. Move the reader variable
declaration to module scope so it can be accessed from both connect() and
disconnect() functions, then call reader.cancel() in the disconnect() function
before nullifying the audio-related variables to properly stop the read loop.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0cdb39b3-4947-46bb-be49-b8c730bb157c
📒 Files selected for processing (11)
src/i18n/resources/en.jsonsrc/pear-desktop.tssrc/plugins/audio-stream/BroadcastStream.tssrc/plugins/audio-stream/StreamProcessor.jssrc/plugins/audio-stream/backend.tssrc/plugins/audio-stream/config.tssrc/plugins/audio-stream/index.tssrc/plugins/audio-stream/menu.tssrc/plugins/audio-stream/ogg-opus.tssrc/plugins/audio-stream/renderer.tssrc/plugins/audio-stream/test-gui.html
|
Huh, coderabbit did a nice review |
There was a problem hiding this comment.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
oxlint
🚫 [oxlint] <typescript(no-unsafe-member-access)> reported by reviewdog 🐶
Unsafe member access [0] on an any value.
🚫 [oxlint] <typescript(no-unsafe-member-access)> reported by reviewdog 🐶
Unsafe member access [0] on an any value.
🚫 [oxlint] <typescript(no-unsafe-call)> reported by reviewdog 🐶
Unsafe call of a(n) error type typed value.
🚫 [oxlint] <perfectionist(sort-imports)> reported by reviewdog 🐶
Expected "./backend" to come before "./config".
🚫 [oxlint] <perfectionist(sort-imports)> reported by reviewdog 🐶
Expected "./config" to come before "@/types/music-player".
🚫 [oxlint] <perfectionist(sort-imports)> reported by reviewdog 🐶
Extra spacing between "@/types/music-player" and "./config".
🚫 [oxlint] <typescript(no-unsafe-assignment)> reported by reviewdog 🐶
Unsafe assignment of an any value.
🚫 [oxlint] <perfectionist(sort-imports)> reported by reviewdog 🐶
Expected "@hono/node-server" to come before "hono/streaming".
🚫 [oxlint] <perfectionist(sort-imports)> reported by reviewdog 🐶
Expected "@/providers/song-info" to come before "@/utils".
🚫 [oxlint] <perfectionist(sort-imports)> reported by reviewdog 🐶
Missed spacing between "@/providers/song-info" and "./config".
🚫 [oxlint] <perfectionist(sort-imports)> reported by reviewdog 🐶
Expected "./BroadcastStream" to come before "./config".
🚀 Build Artifacts Ready!The builds have completed successfully. You can download the artifacts from the workflow run: Available builds:
Note: Artifacts are available for 7 days. |
|
@JellyBrick CodeRabbit is now happy, oxlint doesn't complain, only macos build fails for whatever reason maybe this could make it into the 3.12 release? |
🚀 Build Artifacts Ready!The builds have completed successfully. You can download the artifacts from the workflow run: Available builds:
Note: Artifacts are available for 7 days. |
❌ Build FailedUnfortunately, one or more builds failed. Please check the workflow run for details: |
Audio Stream Plugin
Description
This plugin streams Pear Desktop's audio over HTTP as Opus in an Ogg container (Icecast-compatible). Any player or device on your local network — mpv, VLC, browsers, or other Icecast clients — can connect to a single URL and listen in real time, with per-song metadata embedded in the stream.
How It Works
Key Features
Use Cases
Technical Details
Configuration
Configurable through the Pear Desktop menu:
Stream URL format: http://localhost:{port}/stream
Summary by CodeRabbit