Skip to content

Add opus encoder and improve logging for audio streams#125

Closed
chicco-carone wants to merge 3 commits intosnapcast:developfrom
chicco-carone:add-opus-encoder
Closed

Add opus encoder and improve logging for audio streams#125
chicco-carone wants to merge 3 commits intosnapcast:developfrom
chicco-carone:add-opus-encoder

Conversation

@chicco-carone
Copy link
Contributor

This pull request includes several improvements and feature additions to the snapstream.ts file, focusing on replacing console statements with a custom Logger class for better logging and adding support for Opus decoding.

Logging Improvements:

  • Replaced all console statements with the Logger class for improved logging consistency and control in AudioStream, TimeProvider, OpusDecoder, FlacDecoder, and SnapStream classes.

Opus Decoder Integration:

  • Added opus-decoder dependency to package.json for Opus decoding support.
  • Implemented OpusDecoder class with methods for initialization and decoding of Opus audio streams.

Code Cleanup:

  • Removed duplicate @types/node dependency from package.json.
  • Cleaned up commented-out console statements and replaced them with appropriate logger calls in FlacDecoder and TimeProvider classes.

Miscellaneous:

  • Added Logger class import and initialized logger instances in various classes to facilitate the new logging mechanism.
  • Replaced the majority of commented console.log statements with logger.debug statements so that if needed only the log level needs to be changed to show debug info.

@chicco-carone
Copy link
Contributor Author

Also I accidentally pressed the shortcut on vscode to format the code with prettier which made this pr completely unreadable so i reverted it but it could be useful to implement a linter process when pushing to git or at least running manually prettier on the whole codebase to make the code more cohesive and follow a standardized format.

@badaix
Copy link
Member

badaix commented Mar 31, 2025

Thanks! I think the pnpm-lock.yaml file is not needed, right? Can you please remove it from the PR?
The CI (npm ci) fails because the package-lock.json file is not updated, as also described on Stackoverflow.
Could you also please make the PR "opus only"? I like the idea of having some more sophisticated logging and there seem to be a lot of "ready to use" 3rd party loggers available, which I would like to have a look at, before using a hand-crafted logger.

@chicco-carone
Copy link
Contributor Author

chicco-carone commented Mar 31, 2025

Sorry for the pnpm thing. Since i use pnpm when I work on my project I forgot it. I'll also remove the logging thing.

Edit: btw you can take a look at pino to use as a logger since its pretty simple to implement and should handle well lots of logs when debugging audio streams

@chicco-carone
Copy link
Contributor Author

Moved to #129 to clean the pr

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