Skip to content

fix: add audio stream#21

Merged
acerbetti merged 10 commits into
caplaz:mainfrom
DTse:fix/add-audio
Apr 19, 2026
Merged

fix: add audio stream#21
acerbetti merged 10 commits into
caplaz:mainfrom
DTse:fix/add-audio

Conversation

@DTse
Copy link
Copy Markdown
Contributor

@DTse DTse commented Apr 19, 2026

I've vibe coded a solution for adding audio. Main pain was some weirdness found with AAC format comming from either my camera or eufy-security-ws so we have to mux it to matroska to make it work.

I checked the code and the logic as far as my knowledge goes and I tried it with my Eufy camera(https://www.eufy.com/gr-en/products/t8400?variant=39620917592216) which seems to be working as expected.

Talkback I couldn't make it to work and I don't know if it's related to my camera, code issue or eufy-security-ws or eufy-security-client.

Any sugesstions are welcome

EDIT: Talkback works with the latests changes too
EDIT 2: added JMuxer to improve performance as it was dropping some frames

@acerbetti
Copy link
Copy Markdown
Member

Thank you for your contribution, can you please fix the PR removing the code that changed the version of the package to 0.2.2

@DTse
Copy link
Copy Markdown
Contributor Author

DTse commented Apr 19, 2026

Thank you for your contribution, can you please fix the PR removing the code that changed the version of the package to 0.2.2

Thank you, removed.

@acerbetti
Copy link
Copy Markdown
Member

Code Review

The Good

  • Zombie connection fix in cleanupStaleConnections is correct and overdue.
  • Waiting for TALKBACK_STARTED before sending audio is the right approach — matches the documented Eufy protocol behavior.
  • Falling back to raw video-only when no muxed port is available is a reasonable degradation path.
  • updateLivestreamStateForMuxerClients deliberately not tearing down on empty consumers is a smart call — the comment explaining the Rebroadcast cycling behavior is actually useful context.

Problems

1. Half the diff is a whitespace reformat — do not merge this.

Every package.json, tsconfig.json, and .vscode/settings.json in the repo was converted from 2-space to tab indentation. This is ~400 lines of pure noise that makes the actual changes impossible to review at a glance and will create merge conflicts with every branch that touches those files. Separate that out or revert it.

2. Intercom is added unconditionally to every camera.

// device-utils.ts
ScryptedInterface.Intercom,  // Added to all devices

Not every Eufy camera has a microphone or supports talkback. There is zero capability check here. Scrypted will show a "Talk" button on every camera including sensors and solo cameras that will just fail when pressed.

3. wrapAacInAdtsIfNeeded hardcodes the audio format.

const profile = 2;   // AAC-LC
const freqIndex = 8; // 16000 Hz — hardcoded
const channels = 1;  // mono — hardcoded

This assumes every Eufy camera streams 16 kHz mono AAC-LC. The metadata received in the audio event is ignored entirely when wrapping. If any camera uses 8 kHz (freqIndex 11) or 44.1 kHz (freqIndex 4), you'll produce a broken ADTS header and the muxer will silently ingest garbage audio.

4. as any scattered through event handler code.

remove = this.wsClient.addEventListener(
  eventType as any,
  (() => { ... }) as any,  // two casts on one call
  ...
);

If the event type system can't handle this without as any, the types are wrong — fix the types or add the missing overload. Type casts at API boundaries hide real bugs.

5. muxer: any in the stream map.

private muxerStreams = new Map<
  net.Socket,
  { muxer: any; duplex: Duplex }  // `any` on a central data structure
>();

JMuxer ships with TypeScript definitions. Use them.

6. getAudioPort() is deprecated before it ever shipped.

/**
 * @deprecated Audio is now muxed in-process via JMuxer. The dedicated
 * audio TCP server was removed. Kept for API compatibility; always
 * returns undefined.
 */
getAudioPort(): number | undefined {
  return undefined;
}

This method was added to the IStreamServer interface in this same PR (see types.ts). There is no prior version of this API in production to be compatible with. You've added a stub that exists solely to implement an interface you also just added, that always returns undefined. Delete both.

7. jmuxer appears in eufy-security-cli's lock entry.

The package-lock.json diff shows jmuxer added under the CLI package's resolved dependencies, but the CLI package.json doesn't declare it. This is lock file inconsistency — it suggests the lock file was generated in a dirty state. Run a clean npm install from a fresh checkout and regenerate the lock file before merging.

8. No tests for any of the intercom code.

startIntercom, stopIntercom, and waitForDeviceEvent have zero unit tests. The talkback flow has meaningful error paths (talkback start fails, FFmpeg exits early, double-start while active) that are entirely untested. The stream service tests were updated, but the device-level intercom logic was not.

9. Emojis in log messages.

this.logger.info(`🔊 Captured audio metadata: ...`);
this.logger.info(`🔀 Muxed client attached ...`);

The rest of the codebase doesn't use emojis in log messages. Pick a style and stick to it.

10. muxer.destroy?.() optional chain is a silent no-op signal.

If JMuxer doesn't have a destroy method, this does nothing on teardown and muxer state leaks. Either confirm the method exists (and type it properly — see point 5) or find the actual cleanup API.

11. isLivestreaming() has no error handling in startIntercom.

const status = await this.api.isLivestreaming();
if (!status.livestreaming) { ... }

If this throws (network error, device offline), the exception propagates with no cleanup. At minimum wrap it in a try/catch or let it propagate with a clear error message.


Summary

The core idea is sound and the implementation is mostly correct where it matters (event ordering, fallback path, livestream lifecycle). But this PR isn't ready: the whitespace reformat must be stripped out, the hardcoded audio assumptions need to use the actual metadata, the unconditional Intercom interface assignment needs a capability check, and the any types need to go. Fix those and add at least a few tests for the talkback flow.

@DTse
Copy link
Copy Markdown
Contributor Author

DTse commented Apr 19, 2026

Code Review

The Good

  • Zombie connection fix in cleanupStaleConnections is correct and overdue.
  • Waiting for TALKBACK_STARTED before sending audio is the right approach — matches the documented Eufy protocol behavior.
  • Falling back to raw video-only when no muxed port is available is a reasonable degradation path.
  • updateLivestreamStateForMuxerClients deliberately not tearing down on empty consumers is a smart call — the comment explaining the Rebroadcast cycling behavior is actually useful context.

Problems

1. Half the diff is a whitespace reformat — do not merge this.

Every package.json, tsconfig.json, and .vscode/settings.json in the repo was converted from 2-space to tab indentation. This is ~400 lines of pure noise that makes the actual changes impossible to review at a glance and will create merge conflicts with every branch that touches those files. Separate that out or revert it.

2. Intercom is added unconditionally to every camera.

// device-utils.ts
ScryptedInterface.Intercom,  // Added to all devices

Not every Eufy camera has a microphone or supports talkback. There is zero capability check here. Scrypted will show a "Talk" button on every camera including sensors and solo cameras that will just fail when pressed.

3. wrapAacInAdtsIfNeeded hardcodes the audio format.

const profile = 2;   // AAC-LC
const freqIndex = 8; // 16000 Hz — hardcoded
const channels = 1;  // mono — hardcoded

This assumes every Eufy camera streams 16 kHz mono AAC-LC. The metadata received in the audio event is ignored entirely when wrapping. If any camera uses 8 kHz (freqIndex 11) or 44.1 kHz (freqIndex 4), you'll produce a broken ADTS header and the muxer will silently ingest garbage audio.

4. as any scattered through event handler code.

remove = this.wsClient.addEventListener(
  eventType as any,
  (() => { ... }) as any,  // two casts on one call
  ...
);

If the event type system can't handle this without as any, the types are wrong — fix the types or add the missing overload. Type casts at API boundaries hide real bugs.

5. muxer: any in the stream map.

private muxerStreams = new Map<
  net.Socket,
  { muxer: any; duplex: Duplex }  // `any` on a central data structure
>();

JMuxer ships with TypeScript definitions. Use them.

6. getAudioPort() is deprecated before it ever shipped.

/**
 * @deprecated Audio is now muxed in-process via JMuxer. The dedicated
 * audio TCP server was removed. Kept for API compatibility; always
 * returns undefined.
 */
getAudioPort(): number | undefined {
  return undefined;
}

This method was added to the IStreamServer interface in this same PR (see types.ts). There is no prior version of this API in production to be compatible with. You've added a stub that exists solely to implement an interface you also just added, that always returns undefined. Delete both.

7. jmuxer appears in eufy-security-cli's lock entry.

The package-lock.json diff shows jmuxer added under the CLI package's resolved dependencies, but the CLI package.json doesn't declare it. This is lock file inconsistency — it suggests the lock file was generated in a dirty state. Run a clean npm install from a fresh checkout and regenerate the lock file before merging.

8. No tests for any of the intercom code.

startIntercom, stopIntercom, and waitForDeviceEvent have zero unit tests. The talkback flow has meaningful error paths (talkback start fails, FFmpeg exits early, double-start while active) that are entirely untested. The stream service tests were updated, but the device-level intercom logic was not.

9. Emojis in log messages.

this.logger.info(`🔊 Captured audio metadata: ...`);
this.logger.info(`🔀 Muxed client attached ...`);

The rest of the codebase doesn't use emojis in log messages. Pick a style and stick to it.

10. muxer.destroy?.() optional chain is a silent no-op signal.

If JMuxer doesn't have a destroy method, this does nothing on teardown and muxer state leaks. Either confirm the method exists (and type it properly — see point 5) or find the actual cleanup API.

11. isLivestreaming() has no error handling in startIntercom.

const status = await this.api.isLivestreaming();
if (!status.livestreaming) { ... }

If this throws (network error, device offline), the exception propagates with no cleanup. At minimum wrap it in a try/catch or let it propagate with a clear error message.

Summary

The core idea is sound and the implementation is mostly correct where it matters (event ordering, fallback path, livestream lifecycle). But this PR isn't ready: the whitespace reformat must be stripped out, the hardcoded audio assumptions need to use the actual metadata, the unconditional Intercom interface assignment needs a capability check, and the any types need to go. Fix those and add at least a few tests for the talkback flow.

thank you for the feedback. I hope I got all of the comments

@DTse
Copy link
Copy Markdown
Contributor Author

DTse commented Apr 19, 2026

@acerbetti
I hope you don't mind I added a prettier config for people like me using prettier to not mess with indentation

@acerbetti
Copy link
Copy Markdown
Member

100% agree, just added a script

npm run format

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants