initial implementation of screen sharing via cf workers - #10
Open
silicakes wants to merge 13 commits into
Open
Conversation
Deploying deluge-extensions with
|
| Latest commit: |
0ba9ff7
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5489b55a.deluge-extensions.pages.dev |
| Branch Preview URL: | https://support-screen-streaming.deluge-extensions.pages.dev |
Deploying dex-beta with
|
| Latest commit: |
0ba9ff7
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6828b2a7.dex-beta.pages.dev |
| Branch Preview URL: | https://support-screen-streaming.dex-beta.pages.dev |
There was a problem hiding this comment.
Pull request overview
This PR implements screen sharing/streaming functionality using Cloudflare Workers and Durable Objects, allowing users to share their Deluge display to viewers on devices without WebMIDI support (like iOS).
Key Changes
- Adds WebSocket-based relay infrastructure (Cloudflare Worker + Durable Object and local Node.js relay)
- Implements viewer mode for displaying shared screens without WebMIDI
- Adds room-based streaming with optional password protection and viewer cap
Reviewed changes
Copilot reviewed 29 out of 30 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| worker/wrangler.toml | Cloudflare Worker configuration for screen streaming relay |
| worker/src/index.ts | Durable Object implementation for room management and frame relay |
| server/relay.mjs | Local Node.js relay server for development/LAN testing |
| functions/_worker.ts | Cloudflare Pages integration wrapper for the relay |
| src/main.tsx | Entry point modified to support viewer mode routing via roomId param |
| src/lib/screenStreaming/* | Core utilities for room codes, password tokens, codec, and WebSocket URL building |
| src/services/screenStreamingStreamer.ts | Streamer-side service managing MIDI subscription and frame encoding |
| src/components/screenStreaming/* | UI components for streaming modal, viewer app, and QR code generation |
| src/lib/fullscreen.ts | Enhanced wake lock management for both fullscreen and visibility changes |
| src/lib/display.ts | Added polling state check function |
| docs/screen-streaming.md | Comprehensive documentation for the screen streaming feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ): DisplaySysexFrame { | ||
| const bytes = data instanceof Uint8Array ? data : new Uint8Array(data); | ||
| if (bytes.length < STREAM_HEADER_BYTES) { | ||
| throw new Error("frame too short"); |
There was a problem hiding this comment.
The error message "frame too short" is vague. Consider making it more descriptive, such as "Frame is too short (minimum 11 bytes required)" to help with debugging.
Suggested change
| throw new Error("frame too short"); | |
| throw new Error(`Frame is too short (minimum ${STREAM_HEADER_BYTES} bytes required)`); |
| throw new Error("frame too short"); | ||
| } | ||
| if (bytes[0] !== STREAM_MAGIC_0 || bytes[1] !== STREAM_MAGIC_1) { | ||
| throw new Error("bad magic"); |
There was a problem hiding this comment.
The error message "bad magic" is unclear. Consider making it more descriptive, such as "Invalid frame magic bytes (expected 'DX')" to aid debugging.
Suggested change
| throw new Error("bad magic"); | |
| throw new Error( | |
| `Invalid frame magic bytes (expected 0x${STREAM_MAGIC_0.toString(16).padStart(2, "0")}, 0x${STREAM_MAGIC_1.toString(16).padStart(2, "0")}; got 0x${bytes[0].toString(16).padStart(2, "0")}, 0x${bytes[1].toString(16).padStart(2, "0")})` | |
| ); |
| for (let x = 0; x < qr.size; x++) { | ||
| if (!qr.getModule(x, y)) continue; | ||
| modules.push( | ||
| <rect x={x + border} y={y + border} width="1" height="1" />, |
There was a problem hiding this comment.
The rect elements generated in a loop lack unique key props. In Preact/React, list items should have a unique key to help with efficient rendering. Consider adding a key based on x and y coordinates.
Suggested change
| <rect x={x + border} y={y + border} width="1" height="1" />, | |
| <rect key={`${x},${y}`} x={x + border} y={y + border} width="1" height="1" />, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.