Skip to content
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

Output sampleRate is always 24KHz #11

Closed
tevogyji opened this issue Mar 27, 2024 · 9 comments
Closed

Output sampleRate is always 24KHz #11

tevogyji opened this issue Mar 27, 2024 · 9 comments

Comments

@tevogyji
Copy link

I am trying to use wasm-media-encoders in an application where I am mixing streams. I can set the input sampleRate, but the output sampleRate is always 24KHz. This is fine for playback, as the decoder will use the encoded sampleRate. It causes big problems for live mixing, because the audio context sampleRate has already been set (to 44100). The underlying LAME library allows setting the output sampleRate, so hopefully this is a simple matter of adding this parameter to the wasm-media-encoders.configure() function and passing it on to lame_set_out_samplerate().

@tevogyji
Copy link
Author

It would be fine (for me at least) if src/wasm/lame/lame_enc.c: enc_init() would pass the same sample_rate value to both lame_set_in_samplerate() and lame_set_out_samplerate(). This shortcut will leave the top-level API unchanged, but would change the output.

@tevogyji
Copy link
Author

tevogyji commented Mar 27, 2024

I would like to make a 1-line change to wasm-media-encoders, but the list of prereq's is a mile long: VSCode, emscripten, clang, cmake, yarn, ... Especially since this project has been on ice for a while, and the toolchain has changed (yarn especially). I am not sure which version(s) of which tools are needed. Would it be possible to get a how-to on building wasm-media-encoders? I have all the C++ tools and I have used LAME in other projects, but wasm is new to me. Any pointers would be greatly appreciated. I am hoping you will read this and be motivated to look at the code again. Your version has come the closest to what I need. Oh so close, just one line of code away....

@arseneyr
Copy link
Owner

arseneyr commented Apr 4, 2024

Sorry for the slow response. I hadn't built the project in a while and had to get back up to speed.

I've simplified the building process and added some documentation.

I'll need a little more time to add the output sample rate change.

@tevogyji
Copy link
Author

tevogyji commented Apr 5, 2024

I was able to rebuild the project and make the change I needed, so no rush. It would be nice to have an alternate build method that removes docker has a requirement. I do all my work inside a vm, and launching a docker vm from within a vm is a lot of trouble. I have built lame in Windows, and I have used emscripten in Windows so it should be possible to rebuild the entire project in Windows. (I am currently using a Linux vm running side by side for the lame/emscriptem part.) Also, I replaced all the yarn/unpkg.com stuff with about 20 lines of javascript. Everything is working now, and even though I have moved on I want to thank you for publishing this project as it gave me a "known good" example to use as a starting point.

@andrenatal
Copy link

Hi @tevogyji. Would you mind contributing back your changes in order to change building system to support passing params to change the encoded sample rate?

And thanks for @arseneyr for this project. It works very well and seamless!

@tevogyji
Copy link
Author

@andrenatal : My build process diverged radically from @arseneyr ; I removed any dependency on VSCode, Yarn, Docker, and unpkg.com and build in Linux. This is not something I think Arseney would want back. Once I did this, the code changes were trivial. You would be better off nudging Arseney to make the api change. (Or figure it out; the build dependencies are the hardest part, and not very. It was my own hard requirement to build inside a vm that made it difficult.) I could provide the steps offline if necessary.

@arseneyr
Copy link
Owner

@andrenatal I'm working on adding the output sample rate changes to the LAME encoder. I'm using this opportunity to also upgrade the dependencies and Node version, so I'll hopefully get something by next week.

I removed any dependency on VSCode, Yarn, Docker, and unpkg.com and build in Linux

@tevogyji Do you have any suggestions for making it easier to build in Linux? I've tried to make the only hard requirements be emsdk and Node. Yarn is checked into source control so it doesn't need to be installed separately. VSCode and Docker are only tools to enable setting up a full dev environment as quickly as possible; building should still work if you install emsdk manually. I do regret hardcoding unpkg.com for the UMD build but I wouldn't consider it a dependency as you can just ignore createMp3Encoder/createOggEncoder and just use createEncoder with your own WASM binary. For the next version, I'll probably just publish two UMD files with the respective WASM inlined as a data64 URL.

BTW, you can use a Docker container without having to create a nested VM; it's just a way of bundling all the build tools into an image and namespacing the file system/processes.

I have built lame in Windows, and I have used emscripten in Windows so it should be possible to rebuild the entire project in Windows.

I actually develop on Windows myself but I've found it easier to just use a WSL2 VM for everything. I would accept a PR if anyone would like to make the Makefile cross-platform.

@arseneyr
Copy link
Owner

Better late than never, I've published 0.7.0 with this feature. Let me know if you have issues with this release.

@andrenatal
Copy link

andrenatal commented May 25, 2024 via email

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

No branches or pull requests

3 participants