Screen Sharing in-app using ffmpeg + HLS#28
Conversation
WalkthroughAdds an HLS screen‑sharing subsystem: Rust backend code to spawn FFmpeg, run an Axum HTTP server with access‑code protection and optional localtunnel exposure; new tauri commands and shared state; plus a React screen‑share page, navigation option, and CSS for the UI. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Frontend (React)
participant Tauri as Tauri Commands
participant Backend as Rust Backend
participant FFmpeg
participant Tunnel as LocalTunnel
participant HTTP as Axum Server
User->>UI: Open HLS Screen Share Page
UI->>Tauri: check_ffmpeg(), list_ffmpeg_devices(), get_hls_server_info()
Tauri->>Backend: query status/devices
Backend-->>Tauri: status/devices
Tauri-->>UI: display results
User->>UI: Click Start Server
UI->>Tauri: start_hls_server_cmd(selectedDevices)
Tauri->>Backend: start server
Backend->>FFmpeg: spawn process (HLS output)
Backend->>HTTP: start Axum server (serve .m3u8/.ts with access-code)
Backend->>Tunnel: optionally spawn localtunnel
Backend-->>Tauri: return server info (code, port, url, tunnel)
Tauri-->>UI: show server info
User->>UI: Click Stop Server
UI->>Tauri: stop_hls_server_cmd()
Tauri->>Backend: stop server
Backend->>FFmpeg: terminate
Backend->>Tunnel: terminate
Backend->>HTTP: shutdown
Backend-->>Tauri: stopped
Tauri-->>UI: clear state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
src-tauri/Cargo.toml(1 hunks)src-tauri/src/lib.rs(5 hunks)src/App.css(1 hunks)src/App.jsx(2 hunks)src/components/HlsScreenSharePage.jsx(1 hunks)src/components/HomeOptions.jsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/App.jsx (1)
src/components/HlsScreenSharePage.jsx (1)
HlsScreenSharePage(4-202)
🪛 Biome (2.1.2)
src/components/HlsScreenSharePage.jsx
[error] 86-87: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 104-105: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 115-120: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 147-151: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 155-159: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 162-163: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🔇 Additional comments (5)
src/App.jsx (1)
8-8: LGTM! Clean integration of the new HLS screen-share page.The import and conditional rendering follow the established pattern used by other pages in the application.
Also applies to: 82-82
src-tauri/Cargo.toml (1)
31-36: Dependencies are appropriate for the HLS server feature.The added dependencies (tokio, axum, tower-http, etc.) align well with the async HTTP server requirements. The tokio "full" features flag includes more than needed, but this is acceptable for development convenience.
src/components/HomeOptions.jsx (1)
10-10: LGTM! Navigation option added correctly.The new "HLS Screen Share" option follows the established pattern and integrates cleanly with the existing navigation system.
src/App.css (1)
387-512: LGTM! Screen-sharing styles are well-structured.The new CSS classes provide a cohesive styling theme for the HLS screen-share UI, with proper button variants, responsive layouts, and consistent visual hierarchy.
src-tauri/src/lib.rs (1)
748-755: Verify that 6-character access codes provide sufficient security.The access code uses 36^6 (≈2.2 billion) possible combinations with no rate limiting or brute-force protection. For a localhost-only server this may be acceptable, but when exposed via localtunnel to the internet, an attacker could potentially brute-force the code.
Consider:
- Increase code length to 8-10 characters for stronger security
- Add rate limiting on the API endpoints to prevent brute-force attempts
- Implement temporary lockout after N failed attempts from the same IP
- Add expiration - codes should expire after a configured time period
For the tunneled scenario specifically, document the security implications and recommend users only share the access code through secure channels.
| #[tauri::command] | ||
| async fn check_localtunnel() -> Result<bool, String> { | ||
| // Check if npx is available | ||
| let npx_check = Command::new("npx") | ||
| .arg("--version") | ||
| .output() | ||
| .await; | ||
|
|
||
| if npx_check.is_err() { | ||
| return Ok(false); | ||
| } | ||
|
|
||
| // Try to run localtunnel --help (this will download it if needed, but we just check if it works) | ||
| // Actually, we'll just check if npx works - localtunnel will be downloaded on first use | ||
| Ok(true) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused command check_localtunnel should be removed or utilized.
The check_localtunnel command is defined but never called from the frontend. This is dead code that should either be removed or properly integrated if the functionality is needed.
If the frontend should check localtunnel availability before starting the server, add a call in HlsScreenSharePage.jsx:
const checkLocaltunnel = async () => {
const available = await invoke('check_localtunnel');
if (!available) {
setError('Warning: localtunnel is not available. Server will only be accessible on localhost.');
}
};Otherwise, remove the command from the Rust code to reduce maintenance burden.
🤖 Prompt for AI Agents
In src-tauri/src/lib.rs around lines 599 to 614, the tauri command function
check_localtunnel is defined but never used; either remove it to eliminate dead
code or wire it into the frontend: if removing, delete the check_localtunnel
function and any unused imports to avoid warnings and rebuild; if keeping, add a
call from HlsScreenSharePage.jsx (or the appropriate frontend component) that
uses invoke('check_localtunnel') on mount or before starting the server and
handles the boolean result (set a warning/error state when false) so the command
is actually utilized.
| let content = fs::read(&file_path).map_err(|e| { | ||
| eprintln!("❌ Error reading file: {}", e); | ||
| StatusCode::INTERNAL_SERVER_ERROR | ||
| })?; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Reading entire segment files into memory is inefficient.
Segment files are loaded entirely into memory before sending to clients. For high-resolution streams, each segment can be several megabytes, causing unnecessary memory pressure and latency.
Consider using tower_http::services::ServeFile or implement streaming:
use tokio::fs::File;
use tokio::io::AsyncReadExt;
use axum::body::Body;
// Instead of fs::read, use streaming
let file = File::open(&file_path).await.map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
let stream = tokio_util::io::ReaderStream::new(file);
let body = Body::from_stream(stream);
Ok((
StatusCode::OK,
[(axum::http::header::CONTENT_TYPE, content_type)],
body,
))This reduces memory usage and improves responsiveness for large files.
Also applies to: 1006-1009
🤖 Prompt for AI Agents
In src-tauri/src/lib.rs around lines 907-910 (and similarly 1006-1009), the code
reads entire segment files into memory with fs::read; instead open the file
asynchronously and stream it to the response to avoid buffering the whole file:
use tokio::fs::File::open(...).await, wrap it with
tokio_util::io::ReaderStream::new(file), convert to an axum body
(Body::from_stream or Body::wrap_stream), set the appropriate content-type
header, and return (StatusCode::OK, headers, body), mapping file open/read
errors to StatusCode::INTERNAL_SERVER_ERROR as before. Ensure you add the tokio
and tokio-util/axum imports and adjust error handling for async operations.
src-tauri/src/lib.rs
Outdated
| // Start FFmpeg | ||
| let ffmpeg_handle = start_ffmpeg(&public_dir) | ||
| .await | ||
| .map_err(|e| format!("Failed to start FFmpeg: {}", e))?; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
No monitoring or error recovery for FFmpeg process.
After spawning FFmpeg, there's no verification that it's actually working or producing HLS segments. If FFmpeg fails immediately after startup (e.g., due to invalid device configuration or permissions), the server will appear to run successfully but produce no stream.
Add FFmpeg health monitoring:
// After spawning FFmpeg, wait and verify segment files are created
tokio::time::sleep(tokio::time::Duration::from_secs(5)).await;
// Check if stream.m3u8 was created
let m3u8_path = public_dir.join("stream.m3u8");
if !m3u8_path.exists() {
// Kill FFmpeg and return error
let _ = ffmpeg_handle.kill().await;
return Err("FFmpeg failed to generate HLS stream. Check device permissions and configuration.".to_string());
}Also consider adding a background task to monitor the FFmpeg process and restart it if it crashes during streaming.
🤖 Prompt for AI Agents
In src-tauri/src/lib.rs around lines 1080 to 1083, after spawning FFmpeg you
must verify it actually produces HLS output and recover if not: wait a short
period (e.g., 3–5s), check that public_dir/stream.m3u8 (or the configured HLS
manifest) exists and contains content, and if it doesn’t, terminate the spawned
FFmpeg process and return an error describing the failure (include hints about
device permissions/config). Additionally, add an optional background task that
watches the FFmpeg child process (await its exit or poll periodically) and
restarts it or logs/crashes the server if it unexpectedly dies during streaming.
| <button onClick={checkFfmpeg} className="btn-secondary" style={{ marginTop: '10px' }}> | ||
| Check Again |
There was a problem hiding this comment.
Add explicit type="button" to all button elements.
All six button elements are missing the explicit type attribute. In React, buttons default to type="submit", which can trigger unintended form submissions if placed inside a form element. This is a correctness and accessibility issue.
Apply this pattern to all buttons in the file:
-<button onClick={checkFfmpeg} className="btn-secondary" style={{ marginTop: '10px' }}>
+<button type="button" onClick={checkFfmpeg} className="btn-secondary" style={{ marginTop: '10px' }}>Repeat for all six button elements at lines 86, 104, 115, 147, 155, and 162.
Also applies to: 104-105, 115-120, 147-151, 155-159, 162-163
🧰 Tools
🪛 Biome (2.1.2)
[error] 86-87: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🤖 Prompt for AI Agents
In src/components/HlsScreenSharePage.jsx around lines 86, 104-105, 115-120,
147-151, 155-159, and 162-163, the button elements lack an explicit type and
will default to type="submit"; update each button element at those line ranges
to include type="button" (e.g., add type="button" to the JSX <button ...> tags)
so they do not trigger unintended form submissions.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (8)
src/components/HlsScreenSharePage.jsx (1)
157-159: Add explicittype="button"to all button elements.All six button elements are missing the explicit
typeattribute. In React, buttons default totype="submit", which can trigger unintended form submissions.Apply
type="button"to each button:-<button onClick={checkFfmpeg} className="btn-secondary" style={{ marginTop: '10px' }}> +<button type="button" onClick={checkFfmpeg} className="btn-secondary" style={{ marginTop: '10px' }}>Repeat for all buttons at lines 157, 249, 261, 292, 300, and 307.
Also applies to: 249-251, 261-266, 292-297, 300-305, 307-310
src-tauri/src/lib.rs (7)
729-744: Unused command: remove or integratecheck_localtunnel.The
check_localtunnelcommand is defined but never called from the frontend. Consider removing it to reduce dead code, or integrate it into the UI if the functionality is needed.
888-930: Hardcoded capture settings limit portability.The FFmpeg input arguments use hardcoded device indices (macOS: "2:0") and resolutions (1920x1080) that won't work on all systems. This is a known limitation of the current implementation.
As noted in previous reviews, consider allowing users to select devices (already partially implemented via
list_ffmpeg_devices) and detecting screen resolution dynamically.
933-995: No verification that FFmpeg successfully produces HLS output.After spawning FFmpeg, there's no check to ensure it actually generates the HLS stream files. If FFmpeg fails immediately, the server will appear running but produce no stream.
As noted in previous reviews, add a health check after spawning to verify that
stream.m3u8is created within a few seconds.
997-1003: Critical: API info endpoint exposes access code without authentication.The
/api/infoendpoint returns the access code to any requester, completely bypassing access control. Anyone who discovers the server URL can retrieve the code and access the stream.As noted in previous reviews, either remove this endpoint or require authentication before returning the access code.
1171-1171: Critical: Replace permissive CORS with restricted policy.
CorsLayer::permissive()allows requests from any origin, exposing the server to CSRF attacks and unauthorized access.As noted in previous reviews, restrict CORS to localhost origins and dynamically add the tunnel domain if needed.
1207-1207: Hardcoded port can cause startup failure.Port 3000 is hardcoded without checking availability. If another application is using this port, the server will fail to start.
As noted in previous reviews, implement port selection with fallback or allow users to specify a port.
1270-1294: Process cleanup is correct, but HLS segment files are not removed.The command properly terminates all processes, but doesn't clean up the HLS segment files in
public_dir, which can accumulate over time.As noted in previous reviews, add cleanup of
public_dircontents when stopping the server.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src-tauri/src/lib.rs(5 hunks)src/components/HlsScreenSharePage.jsx(1 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
src/components/HlsScreenSharePage.jsx
[error] 191-195: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 218-222: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 157-159: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 249-251: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 261-266: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 292-297: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 300-305: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 307-310: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🔇 Additional comments (9)
src/components/HlsScreenSharePage.jsx (2)
118-141: LGTM: Server lifecycle handlers are correctly implemented.The
startServerandstopServerfunctions properly invoke the corresponding Tauri commands, handle errors, and update state. The console logging for server start/stop events is appropriate.
13-20: LGTM: Component initialization is properly structured.The initial
useEffectcorrectly triggers the necessary checks on mount with an appropriate empty dependency array.src-tauri/src/lib.rs (7)
567-582: LGTM: Server state structures are well-designed.The
HlsServerStateandHlsServerHandlestructs appropriately model the HLS server lifecycle, with proper trait bounds (Clone for shared state, non-Clone for handles).
585-596: LGTM: FFmpeg availability check is correctly implemented.The command properly detects FFmpeg by attempting to run
ffmpeg -versionand gracefully handles errors.
599-726: Device enumeration is macOS-only but correctly implemented.The
list_ffmpeg_devicescommand correctly parses FFmpeg's device list on macOS and gracefully returns empty arrays on other platforms. Users should be aware that device selection is currently limited to macOS.Ensure the frontend UI handles empty device lists gracefully on Windows/Linux (it appears to, based on the conditional rendering in HlsScreenSharePage.jsx).
878-885: LGTM: Access code generation is appropriately implemented.The 6-character alphanumeric code provides adequate entropy (~2.1 billion combinations) for a temporary access mechanism, and uses a cryptographically secure RNG.
1184-1267: Server startup orchestration is well-structured.The
start_hls_server_cmdfunction properly coordinates FFmpeg, HTTP server, and tunnel startup with good error handling. The graceful handling of tunnel failure (lines 1228-1239) is particularly good, allowing localhost-only operation.
1297-1321: LGTM: Server info retrieval is correctly implemented.The
get_hls_server_infocommand properly returns server state or None, with conditional tunnel information.
1373-1375: LGTM: HLS server integration into app lifecycle is correct.The HLS server state is properly initialized and all commands are correctly registered in the invoke handler.
Also applies to: 1401-1405
| import { useState, useEffect } from 'react'; | ||
| import { invoke } from '@tauri-apps/api/core'; | ||
|
|
||
| export default function HlsScreenSharePage({ query }) { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove unused query prop.
The query prop is declared but never used in the component.
Apply this diff:
-export default function HlsScreenSharePage({ query }) {
+export default function HlsScreenSharePage() {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export default function HlsScreenSharePage({ query }) { | |
| export default function HlsScreenSharePage() { |
🤖 Prompt for AI Agents
In src/components/HlsScreenSharePage.jsx around line 4, the component declares
an unused prop "query"; remove "query" from the function parameter list (i.e.,
change the signature to not accept that prop), and also remove any related
PropTypes/defaultProps or unused imports referring to "query" in this file; if
any parent/components pass a "query" prop exclusively for this component, stop
passing it or consolidate usage there.
| const loadDevices = async () => { | ||
| console.log('🔍 Frontend: Starting to load devices...'); | ||
| try { | ||
| console.log('📞 Frontend: Calling list_ffmpeg_devices...'); | ||
| const deviceList = await invoke('list_ffmpeg_devices'); | ||
| console.log('✅ Frontend: Received device list:', deviceList); | ||
| console.log('📊 Frontend: Video devices:', deviceList.video); | ||
| console.log('📊 Frontend: Audio devices:', deviceList.audio); | ||
|
|
||
| setDevices(deviceList); | ||
|
|
||
| // Set default video device to index 2 if available | ||
| if (deviceList.video && deviceList.video.length > 0) { | ||
| console.log(`📹 Frontend: Found ${deviceList.video.length} video devices`); | ||
| const device2 = deviceList.video.find(d => d.index === 2); | ||
| if (device2) { | ||
| console.log('✅ Frontend: Setting default video device to index 2'); | ||
| setSelectedVideoDevice('2'); | ||
| } else if (deviceList.video.length > 0) { | ||
| console.log(`⚠️ Frontend: Device 2 not found, using first device: ${deviceList.video[0].index}`); | ||
| setSelectedVideoDevice(deviceList.video[0].index.toString()); | ||
| } | ||
| } else { | ||
| console.log('⚠️ Frontend: No video devices found'); | ||
| } | ||
|
|
||
| // Set default audio device to index 0 if available | ||
| if (deviceList.audio && deviceList.audio.length > 0) { | ||
| console.log(`🔊 Frontend: Found ${deviceList.audio.length} audio devices`); | ||
| const device0 = deviceList.audio.find(d => d.index === 0); | ||
| if (device0) { | ||
| console.log('✅ Frontend: Setting default audio device to index 0'); | ||
| setSelectedAudioDevice('0'); | ||
| } else if (deviceList.audio.length > 0) { | ||
| console.log(`⚠️ Frontend: Device 0 not found, using first device: ${deviceList.audio[0].index}`); | ||
| setSelectedAudioDevice(deviceList.audio[0].index.toString()); | ||
| } | ||
| } else { | ||
| console.log('⚠️ Frontend: No audio devices found'); | ||
| } | ||
|
|
||
| console.log('✅ Frontend: Device loading complete. Final state:', { | ||
| videoDevices: deviceList.video?.length || 0, | ||
| audioDevices: deviceList.audio?.length || 0, | ||
| selectedVideo: selectedVideoDevice, | ||
| selectedAudio: selectedAudioDevice | ||
| }); | ||
| } catch (err) { | ||
| console.error('❌ Frontend: Failed to load devices:', err); | ||
| console.error('❌ Frontend: Error details:', JSON.stringify(err, null, 2)); | ||
| // Don't show error, just use defaults | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider removing or conditionalizing verbose console logs.
The loadDevices function contains extensive console logging (lines 47, 49, 51-53, 59, 62, 65, 69, 74, 77, 80, 84, 87, 94-95). While useful for debugging during development, these should be removed or wrapped in a debug flag for production.
Consider replacing with a lightweight logger that can be disabled in production, or remove non-critical logs.
🤖 Prompt for AI Agents
In src/components/HlsScreenSharePage.jsx around lines 46 to 98, the loadDevices
function contains many noisy console.* calls used for development; remove or
wrap them behind a debug flag (e.g. process.env.NODE_ENV !== 'production' or a
simple isDebug constant) or replace with a lightweight logger (logger.debug) so
non-critical logs are disabled in production, while keeping only essential error
logs (console.error or logger.error) intact; ensure any retained debug logs use
the same conditional so they don’t run in production.
| console.log('✅ Frontend: Device loading complete. Final state:', { | ||
| videoDevices: deviceList.video?.length || 0, | ||
| audioDevices: deviceList.audio?.length || 0, | ||
| selectedVideo: selectedVideoDevice, | ||
| selectedAudio: selectedAudioDevice | ||
| }); |
There was a problem hiding this comment.
Logging captures stale state values.
The log statement at lines 87-92 reads selectedVideoDevice and selectedAudioDevice from state, but the preceding setSelectedVideoDevice and setSelectedAudioDevice calls haven't taken effect yet (React state updates are asynchronous). This will log the old values, not the newly selected ones.
If you want to log the updated values, log them directly from the local variables:
console.log('✅ Frontend: Device loading complete. Final state:', {
videoDevices: deviceList.video?.length || 0,
audioDevices: deviceList.audio?.length || 0,
- selectedVideo: selectedVideoDevice,
- selectedAudio: selectedAudioDevice
+ selectedVideo: deviceList.video.find(d => d.index === 2)?.index.toString() || deviceList.video[0]?.index.toString() || '2',
+ selectedAudio: deviceList.audio.find(d => d.index === 0)?.index.toString() || deviceList.audio[0]?.index.toString() || '0'
});Or remove the logging since the second useEffect (lines 23-31) already logs device state changes.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log('✅ Frontend: Device loading complete. Final state:', { | |
| videoDevices: deviceList.video?.length || 0, | |
| audioDevices: deviceList.audio?.length || 0, | |
| selectedVideo: selectedVideoDevice, | |
| selectedAudio: selectedAudioDevice | |
| }); | |
| console.log('✅ Frontend: Device loading complete. Final state:', { | |
| videoDevices: deviceList.video?.length || 0, | |
| audioDevices: deviceList.audio?.length || 0, | |
| selectedVideo: deviceList.video.find(d => d.index === 2)?.index.toString() || deviceList.video[0]?.index.toString() || '2', | |
| selectedAudio: deviceList.audio.find(d => d.index === 0)?.index.toString() || deviceList.audio[0]?.index.toString() || '0' | |
| }); |
🤖 Prompt for AI Agents
In src/components/HlsScreenSharePage.jsx around lines 87 to 92, the console.log
reads selectedVideoDevice and selectedAudioDevice from React state immediately
after calling setSelectedVideoDevice/setSelectedAudioDevice, so it prints stale
values; replace those state reads with the local variables that hold the new
selections (e.g., use the variables you just computed for the selected devices)
or remove the log entirely and rely on the existing useEffect that logs device
state changes.
| {(() => { | ||
| const shouldShow = devices.video.length > 0 || devices.audio.length > 0; | ||
| console.log('🎨 Frontend: Rendering device selection UI check:', { | ||
| shouldShow, | ||
| videoCount: devices.video.length, | ||
| audioCount: devices.audio.length, | ||
| devices: devices | ||
| }); | ||
| return shouldShow; | ||
| })() && ( |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Simplify conditional rendering logic.
The IIFE with console.log inside JSX render (lines 176-185) is unnecessarily complex and logs on every render.
Simplify to a direct boolean expression:
-{(() => {
- const shouldShow = devices.video.length > 0 || devices.audio.length > 0;
- console.log('🎨 Frontend: Rendering device selection UI check:', {
- shouldShow,
- videoCount: devices.video.length,
- audioCount: devices.audio.length,
- devices: devices
- });
- return shouldShow;
-})() && (
+{(devices.video.length > 0 || devices.audio.length > 0) && (The device state is already logged in the useEffect at lines 23-31.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {(() => { | |
| const shouldShow = devices.video.length > 0 || devices.audio.length > 0; | |
| console.log('🎨 Frontend: Rendering device selection UI check:', { | |
| shouldShow, | |
| videoCount: devices.video.length, | |
| audioCount: devices.audio.length, | |
| devices: devices | |
| }); | |
| return shouldShow; | |
| })() && ( | |
| {(devices.video.length > 0 || devices.audio.length > 0) && ( |
🤖 Prompt for AI Agents
In src/components/HlsScreenSharePage.jsx around lines 176 to 185 replace the
IIFE that computes shouldShow and logs to console with a direct boolean
expression; remove the console.log and the immediately-invoked function wrapper
and instead use (devices.video.length > 0 || devices.audio.length > 0) && (...)
for conditional rendering, relying on the existing device logging in the
useEffect at lines 23-31.
| <label style={{ display: 'block', marginBottom: '5px', fontSize: '14px' }}> | ||
| Video Device: | ||
| </label> | ||
| <select | ||
| value={selectedVideoDevice} | ||
| onChange={(e) => setSelectedVideoDevice(e.target.value)} | ||
| style={{ | ||
| width: '100%', | ||
| padding: '8px', | ||
| borderRadius: '4px', | ||
| backgroundColor: 'rgba(255,255,255,0.1)', | ||
| color: 'white', | ||
| border: '1px solid rgba(255,255,255,0.2)', | ||
| fontSize: '14px' | ||
| }} | ||
| > | ||
| {devices.video.map((device) => ( | ||
| <option key={device.index} value={device.index.toString()}> | ||
| [{device.index}] {device.name} | ||
| </option> | ||
| ))} | ||
| </select> | ||
| </div> |
There was a problem hiding this comment.
Associate labels with form controls for accessibility.
The label elements at lines 191-195 and 218-222 are not explicitly associated with their corresponding select elements, creating an accessibility barrier for screen readers.
Add htmlFor attributes to labels and id attributes to selects:
-<label style={{ display: 'block', marginBottom: '5px', fontSize: '14px' }}>
+<label htmlFor="video-device-select" style={{ display: 'block', marginBottom: '5px', fontSize: '14px' }}>
Video Device:
</label>
<select
+ id="video-device-select"
value={selectedVideoDevice}
onChange={(e) => setSelectedVideoDevice(e.target.value)}Apply the same pattern to the audio device label and select (use id="audio-device-select").
Based on static analysis hints.
Also applies to: 218-240
🧰 Tools
🪛 Biome (2.1.2)
[error] 191-195: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
🤖 Prompt for AI Agents
In src/components/HlsScreenSharePage.jsx around lines 191 to 213, the <label>
for the video device is not associated with its <select>, and similarly the
audio label/select at lines ~218-240 lack association; add an id attribute to
the video <select> (e.g., id="video-device-select") and set the corresponding
label's htmlFor="video-device-select", then do the same for the audio select
using id="audio-device-select" and htmlFor="audio-device-select"; ensure ids are
unique and update any related references if present.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (11)
src-tauri/src/lib.rs (5)
732-747: Duplicate: Unused command should be removed or utilized.The
check_localtunnelcommand remains unused in the frontend. This is dead code that increases maintenance burden.Either remove this command or integrate it into the frontend to verify localtunnel availability before starting the server.
1051-1056: Duplicate: Critical security vulnerability - API endpoint exposes access code.The
/api/infoendpoint returns the access code to any requester without authentication, completely bypassing access control. This allows anyone who discovers the server URL to retrieve the code and access the stream.Remove this endpoint entirely or protect it with authentication by requiring the access code in the request and using constant-time comparison.
1283-1283: Duplicate: Replace permissive CORS with restricted policy.
CorsLayer::permissive()allows requests from any origin, exposing the HLS server to CSRF attacks and unauthorized access. This is a critical security risk.Replace with a restrictive CORS policy that only allows localhost origins, or dynamically add the tunnel domain after it's established.
1319-1319: Duplicate: Hardcoded port can cause startup failure.Port 3000 is hardcoded without availability checking. If already in use, the server will fail to start with an unclear error.
Implement port selection with fallback (e.g., try ports 3000-3010) or accept a port parameter from the frontend.
1094-1097: Duplicate: Loading entire segment files into memory is inefficient.Segment files are read entirely into memory before sending to clients. For high-resolution streams, each segment can be several megabytes, causing memory pressure and latency.
Use streaming with
tokio::fs::Fileandtokio_util::io::ReaderStreamto avoid buffering entire files in memory.Also applies to: 1229-1232
src/components/HlsScreenSharePage.jsx (6)
4-4: Duplicate: Remove unusedqueryprop.The
queryprop is declared but never used in the component.Remove the unused prop:
-export default function HlsScreenSharePage({ query }) { +export default function HlsScreenSharePage() {
179-181: Duplicate: Add explicittype="button"to all buttons.All six button elements lack the explicit
typeattribute and will default totype="submit", potentially triggering unintended form submissions.Apply
type="button"to all buttons at lines 179, 271, 283, 323, 331, and 338.-<button onClick={checkFfmpeg} className="btn-secondary" style={{ marginTop: '10px' }}> +<button type="button" onClick={checkFfmpeg} className="btn-secondary" style={{ marginTop: '10px' }}>Also applies to: 271-273, 283-288, 323-328, 331-336, 338-341
Based on static analysis hints.
213-217: Duplicate: Associate labels with form controls for accessibility.The video and audio device labels are not explicitly associated with their select elements, creating an accessibility barrier for screen readers.
Add
htmlForto labels andidto selects:-<label style={{ display: 'block', marginBottom: '5px', fontSize: '14px' }}> +<label htmlFor="video-device-select" style={{ display: 'block', marginBottom: '5px', fontSize: '14px' }}> Video Device: </label> <select + id="video-device-select" value={selectedVideoDevice}Apply the same pattern to the audio label/select at lines 240-244 using
id="audio-device-select".Also applies to: 240-244
Based on static analysis hints.
47-99: Duplicate: Remove or conditionalize verbose console logs.The
loadDevicesfunction contains extensive console logging (lines 48, 50-54, 60, 63, 66, 70, 75, 78, 81, 85, 88-93, 95-96) useful for development but inappropriate for production.Wrap debug logs behind a condition:
const DEBUG = import.meta.env.DEV; if (DEBUG) console.log('🔍 Frontend: Starting to load devices...');Or remove non-critical logs entirely, keeping only error logs.
88-93: Duplicate: Console log captures stale state values.The log statement reads
selectedVideoDeviceandselectedAudioDevicefrom state immediately after calling the setters, which are asynchronous. This logs the old values, not the newly selected ones.Remove this log and rely on the existing
useEffectat lines 23-32 which already logs device state changes after updates take effect.
198-207: Duplicate: Simplify conditional rendering logic.The IIFE with console.log inside JSX (lines 198-207) is unnecessarily complex and logs on every render.
Replace with a direct boolean expression:
-{(() => { - const shouldShow = devices.video.length > 0 || devices.audio.length > 0; - console.log('🎨 Frontend: Rendering device selection UI check:', { - shouldShow, - videoCount: devices.video.length, - audioCount: devices.audio.length, - devices: devices - }); - return shouldShow; -})() && ( +{(devices.video.length > 0 || devices.audio.length > 0) && (
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src-tauri/src/lib.rs(5 hunks)src/components/HlsScreenSharePage.jsx(1 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
src/components/HlsScreenSharePage.jsx
[error] 213-217: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 240-244: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 179-181: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 271-273: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 283-288: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 323-328: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 331-336: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 338-341: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
| struct HlsServerState { | ||
| access_code: String, | ||
| port: u16, | ||
| public_dir: PathBuf, | ||
| viewers: Arc<Mutex<std::collections::HashMap<String, std::time::SystemTime>>>, // IP -> last seen | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider more precise typing for viewer tracking.
Using String for IP addresses and HashMap for viewers works but lacks type safety. Consider using std::net::IpAddr for IP addresses or a newtype wrapper to prevent misuse.
viewers: Arc<Mutex<std::collections::HashMap<std::net::IpAddr, std::time::SystemTime>>>,Then parse IPs properly in get_client_ip.
🤖 Prompt for AI Agents
In src-tauri/src/lib.rs around lines 568 to 573, the HlsServerState currently
types viewers as HashMap<String, SystemTime>, which is error-prone; change the
map key to std::net::IpAddr (or a newtype wrapper) so IPs are strongly typed,
update the viewers field signature accordingly, and then update get_client_ip to
parse incoming address strings into IpAddr (handling parse errors) before
inserting/looking up in the viewers map so all reader/writer code uses IpAddr
rather than raw String.
| async fn start_localtunnel(port: u16) -> anyhow::Result<(tokio::process::Child, String, String)> { | ||
| let mut cmd = Command::new("npx"); | ||
| cmd.args(&["-y", "localtunnel", "--port", &port.to_string()]); | ||
| cmd.stdout(Stdio::piped()); | ||
| cmd.stderr(Stdio::piped()); | ||
|
|
||
| let mut child = cmd.spawn()?; | ||
|
|
||
| // Wait a bit for localtunnel to start and output the URL | ||
| tokio::time::sleep(tokio::time::Duration::from_secs(3)).await; | ||
|
|
||
| use tokio::io::{AsyncBufReadExt, BufReader}; | ||
|
|
||
| // Helper function to extract URL and domain from a line | ||
| fn extract_url_and_domain(line: &str) -> Option<(String, String)> { | ||
| // Look for URL pattern: "https://xxx.loca.lt" anywhere in the line | ||
| if line.contains("https://") && line.contains(".loca.lt") { | ||
| if let Some(url_start) = line.find("https://") { | ||
| let url_part = &line[url_start..]; | ||
| // Find the end of the URL (space, newline, or end of string) | ||
| let url_end = url_part | ||
| .find(' ') | ||
| .or_else(|| url_part.find('\n')) | ||
| .or_else(|| url_part.find('\r')) | ||
| .unwrap_or(url_part.len()); | ||
|
|
||
| let url = url_part[..url_end].trim().to_string(); | ||
|
|
||
| // Extract domain (e.g., "xxx" from "https://xxx.loca.lt") | ||
| // URL format is "https://xxx.loca.lt" | ||
| if let Some(domain_start) = url.find("https://") { | ||
| let after_https = &url[domain_start + 8..]; // Skip "https://" | ||
| if let Some(domain_end) = after_https.find(".loca.lt") { | ||
| let domain = after_https[..domain_end].to_string(); | ||
| return Some((url, domain)); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| None | ||
| } | ||
|
|
||
| // Try to read from stderr first (localtunnel usually outputs to stderr) | ||
| let mut found_url = None; | ||
| let mut stderr_consumed = false; | ||
|
|
||
| if let Some(mut stderr) = child.stderr.take() { | ||
| let reader = BufReader::new(&mut stderr); | ||
| let mut lines = reader.lines(); | ||
|
|
||
| // Read lines for a few seconds to find the URL | ||
| let timeout = tokio::time::sleep(tokio::time::Duration::from_secs(8)); | ||
| tokio::pin!(timeout); | ||
|
|
||
| loop { | ||
| tokio::select! { | ||
| _ = &mut timeout => { | ||
| break; | ||
| } | ||
| line_result = lines.next_line() => { | ||
| match line_result { | ||
| Ok(Some(line)) => { | ||
| eprintln!("Localtunnel stderr: {}", line); | ||
| if let Some((url, domain)) = extract_url_and_domain(&line) { | ||
| found_url = Some((url, domain)); | ||
| stderr_consumed = true; | ||
| break; | ||
| } | ||
| } | ||
| Ok(None) => break, | ||
| Err(_) => break, | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Put stderr back if we haven't consumed it | ||
| if !stderr_consumed { | ||
| child.stderr = Some(stderr); | ||
| } | ||
| } | ||
|
|
||
| // If not found in stderr, try stdout | ||
| let mut stdout_consumed = false; | ||
| if found_url.is_none() { | ||
| if let Some(mut stdout) = child.stdout.take() { | ||
| let reader = BufReader::new(&mut stdout); | ||
| let mut lines = reader.lines(); | ||
|
|
||
| let timeout = tokio::time::sleep(tokio::time::Duration::from_secs(5)); | ||
| tokio::pin!(timeout); | ||
|
|
||
| loop { | ||
| tokio::select! { | ||
| _ = &mut timeout => { | ||
| break; | ||
| } | ||
| line_result = lines.next_line() => { | ||
| match line_result { | ||
| Ok(Some(line)) => { | ||
| eprintln!("Localtunnel stdout: {}", line); | ||
| if let Some((url, domain)) = extract_url_and_domain(&line) { | ||
| found_url = Some((url, domain)); | ||
| stdout_consumed = true; | ||
| break; | ||
| } | ||
| } | ||
| Ok(None) => break, | ||
| Err(_) => break, | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Put stdout back if we haven't consumed it | ||
| if !stdout_consumed { | ||
| child.stdout = Some(stdout); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if let Some((url, domain)) = found_url { | ||
| Ok((child, url, domain)) | ||
| } else { | ||
| // Wait a bit more and check if process is still running | ||
| tokio::time::sleep(tokio::time::Duration::from_secs(1)).await; | ||
| Err(anyhow::anyhow!("Could not parse localtunnel URL from output. Check if localtunnel is working correctly.")) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Complex URL parsing could be more robust.
The start_localtunnel function has intricate parsing logic for extracting the URL from stdout/stderr. While functional, it could fail silently if localtunnel changes its output format.
Consider adding a fallback that checks if the localtunnel process is healthy even if URL parsing fails, and log the raw output for debugging:
if found_url.is_none() {
eprintln!("⚠️ Could not parse URL. Raw output logged above.");
eprintln!("Process still running: {}", child.id().is_some());
}🤖 Prompt for AI Agents
In src-tauri/src/lib.rs around lines 750 to 878, the URL-extraction logic is
brittle and can fail silently if localtunnel changes its output; add a robust
fallback when found_url is None: read and log the remaining raw stdout/stderr
contents (so you have the full output for debugging), log a warning that URL
parsing failed, and log whether the child process is still running via
child.id().is_some() (or child.try_wait()) before returning the error; include
the raw output snippet in the returned anyhow::Error message or log so callers
can diagnose the issue.
| let ffmpeg_handle = start_ffmpeg(&public_dir, device_str) | ||
| .await | ||
| .map_err(|e| format!("Failed to start FFmpeg: {}", e))?; |
There was a problem hiding this comment.
No verification that FFmpeg produces HLS output.
After spawning FFmpeg, there's no check that it successfully generates HLS segments. If FFmpeg fails immediately (e.g., due to device permissions), the server appears to run but produces no stream.
Add a health check after spawning FFmpeg:
// After spawning FFmpeg, wait and verify output
tokio::time::sleep(tokio::time::Duration::from_secs(5)).await;
let m3u8_path = public_dir.join("stream.m3u8");
if !m3u8_path.exists() {
let _ = ffmpeg_handle.kill().await;
return Err("FFmpeg failed to generate HLS stream. Check device permissions and configuration.".to_string());
}🤖 Prompt for AI Agents
In src-tauri/src/lib.rs around lines 1330 to 1332, after starting FFmpeg with
start_ffmpeg(...).await, add a short wait (e.g., tokio::time::sleep for ~5s)
then check that public_dir/stream.m3u8 exists; if it does not, attempt to kill
the ffmpeg_handle (await the kill) and return an Err with a clear message like
"FFmpeg failed to generate HLS stream. Check device permissions and
configuration." to ensure the server fails fast when FFmpeg cannot produce HLS
output.
| const [selectedVideoDevice, setSelectedVideoDevice] = useState('2'); | ||
| const [selectedAudioDevice, setSelectedAudioDevice] = useState('0'); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Hardcoded default device indices may not match user hardware.
Default values '2' for video and '0' for audio match the backend defaults but are hardcoded and may not exist on all systems.
These defaults are already overridden by loadDevices on mount, so this is low-risk. However, consider initializing to null or '' and disabling the Start button until devices are loaded to avoid confusion:
const [selectedVideoDevice, setSelectedVideoDevice] = useState('');
const [selectedAudioDevice, setSelectedAudioDevice] = useState('');
// In render:
<button
type="button"
onClick={startServer}
disabled={!selectedVideoDevice || !selectedAudioDevice}
className="btn-primary"
>🤖 Prompt for AI Agents
In src/components/HlsScreenSharePage.jsx around lines 10-11, the component
initializes selectedVideoDevice and selectedAudioDevice with hardcoded indices
('2' and '0') which may not exist on all systems; change initialization to empty
string or null (e.g., ''), ensure loadDevices still sets the real defaults on
mount, and update the Start button to be disabled until both selectedVideoDevice
and selectedAudioDevice are populated to prevent starting with invalid devices.
| onClick={() => navigator.clipboard.writeText(serverInfo.code)} | ||
| className="btn-secondary" | ||
| style={{ marginTop: '10px' }} | ||
| > | ||
| 📋 Copy Code | ||
| </button> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Missing error handling for clipboard operations.
navigator.clipboard.writeText() can fail (e.g., permissions denied, HTTPS required) but errors are not handled, leaving users without feedback.
Add error handling to all clipboard operations:
-onClick={() => navigator.clipboard.writeText(serverInfo.code)}
+onClick={async () => {
+ try {
+ await navigator.clipboard.writeText(serverInfo.code);
+ } catch (err) {
+ setError(`Failed to copy: ${err.message}`);
+ }
+}}Apply to clipboard operations at lines 283, 323, and 331.
Also applies to: 323-328, 331-336
🧰 Tools
🪛 Biome (2.1.2)
[error] 283-288: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🤖 Prompt for AI Agents
In src/components/HlsScreenSharePage.jsx around lines 283-288 (and similarly at
323-328 and 331-336), the clipboard writeText calls lack error handling; update
each onClick handler to call navigator.clipboard.writeText(...).then(...) and
add a .catch(...) (or use async/try-catch) to handle rejected promises, showing
user-friendly feedback (e.g., set a state message or call an existing
notification function) on success and error, and optionally log the error for
debugging; ensure handlers gracefully handle unsupported clipboard APIs by
checking navigator.clipboard exists and providing fallback behavior or an
explanatory message.
1 participant
Summary by CodeRabbit
New Features
Style
Chores