Implement external streaming setup and enhance stream controls

[carsonSgit]

• Added ExternalStreamSetup component to manage external streaming configurations, including RTMP and SRT setup instructions.
• Updated StreamPage to include tabbed navigation for selecting between browser and external streaming options.
• Enhanced StreamControls to support different stream types, adjusting UI elements based on the selected streaming method.
• Improved user experience by providing real-time feedback on stream status and connection details.

These changes aim to streamline the streaming process and provide users with clear instructions and controls for both browser and external streaming options.

[carsonSgit]

lgtm

[claude]

Code Review - PR #7: Implement external streaming setup and enhance stream controls

Summary

This PR adds comprehensive external streaming support (RTMP/SRT) to the application, allowing users to stream from external software like OBS Studio, FFmpeg, or other broadcasting tools. The implementation includes a new tabbed interface to switch between browser-based and external streaming modes.

---

Positive Aspects

Well-structured component organization:

• The new ExternalStreamSetup component is cleanly separated with clear responsibilities
• Good use of existing UI components (Card, Button, Input, etc.)
• Clean separation between browser and external streaming modes

Real-time status monitoring:

• Excellent use of Supabase Realtime to monitor stream status changes (components/stream/external-stream-setup.tsx:39-90)
• Proper toast notifications provide good user feedback on connection state changes
• Shows active protocol (RTMP/SRT) when streaming is active

Good UX considerations:

• Stream key is hidden by default (password input type) for security
• Copy-to-clipboard functionality with visual feedback
• Comprehensive setup instructions for popular streaming software
• Clear numbered steps for configuration

[claude]

Issues & Concerns

1. Security - Stream Key Exposure (HIGH PRIORITY)

Location: components/stream/external-stream-setup.tsx:185, 221

The stream key is rendered multiple times in password inputs, but it can still be copied to clipboard and is visible in the DOM. More importantly, there's no additional authentication or permission checks.

Recommendations:

• Consider adding user authentication checks before displaying stream keys
• Add rate limiting on the stream creation endpoint
• Consider implementing stream key rotation/regeneration functionality

---

2. Memory Leak - Missing Cleanup Dependency (MEDIUM PRIORITY)

Location: components/stream/external-stream-setup.tsx:91

The useEffect has streamStatus in the dependency array, which causes the subscription to be recreated every time the status changes. This can lead to multiple active subscriptions and memory leaks.

Recommendation: Remove streamStatus from the dependency array - it should only depend on streamId. The streamStatus is used inside the callback for comparison logic, not for subscription setup.

---

3. Error Handling - Unhandled Promise Rejection (MEDIUM PRIORITY)

Location: components/stream/external-stream-setup.tsx:75-84

The fetchInitialStatus async function is called but errors are silently ignored. If the database query fails, the UI won't show any indication of the problem.

Recommendation: Add proper error handling and consider showing a toast or error state to the user when the initial status fetch fails.

[claude]

4. Type Safety Issue (LOW PRIORITY)

Location: app/(dashboard)/stream/page.tsx:302

The tab type is cast directly without validation. Consider adding type guards or using const assertions for better type safety.

---

5. Potential Race Condition (MEDIUM PRIORITY)

Location: components/stream/external-stream-setup.tsx:61-63

Toast notifications could be shown multiple times if the component updates quickly. The condition checks if the new status is active and the current streamStatus is not active, but during rapid updates, this could trigger incorrectly.

Recommendation: Use a ref to track the last notified status to prevent duplicate notifications.

---

6. Accessibility Concerns (LOW PRIORITY)

Location: components/stream/external-stream-setup.tsx:144-146

The status indicators use color alone to convey information. Consider adding role and aria-label attributes for better accessibility for screen readers.

[claude]

Performance Considerations

Positive:

• Good use of React's conditional rendering to avoid rendering unnecessary components
• Proper memoization opportunities with the copy button component (already extracted)

Suggestions:

• Consider memoizing the getStatusIndicator function with useMemo to avoid recalculating on every render
• The CopyButton component could be memoized with React.memo since it's rendered multiple times

---

Test Coverage

Missing tests:

• No tests found for the new ExternalStreamSetup component
• No tests for the stream type switching logic in StreamPage
• No tests for the updated StreamControls with the new streamType prop

Recommendations:

• Add unit tests for the ExternalStreamSetup component, especially:
  • Realtime subscription behavior
  • Copy to clipboard functionality
  • Status indicator rendering based on different states
• Add integration tests for the tab switching between browser and external streaming
• Test edge cases like stream creation failures, missing stream keys, etc.

[claude]

Overall Assessment

This is a solid implementation that adds valuable functionality to the application. The code is well-structured and follows React best practices. However, there are some important issues that should be addressed:

Must fix before merge:

1. Memory leak in the Realtime subscription (dependency array issue) - components/stream/external-stream-setup.tsx:91
2. Error handling for the initial status fetch - components/stream/external-stream-setup.tsx:75-84

Should fix before merge:
3. Race condition in toast notifications - components/stream/external-stream-setup.tsx:61-63
4. Type safety for tab changes - app/(dashboard)/stream/page.tsx:302

Can be addressed in follow-up:
5. Test coverage
6. Accessibility improvements
7. Performance optimizations
8. Security enhancements (authentication/authorization)

The external streaming feature is well-designed with good UX, but addressing the technical issues above will make it production-ready. Great work overall!

---

Recommendation: Request changes - Address the memory leak and error handling issues, then this will be ready to merge.