M⚠️ ◾ WIP - system audio test by babakamyljanovssw · Pull Request #660 · SSWConsulting/SSW.YakShaver.Desktop

babakamyljanovssw · 2026-02-04T08:30:04Z

AI Development - Prompt & Model (include prompts/models used or N/A)

✏️

What triggered this change? (PBI link, Email Subject, conversation + reason, etc)

✏️

What was changed?

✏️

I paired or mob programmed with:

✏️

github-actions · 2026-02-04T08:30:17Z

PR Metrics

❌ Try to keep pull requests smaller than 400 lines of new product code by following the Single Responsibility Principle (SRP).
⚠️ Consider adding additional tests.

	Lines
Product Code	400
Test Code	-
Subtotal	400
Ignored Code	114
Total	514

Metrics computed by PR Metrics. Add it to your Azure DevOps and GitHub PRs!

package-lock.json

Copilot

Pull request overview

This is a work-in-progress PR that implements system audio capture for screen recordings using the native-audio-node package to replace the previous getDisplayMedia-based approach. The PR introduces a new backend service for managing system audio recording and modifies the frontend recording hook to receive and mix PCM audio data from the main process.

Changes:

Added system audio capture via native-audio-node package with a new SystemAudioService backend service
Replaced browser-based getDisplayMedia system audio with direct PCM audio streaming from the main process
Implemented a custom audio buffer and Web Audio API integration to mix system audio with microphone input

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 20 comments.

Show a summary per file

File	Description
`package.json`	Added `native-audio-node` (v0.3.5) and `electron-audio-loopback` (v1.0.6) dependencies
`package-lock.json`	Lock file updates for new dependencies, including platform-specific binaries
`src/ui/src/services/ipc-client.ts`	Added TypeScript definitions for new system audio IPC methods
`src/backend/preload.ts`	Implemented IPC bridge methods for system audio start/stop/status and event listeners
`src/backend/services/recording/system-audio-service.ts`	New service managing native system audio recording with permission handling and IPC communication
`src/backend/index.ts`	Removed old `setDisplayMediaRequestHandler` code; initialized new SystemAudioService
`src/ui/src/hooks/useScreenRecording.ts`	Refactored to use PCM audio buffer instead of MediaStream, added ScriptProcessorNode for audio mixing
`src/ui/src/hooks/useScreenRecording.old.ts`	Backup file of previous implementation (should not be committed)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-04T08:37:11Z

src/backend/index.ts

+  // System audio loopback is handled by electron-audio-loopback package
+  // The package registers IPC handlers: 'enable-loopback-audio' and 'disable-loopback-audio'
+  // The frontend must call enableLoopbackAudio() before getDisplayMedia() to capture system audio
+  // See: https://github.com/alectrocute/electron-audio-loopback


These comments reference electron-audio-loopback but the code is actually using native-audio-node. The comments should be updated to reflect the actual implementation, or removed if they're no longer accurate. The package electron-audio-loopback is added as a dependency in package.json but doesn't appear to be used anywhere in the code.

Suggested change

// System audio loopback is handled by electron-audio-loopback package

// The package registers IPC handlers: 'enable-loopback-audio' and 'disable-loopback-audio'

// The frontend must call enableLoopbackAudio() before getDisplayMedia() to capture system audio

// See: https://github.com/alectrocute/electron-audio-loopback

// System audio loopback for screen recording is handled by the current audio implementation

// (see the recording services and renderer recording logic for details).

// This block remains here to document that system audio capture is supported, but the exact

// package and IPC wiring may differ from earlier implementations.

Copilot · 2026-02-04T08:37:11Z

src/backend/services/recording/system-audio-service.ts

+    } catch (error) {
+      console.error("[SystemAudio] Failed to start:", error);
+      return {
+        success: false,
+        error: error instanceof Error ? error.message : String(error),
+      };
+    }


When the recorder fails to start, the service should clean up the recorder instance that was created. Currently, if recorder.start() throws an error after the recorder is created, this.recorder remains set to a non-null value, which could cause issues on subsequent start attempts.

Copilot · 2026-02-04T08:37:12Z

src/backend/services/recording/system-audio-service.ts

+        if (!granted) {
+          // Open system settings for user to grant permission
+          console.log("[SystemAudio] Opening system settings...");
+          openSystemSettings();
+          return {
+            success: false,
+            error: "System audio permission required. Please enable 'Screen & System Audio Recording' for this app in System Settings, then restart the app.",
+            needsPermission: true,
+          };


Opening system settings and immediately returning an error doesn't give users a chance to grant permission before the error is shown. Consider providing a better UX flow: either wait for the user to grant permission (if possible), or provide clearer instructions on what to do after granting permission (e.g., "Please restart the app after granting permission").

Copilot · 2026-02-04T08:37:12Z

src/ui/src/hooks/useScreenRecording.ts

@@ -1,24 +1,97 @@
-import { useCallback, useRef, useState } from "react";
+import { useCallback, useEffect, useRef, useState } from "react";


The PR description is incomplete - all placeholder sections (AI Development, What triggered this change, What was changed, Pairing information) are left as "✏️" placeholders. This makes it difficult to understand the purpose and scope of changes. Please fill in the PR template with proper information before merging.

package.json

Copilot · 2026-02-04T08:37:15Z

src/ui/src/hooks/useScreenRecording.ts

+    return () => {
+      cleanup();
+    };
+  }, [cleanup]);


The cleanup function in the useEffect hook creates a new closure each time the dependency changes, which could lead to stale closures being executed. Since cleanup is already in the dependency array, this effect will re-run whenever cleanup changes, potentially causing multiple cleanup calls. Consider removing this effect or ensuring it only runs on unmount by using an empty dependency array and calling cleanup directly.

Suggested change

}, [cleanup]);

}, []);

Copilot · 2026-02-04T08:37:15Z

src/backend/services/recording/system-audio-service.ts

+      };
+    });
+  }
+


The SystemAudioService registers IPC handlers in the constructor but never unregisters them. This could lead to memory leaks or duplicate handlers if the service is ever recreated. Consider implementing a cleanup/dispose method that removes the IPC handlers, or ensure the singleton instance is never recreated.

Suggested change

private unregisterHandlers() {

ipcMain.removeHandler(SYSTEM_AUDIO_START_CHANNEL);

ipcMain.removeHandler(SYSTEM_AUDIO_STOP_CHANNEL);

ipcMain.removeHandler(SYSTEM_AUDIO_STATUS_CHANNEL);

}

/**

* Clean up IPC handlers and internal state.

* Call this before discarding the service instance or on app shutdown.

*/

dispose(): void {

this.unregisterHandlers();

this.recorder = null;

this.mainWindow = null;

this.isRecording = false;

}

Copilot · 2026-02-04T08:37:15Z

src/ui/src/hooks/useScreenRecording.ts

+      const int16Data = new Int16Array(data);
+      float32Data = new Float32Array(int16Data.length);
+      for (let i = 0; i < int16Data.length; i++) {
+        float32Data[i] = int16Data[i] / 32768;


The conversion from Int16 to Float32 divides by 32768 instead of 32768.0. While JavaScript doesn't have separate integer and float division, this could be confusing. More importantly, the conversion doesn't handle the edge case where int16Data[i] equals -32768, which should map to -1.0 but will map to exactly -1.0 when divided by 32768. Consider using the standard conversion: int16Data[i] / (int16Data[i] < 0 ? 32768 : 32767) for proper symmetric range.

Suggested change

float32Data[i] = int16Data[i] / 32768;

float32Data[i] = int16Data[i] / (int16Data[i] < 0 ? 32768 : 32767);

Copilot · 2026-02-04T08:37:15Z

src/backend/services/recording/system-audio-service.ts

+      this.recorder.on("metadata", (metadata) => {
+        console.log("[SystemAudio] Metadata:", metadata);
+        if (this.mainWindow && !this.mainWindow.isDestroyed()) {
+          this.mainWindow.webContents.send("system-audio:metadata", metadata);


The metadata event uses a hardcoded channel name "system-audio:metadata" instead of using the SYSTEM_AUDIO_METADATA_CHANNEL constant that would be consistent with the other channels defined at the top of the file. This inconsistency could lead to bugs if the channel name needs to be changed.

Copilot · 2026-02-04T08:37:16Z

src/ui/src/hooks/useScreenRecording.ts

-      const audioContext = new AudioContext();
-
-      // Create source nodes for each audio stream
+      const audioContext = new AudioContext({ sampleRate: 48000 });


The AudioContext is created with a fixed sample rate of 48000 Hz, matching the SystemAudioRecorder configuration. However, there's no handling for the case where the actual system audio has a different sample rate, which could cause audio pitch/speed issues. Consider resampling the audio data if the sample rates don't match, or using the AudioContext's default sample rate and handling conversion in the buffer.

Suggested change

const audioContext = new AudioContext({ sampleRate: 48000 });

const audioContext = new AudioContext();

github-actions · 2026-02-04T09:00:47Z

🚀 Pre-release build is available for this PR:
https://github.com/SSWConsulting/SSW.YakShaver.Desktop/releases/tag/0.6.0-beta.660.1770195326

github-actions · 2026-02-04T09:39:41Z

🚀 Pre-release build is available for this PR:
https://github.com/SSWConsulting/SSW.YakShaver.Desktop/releases/tag/0.6.0-beta.660.1770197719

github-actions · 2026-02-04T09:54:01Z

🚀 Pre-release build is available for this PR:
https://github.com/SSWConsulting/SSW.YakShaver.Desktop/releases/tag/0.6.0-beta.660.1770198599

system audio test

16c02ec

Copilot AI review requested due to automatic review settings February 4, 2026 08:30

github-actions bot changed the title ~~WIP - system audio test~~ M⚠️ ◾ WIP - system audio test Feb 4, 2026

github-actions bot reviewed Feb 4, 2026

View reviewed changes

package-lock.json Show resolved Hide resolved

Copilot started reviewing on behalf of babakamyljanovssw February 4, 2026 08:30 View session

Copilot AI reviewed Feb 4, 2026

View reviewed changes

fix build

c833d70

github-actions bot changed the title ~~M⚠️ ◾ WIP - system audio test~~ S⚠️ ◾ WIP - system audio test Feb 4, 2026

fix system audio

c93e94c

github-actions bot changed the title ~~S⚠️ ◾ WIP - system audio test~~ M⚠️ ◾ WIP - system audio test Feb 4, 2026

simplify data serialize logic

4619683

github-actions bot changed the title ~~M⚠️ ◾ WIP - system audio test~~ S⚠️ ◾ WIP - system audio test Feb 4, 2026

use shared channels

7899afa

github-actions bot changed the title ~~S⚠️ ◾ WIP - system audio test~~ M⚠️ ◾ WIP - system audio test Feb 5, 2026

babakamyljanovssw closed this Feb 5, 2026

		@@ -1,24 +1,97 @@
		import { useCallback, useRef, useState } from "react";
		import { useCallback, useEffect, useRef, useState } from "react";

+  private unregisterHandlers() {
+    ipcMain.removeHandler(SYSTEM_AUDIO_START_CHANNEL);
+    ipcMain.removeHandler(SYSTEM_AUDIO_STOP_CHANNEL);
+    ipcMain.removeHandler(SYSTEM_AUDIO_STATUS_CHANNEL);
+  }
+  /**
+   * Clean up IPC handlers and internal state.
+   * Call this before discarding the service instance or on app shutdown.
+   */
+  dispose(): void {
+    this.unregisterHandlers();
+    this.recorder = null;
+    this.mainWindow = null;
+    this.isRecording = false;
+  }

	float32Data[i] = int16Data[i] / 32768;
	float32Data[i] = int16Data[i] / (int16Data[i] < 0 ? 32768 : 32767);

	const audioContext = new AudioContext({ sampleRate: 48000 });
	const audioContext = new AudioContext();

Conversation

babakamyljanovssw commented Feb 4, 2026

Uh oh!

github-actions bot commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Metrics

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 4, 2026

Uh oh!

github-actions bot commented Feb 4, 2026

Uh oh!

github-actions bot commented Feb 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

github-actions bot commented Feb 4, 2026 •

edited

Loading