mediasoup icon indicating copy to clipboard operation
mediasoup copied to clipboard

rust: Implement feature flag to disable worker build

Open bekriebel opened this issue 9 months ago • 5 comments

The mediasoup crate may need to be included to allow for using prelude or other defined types without the need to include the mediasoup-sys bindings to libmediasoup-worker.

Place the compilation of mediasoup-sys behind a feature flag that is enabled by defaault. When the feature is disabled, build speeds are dramatically increased and the build dependencies of the library are not needed in the environment.

Example use-case: In our project, we have the mediasoup-based SFU separate from our websocket signaling server. However, since we are passing messages that contain mediasoup types in our message structs, we need to include the mediasoup crate. The websocket server has no need for the libmediasoup C++ bindings, but without this change we still have to build it when the crate is included. Compiling with the worker feature disabled drastically reduces compile times, and also lets us build on CI platforms without the dependencies that are typically needed for the C++ compilation.

Care has been taken to ensure that the default build still works as expected, including the existing DOCS_RS environment variable setting that provides similar functionality during the docs build.

bekriebel avatar Mar 07 '25 05:03 bekriebel

Maybe it'd even make sense to have mediasoup-types crate that mediasoup crate would depend on and which would only contain reusable types.

What do you think?

I 200% agree with that. The whole rationale about this PR is to have types, just types.

ibc avatar Mar 07 '25 11:03 ibc

Maybe it'd even make sense to have mediasoup-types crate that mediasoup crate would depend on and which would only contain reusable types.

What do you think?

Totally agree.

jmillan avatar Mar 07 '25 11:03 jmillan

The use case is clear and makes sense to me overall, but I'd rather see the whole mediasoup-sys becoming optional rather than replacing its APIs with stubs. It is really confusing to have a huge API surface available just to fail in runtime, it doesn't feel Rust-y at all.

Maybe it'd even make sense to have mediasoup-types crate that mediasoup crate would depend on and which would only contain reusable types.

What do you think?

I agree that making mediasoup-sys completely option would be ideal. That was my first attempt, but there are a lot of dependencies in the mediasoup crate that point to the flatbuffers under the mediasoup_sys::fbs namespace.

I could probably make it so the entire pub struct Worker and below was gated behind the feature flag. That would keep the worker from being included at compile time and certainly be more "rusty". I think more than that would require a pretty significant refactor to move the FBS parts out of the mediasoup-sys crate. I'm not familiar enough with the codebase to take that part on right now myself.

As for making a mediasoup-types crate: That would certainly work for my use case. I'm not sure it is needed since, outside of the libmediasoup compilation, the rest of the build is pretty easy to manage. This is also something I wouldn't be able to do at this time, though.

bekriebel avatar Mar 07 '25 18:03 bekriebel

I think you can make mediasoup-types optionally depend on mediasoup-sys, which will allow you to move the types together with flatbuffers conversion impls.

I'm not sure it is needed since, outside of the libmediasoup compilation, the rest of the build is pretty easy to manage

Well, the problem is that it will leave a bunch of stuff that will be available to the user that compiled the crate with default-features = false, only to realize that none of that works in practice, which is confusing and generally a bad developer experience. It makes sense that it was easier to do things this way, but I don't think we're going to upstream current implementation. Looks like mediasoup-types is something everyone is aligned on even though it might be a little bit more work.

nazar-pc avatar Mar 07 '25 18:03 nazar-pc

It makes sense that it was easier to do things this way, but I don't think we're going to upstream current implementation. Looks like mediasoup-types is something everyone is aligned on even though it might be a little bit more work.

Fair enough. I was hoping to avoid maintaining a fork, but I think I'll have to for now. If anyone has another suggestion for an acceptable option, I'm happy to take a stab at it. However, the refactor needed for the full types crate isn't something I can do right now.

bekriebel avatar Mar 07 '25 19:03 bekriebel

Closing this PR in favour of https://github.com/versatica/mediasoup/pull/1572.

ibc avatar Jul 16 '25 10:07 ibc