-
Notifications
You must be signed in to change notification settings - Fork 1
feat: audio #104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: audio #104
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a comprehensive Audio Player node feature to the Electron-based flow editor, enabling users to manage and play audio files within their flows. The implementation spans the full stack from UI components to runtime execution, integrating file selection, preview capabilities, and playback controls.
Key Changes:
- Added
AudioPlayerruntime class with play/stop/loop functionality using theplay-soundlibrary - Created UI components for audio file management with preview playback in a modal editor
- Implemented secure IPC communication for file selection and reading with base64 encoding
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
yarn.lock |
Added play-sound and @types/play-sound dependencies with transitive dependency find-exec |
packages/runtime/src/audioplayer/audioplayer.types.ts |
Defined data schema for AudioPlayer node including audioFiles array, loop, and volume properties |
packages/runtime/src/audioplayer/audioplayer.ts |
Implemented AudioPlayer runtime class with play/stop methods and loop support |
packages/runtime/package.json |
Added runtime dependencies for audio playback functionality |
packages/runtime/index.ts |
Exported AudioPlayer class for use in the application |
apps/electron-app/src/render/components/react-flow/nodes/AudioPlayer/AudioPlayer.tsx |
Created React component for AudioPlayer node with playback status display and settings controls |
apps/electron-app/src/render/components/react-flow/nodes/AudioPlayer/AudioFileEditor.tsx |
Built modal dialog for managing audio files with add/remove/preview functionality |
apps/electron-app/src/preload.ts |
Extended IPC handler types to support audio file operations |
apps/electron-app/src/main/window.ts |
Reduced window recreation delay from 500ms to 250ms for improved UX |
apps/electron-app/src/main/ipc.ts |
Added IPC handlers for audio file selection and reading |
apps/electron-app/src/main/file.ts |
Implemented file dialog and file reading utilities for audio files |
apps/electron-app/src/common/nodes.ts |
Registered AudioPlayer in the node type registry |
apps/electron-app/index.html |
Updated CSP to allow media loading from blob and file sources |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| timeout = setTimeout(() => { | ||
| if (audio.readyState >= 2 && audio.paused) { | ||
| audio.play().catch(error => { | ||
| console.warn('Error playing audio (timeout fallback):', { error }); | ||
| cleanup(); | ||
| }); | ||
| } | ||
| }, 500); |
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a potential memory leak with the fallback timeout. If the audio starts playing before the timeout fires (at 500ms), or if an error occurs before the timeout, the timeout is never cleared. The timeout variable is scoped locally to the try block and cleanup doesn't have access to clear it. Consider moving the timeout to a broader scope or clearing it in all cleanup paths.
| {editedFiles.map((filePath, index) => ( | ||
| <Item variant='outline' key={index}> | ||
| <ItemContent> | ||
| <ItemTitle>{getFileName(filePath, index).fileName}</ItemTitle> | ||
| <ItemDescription>{getFileName(filePath, index).mimeType}</ItemDescription> | ||
| </ItemContent> | ||
| <ItemActions> | ||
| <Button | ||
| variant='outline' | ||
| size='sm' | ||
| onClick={() => handlePlayPause(index, filePath)} | ||
| > | ||
| {playingIndex === index ? <Icons.Pause size={16} /> : <Icons.Play size={16} />} | ||
| </Button> | ||
| <Button | ||
| variant='destructive' | ||
| size='sm' | ||
| onClick={() => { | ||
| setEditedFiles(prev => prev.filter((_, i) => i !== index)); | ||
| }} | ||
| > | ||
| <Icons.Trash2 size={16} /> | ||
| </Button> | ||
| </ItemActions> | ||
| </Item> | ||
| ))} |
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The getFileName function is called twice for each file in the map (once for ItemTitle and once for ItemDescription), which results in duplicate computation. Consider extracting the result once before the Item component to improve performance.
| {editedFiles.map((filePath, index) => ( | |
| <Item variant='outline' key={index}> | |
| <ItemContent> | |
| <ItemTitle>{getFileName(filePath, index).fileName}</ItemTitle> | |
| <ItemDescription>{getFileName(filePath, index).mimeType}</ItemDescription> | |
| </ItemContent> | |
| <ItemActions> | |
| <Button | |
| variant='outline' | |
| size='sm' | |
| onClick={() => handlePlayPause(index, filePath)} | |
| > | |
| {playingIndex === index ? <Icons.Pause size={16} /> : <Icons.Play size={16} />} | |
| </Button> | |
| <Button | |
| variant='destructive' | |
| size='sm' | |
| onClick={() => { | |
| setEditedFiles(prev => prev.filter((_, i) => i !== index)); | |
| }} | |
| > | |
| <Icons.Trash2 size={16} /> | |
| </Button> | |
| </ItemActions> | |
| </Item> | |
| ))} | |
| {editedFiles.map((filePath, index) => { | |
| const fileInfo = getFileName(filePath, index); | |
| return ( | |
| <Item variant='outline' key={index}> | |
| <ItemContent> | |
| <ItemTitle>{fileInfo.fileName}</ItemTitle> | |
| <ItemDescription>{fileInfo.mimeType}</ItemDescription> | |
| </ItemContent> | |
| <ItemActions> | |
| <Button | |
| variant='outline' | |
| size='sm' | |
| onClick={() => handlePlayPause(index, filePath)} | |
| > | |
| {playingIndex === index ? <Icons.Pause size={16} /> : <Icons.Play size={16} />} | |
| </Button> | |
| <Button | |
| variant='destructive' | |
| size='sm' | |
| onClick={() => { | |
| setEditedFiles(prev => prev.filter((_, i) => i !== index)); | |
| }} | |
| > | |
| <Icons.Trash2 size={16} /> | |
| </Button> | |
| </ItemActions> | |
| </Item> | |
| ); | |
| })} |
| } | ||
|
|
||
| play(index?: unknown) { | ||
| const filePath = this.data.audioFiles.at(Math.round(transformValueToNumber(index) - 1)); |
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The index calculation uses Math.round(transformValueToNumber(index) - 1) which could produce unexpected results. If the user passes 0, it becomes -1 (rounded) and .at(-1) returns the last element instead of nothing. If the user passes 0.4, it rounds to 0, then -1, again returning the last element. Consider using Math.floor or Math.trunc for more predictable array indexing behavior, or document that indices are 1-based.
| const filePath = this.data.audioFiles.at(Math.round(transformValueToNumber(index) - 1)); | |
| const idx = Math.floor(transformValueToNumber(index) - 1); | |
| if (idx < 0 || idx >= this.data.audioFiles.length) return; | |
| const filePath = this.data.audioFiles[idx]; |
| ipcMain.handle('ipc-read-audio-file', async (_event, filePath: string) => { | ||
| const buffer = await readAudioFile(filePath); | ||
| // Convert buffer to base64 for transmission | ||
| return buffer.toString('base64'); | ||
| }); |
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no validation on the filePath parameter passed from the IPC call. A malicious or buggy renderer process could potentially pass arbitrary file paths, including system files. Consider adding path validation to restrict access to only audio files or specific directories, or at minimum check if the file exists and has an allowed extension before reading it.
| const mimeTypeMap: Record<string, string> = { | ||
| mp3: 'MP3', | ||
| wav: 'WAV', | ||
| ogg: 'OGG', | ||
| m4a: 'M4A', | ||
| aac: 'AAC', | ||
| flac: 'FLAC', | ||
| }; | ||
| const mimeType = mimeTypeMap[extension] || extension.toUpperCase() || 'UNKNOWN'; | ||
|
|
||
| // Remove extension from fileName | ||
| const fileName = | ||
| extension && fileNameWithExt.endsWith(`.${extension}`) | ||
| ? fileNameWithExt.slice(0, -(extension.length + 1)) | ||
| : fileNameWithExt; | ||
|
|
||
| return { fileName, mimeType }; | ||
| } catch { | ||
| return { fileName: `Audio file ${index + 1}`, mimeType: 'UNKNOWN' }; | ||
| } | ||
| }; | ||
|
|
||
| const handlePlayPause = async (index: number, filePath: string) => { | ||
| // If clicking the same audio that's playing, pause it | ||
| if (playingIndex === index && audioRef.current) { | ||
| audioRef.current.pause(); | ||
| setPlayingIndex(null); | ||
| audioRef.current = null; | ||
| return; | ||
| } | ||
|
|
||
| // Stop any currently playing audio | ||
| if (audioRef.current) { | ||
| audioRef.current.pause(); | ||
| audioRef.current = null; | ||
| } | ||
|
|
||
| let blobUrl: string | null = null; | ||
|
|
||
| const cleanup = () => { | ||
| if (blobUrl) { | ||
| URL.revokeObjectURL(blobUrl); | ||
| blobUrl = null; | ||
| } | ||
| setPlayingIndex(null); | ||
| audioRef.current = null; | ||
| }; | ||
|
|
||
| try { | ||
| // Read file via IPC and create blob URL | ||
| const base64Data = await window.electron.ipcRenderer.invoke('ipc-read-audio-file', filePath); | ||
|
|
||
| // Convert base64 to binary | ||
| const binaryString = atob(base64Data); | ||
| const bytes = new Uint8Array(binaryString.length); | ||
| for (let i = 0; i < binaryString.length; i++) { | ||
| bytes[i] = binaryString.charCodeAt(i); | ||
| } | ||
|
|
||
| // Determine MIME type from file extension | ||
| const extension = filePath.split('.').pop()?.toLowerCase() || ''; | ||
| const mimeTypeMap: Record<string, string> = { | ||
| mp3: 'audio/mpeg', | ||
| wav: 'audio/wav', | ||
| ogg: 'audio/ogg', | ||
| m4a: 'audio/mp4', | ||
| aac: 'audio/aac', | ||
| flac: 'audio/flac', | ||
| }; |
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The duplicate MIME type mapping object definition (lines 46-53 and 107-114) should be extracted to a shared constant to avoid code duplication and potential inconsistencies. Consider defining it once at the module level or in a shared utility.
| play(index?: unknown) { | ||
| const filePath = this.data.audioFiles.at(Math.round(transformValueToNumber(index) - 1)); | ||
| if (!filePath) return; | ||
|
|
||
| this.playTrack(filePath); | ||
| } | ||
|
|
||
| private playTrack(filePath: string) { | ||
| if (!fs.existsSync(filePath)) return; | ||
|
|
||
| this.stop(); | ||
|
|
||
| this.value = true; | ||
| const currentPlayId = ++this.playId; | ||
|
|
||
| this.currentProcess = this.audioPlayer.play(filePath, err => { | ||
| if (currentPlayId === this.playId) this.value = false; | ||
| if (err) return; | ||
| if (!this.data.loop) return; | ||
| if (currentPlayId !== this.playId) return; | ||
|
|
||
| setTimeout(() => { | ||
| this.playTrack(filePath); | ||
| }, 0); | ||
| }); |
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The volume property is defined in the data schema but is never used in the AudioPlayer runtime implementation. The play-sound library used here doesn't support volume control. Consider either removing the volume property from the schema or using a different audio library that supports volume control (such as native Node.js audio solutions or a library like node-speaker with lame for MP3 decoding).
| import { Position } from '@xyflow/react'; | ||
| import { useState } from 'react'; | ||
| import { Handle } from '../../Handle'; | ||
| import { BaseNode, NodeContainer, useNodeControls, useNodeData, useNodeId } from '../Node'; |
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused import useNodeId.
| import { BaseNode, NodeContainer, useNodeControls, useNodeData, useNodeId } from '../Node'; | |
| import { BaseNode, NodeContainer, useNodeControls, useNodeData } from '../Node'; |
This pull request introduces a new "Audio Player" node to the Electron app, allowing users to select, manage, and play audio files from their device within flows. The implementation spans the UI, IPC communication, and runtime logic, including support for multiple audio formats and playback controls.
Audio Player Node Feature
AudioPlayernode, including UI components for displaying, managing, and playing audio files. Users can add, remove, and preview files directly from the node settings. (AudioPlayer.tsx,AudioFileEditor.tsx) [1] [2]AudioPlayernode into the app's node registry, enabling its use in flows. (nodes.ts) [1] [2]Electron IPC and File Handling
ipc.ts,preload.ts,file.ts) [1] [2] [3] [4] [5]Runtime Support
AudioPlayerclass and type definitions to the runtime package, supporting playback (with looping and volume control) using theplay-soundlibrary. (audioplayer.ts,audioplayer.types.ts,index.ts,package.json) [1] [2] [3] [4] [5]Security and UX Improvements
index.htmlCSP to allow media loading from local sources (blob, file).window.ts)