> Also, user doesn't have to think about versions as it includes only one
dependency so it won't try to plug webrtc endpoint that is incompatible
with engine.
FWIW, this can be easily addressed by recommending users to add
{:membrane_plugin, ">= 0.0.0"}, since membrane itself will already restrict
it to a supported version.
At the end of the day, if the goal is replacing 3 lines:
{:membrane_rtc_engine_webrtc, ..}
{:membrane_rtc_engine_hls, ...}
{:membrane_rtc_engine_recorder, ...}
with:
{:membrane_rtc_engine, "~> 0.10.0", features: [:webrtc, :hls, :recorder]}
I have to say this is likely not worth it, given the amount of work
implementing and maintaining this feature would entail in the long term.
I understand why Rust has this feature: compile-time and artefacts are much
larger there. Plus the fact they can't metaprogram at the file level like
us means they need explicit configuration for this. So before moving on, I
think you need to send a more structured proposal, with the problem this is
going to address, code snippets before and after, and so on. As mentioned
above, this is a complex feature, so the benefits need to be really well
justified over what can already be achieved today.
On Tue, Mar 7, 2023 at 5:06 PM Michal Śledź <michal.sl...@swmansion.com>
wrote:
> > I understand better now. To make the issue concise, you would like to
> programmatically include optional dependencies. Today everything is based
> on the dependency name itself but you would like to have an abstraction
> layer for controlling it.
>
> Yes, exactly!
>
> > One potential workaround that you have is to define dependencies to work
> as flags. Let's say you have a realtime feature that requires 3
> dependencies, you can define an optional "membrane_realtime_feature"
> package that, once included, signals the existance of a feature and also
> all of its dependencies. Alghouth it is most likely not a good idea to
> abuse dependencies in this way.
>
> That's what we were thinking about too. In general, we know in the compile
> time which features we are going to need. At the end, user has to plug
> specific endpoints to the Engine on its own. For example:
>
> {:ok, pid} = Membrane.RTC.Engine.start_link()
> Engine.add_endpoint(pid, %HLS{})
> Engine.add_endpoint(pid, %WebRTC{})
> Engine.add_endpoint(pid, %VideoRecorder{})
> etc.
>
> so in general, we could exctract each endpoint (WebRTC, HLS, Recorder)
> into a separate package and I belive this is a pretty elegant solution.
>
> We would end up with:
>
> {:membrane_rtc_engine, ...}
> {:membrane_rtc_engine_webrtc, ..}
> {:membrane_rtc_engine_hls, ...}
> {:membrane_rtc_engine_recorder, ...}
>
> However, allowing user to do
>
> {:membrane_rtc_engine, "~> 0.10.0", features: [:webrtc, :hls, :recorder]}
>
> looks even more attractive to me, and is easier to document as we can list
> all of supported features in the membrane_rtc_engine docs. Also, user
> doesn't have to think about versions as it includes only one dependency so
> it won't try to plug webrtc endpoint that is incompatible with engine.
>
> Regarding:
>
> > I think the biggest concern with implementing this feature is that it
> needs to be part of Hex itself. So the first discussion is if and how to
> extend the Hex registry to incorporate this metadata, which needs to happen
> in Hex first.
>
> and
>
> > In this case, as long as the feature checks are only inclusive, it
> should be fine. But you can also think if a library named A assumes that
> dependency C is compiled without some flag, and library B assumes C is
> compiled with said flag, you will end up with conflicting behaviour.
>
> I don't have answers to those questions but I am willing to investigate
> them and propose more detailed analysis on how we could implement the whole
> concept assuming it sounds valid to you.
>
> wtorek, 7 marca 2023 o 16:34:50 UTC+1 José Valim napisał(a):
>
>> I understand better now. To make the issue concise, you would like to
>> programmatically include optional dependencies. Today everything is based
>> on the dependency name itself but you would like to have an abstraction
>> layer for controlling it.
>>
>> I think the biggest concern with implementing this feature is that it
>> needs to be part of Hex itself. So the first discussion is if and how to
>> extend the Hex registry to incorporate this metadata, which needs to happen
>> in Hex first.
>>
>> > I also thought that configuring libraries via Application environment
>> is discouraged, according to
>>
>> https://hexdocs.pm/elixir/library-guidelines.html#avoid-application-configuration
>> <https://hexdocs.pm/elixir/library-guidelines.html#avoid-application-configuration>
>>
>> Right. Application configuration has many downsides, exactly because it
>> is global. The feature mechanism is also global, regardless if we put it on
>> mix.exs or on the configuration environment. Rust also hints it is
>> configuration (the conditional is called cfg):
>>
>> #[cfg(feature = "webp")]pub mod webp;
>>
>> In this case, as long as the feature checks are only inclusive, it should
>> be fine. But you can also think if a library named A assumes that
>> dependency C is compiled without some flag, and library B assumes C is
>> compiled with said flag, you will end up with conflicting behaviour.
>>
>> ---
>>
>> One potential workaround that you have is to define dependencies to work
>> as flags. Let's say you have a realtime feature that requires 3
>> dependencies, you can define an optional "membrane_realtime_feature"
>> package that, once included, signals the existance of a feature and also
>> all of its dependencies. Alghouth it is most likely not a good idea to
>> abuse dependencies in this way.
>>
>> On Tue, Mar 7, 2023 at 4:17 PM Michal Śledź <michal...@swmansion.com>
>> wrote:
>>
>>> 2. Users of a library with optional dependencies have to include all
>>> optional dependencies in their mix.exs
>>> I meant that to enable one feature, user has to include a lot of
>>> optional dependencies, at least in our case.
>>>
>>> 3. Users might include bad varsions of optional dependencies
>>> Here, I meant that user has to exactly know dependency version that has
>>> to be included. In our case, when there is a lot of optional dependencies,
>>> it starts getting annoying to keep them up to date in the docs.
>>>
>>> Other than that, you should be able to provide this functionality using
>>> config/config.exs files and the Application.compile_env/2.
>>> But I cannot manipulate which deps should be downloaded and compiled
>>> using Application.compile_env, can I? I mean, user still has to include all
>>> needed dependencies and know their correct versions. I also thought that
>>> configuring libraries via Application environment is discouraged, according
>>> to
>>>
>>> https://hexdocs.pm/elixir/library-guidelines.html#avoid-application-configuration
>>> <https://hexdocs.pm/elixir/library-guidelines.html#avoid-application-configuration>
>>>
>>> We very often depend on native libraries written in C like ffmpeg. When
>>> it's possible, we make those components optional, so that user is not
>>> forced to install uneeded native libraries on their system.
>>>
>>> I feel like at the moment user has to be aware of which optional deps
>>> are needed to get the desired feature. What I would like to have is to
>>> focus on the feature itself, leaving deps and their versions to library
>>> maintainers.
>>>
>>>
>>>
>>>
>>>
>>> wtorek, 7 marca 2023 o 14:45:03 UTC+1 José Valim napisał(a):
>>>
>>>> Hi Michał,
>>>>
>>>> Thanks for the proposal. Your initial description makes me think there
>>>> may exist bugs which we would need to investigate first.
>>>>
>>>> 2. Users of a library with optional dependencies have to include all
>>>> optional dependencies in their mix.exs
>>>>
>>>> This should not be required. You only need to include the dependencies
>>>> that you need, which would be equivalent to opting into a feature in Rust.
>>>>
>>>> 3. Users might include bad varsions of optional dependencies
>>>>
>>>> This should not be possible. The requirement has to match for optional
>>>> dependencies.
>>>>
>>>> If the above is not true, please provide more context.
>>>>
>>>> ---
>>>>
>>>> Other than that, you should be able to provide this functionality using
>>>> config/config.exs files and the Application.compile_env/2. In fact, I think
>>>> introducing another mechanism to configure libraries could end-up adding
>>>> more confusion, especially given how configs changed (and also improved)
>>>> throughout the years.
>>>>
>>>>
>>>>
>>>> On Tue, Mar 7, 2023 at 12:40 PM Michal Śledź <michal...@swmansion.com>
>>>> wrote:
>>>>
>>>>> Currently, using optional dependencies is quite inconvenient and error
>>>>> prone:
>>>>>
>>>>> 1. A lot of modules have to use if Code.ensure_loaded statements
>>>>> introducing additional nesting
>>>>> 2. Users of a library with optional dependencies have to include all
>>>>> optional dependencies in their mix.exs
>>>>> 3. Users might include bad varsions of optional dependencies
>>>>>
>>>>> My proposal is to enhance API for optional dependencies basing on the
>>>>> API provided by Cargo in Rust.
>>>>>
>>>>> The main idea is that the user of a library with optional dependencies
>>>>> specify which "features" it is willing to have. For example, in
>>>>> membrane_rtc_engine library, which allows you to exchange audio/video
>>>>> using
>>>>> different multimedia protocols, we have a lot of optional dependencies
>>>>> depending on what protocol the user is willing to use. When the user wants
>>>>> to receive media via webrtc and convert it to the HLS to broadcast it to
>>>>> the broader audience it has to include all of those dependencies
>>>>>
>>>>> # Optional deps for HLS endpoint
>>>>> {:membrane_aac_plugin, "~> 0.13.0", optional: true},
>>>>> {:membrane_opus_plugin, "~> 0.16.0", optional: true},
>>>>> {:membrane_aac_fdk_plugin, "~> 0.14.0", optional: true},
>>>>> {:membrane_generator_plugin, "~> 0.8.0", optional: true},
>>>>> {:membrane_realtimer_plugin, "~> 0.6.0", optional: true},
>>>>> {:membrane_audio_mix_plugin, "~> 0.12.0", optional: true},
>>>>> {:membrane_raw_audio_format, "~> 0.10.0", optional: true},
>>>>> {:membrane_h264_ffmpeg_plugin, "~> 0.25.2", optional: true},
>>>>> {:membrane_audio_filler_plugin, "~> 0.1.0", optional: true},
>>>>> {:membrane_video_compositor_plugin, "~> 0.2.1", optional: true},
>>>>> {:membrane_http_adaptive_stream_plugin, "~> 0.11.0", optional:
>>>>> true},
>>>>>
>>>>> Instead of this, I would love to say to the user, hi if you want to
>>>>> use HLS just specify it in the feature list. For example:
>>>>>
>>>>> {:membrane_rtc_engine, "~> 0.10.0", features: [:hls]}
>>>>>
>>>>> It would also be nice to somehow get rid of "if Code.ensure_loaded"
>>>>> statements. I am not sure how yet but Rust do this that way
>>>>>
>>>>> // This conditionally includes a module which implements WEBP support.
>>>>> #[cfg(feature
>>>>> = "webp")] pub mod webp;
>>>>>
>>>>> What comes to my mind is that in mix.exs we can specify "features",
>>>>> their dependencies and a list of modules. When someone asks for the
>>>>> feature, those dependencies are autmatically downloaded and listed modules
>>>>> are compiled.
>>>>>
>>>>> The final proposal is:
>>>>>
>>>>> # library side
>>>>> # mix.exs
>>>>>
>>>>> features: [
>>>>> hls: [
>>>>> dependencies: [],
>>>>> modules: []
>>>>> ]
>>>>> ]
>>>>>
>>>>> # user side
>>>>> # mix.exs
>>>>>
>>>>> {:membrane_rtc_engine, "~> 0.10.0", features: [:hls]}
>>>>>
>>>>> I would love to help in implementing those features if you decide they
>>>>> are valuable
>>>>>
>>>>> Rust reference:
>>>>> https://doc.rust-lang.org/cargo/reference/features.html#features
>>>>>
>>>>>
>>>>> --
>>>>> You received this message because you are subscribed to the Google
>>>>> Groups "elixir-lang-core" group.
>>>>> To unsubscribe from this group and stop receiving emails from it, send
>>>>> an email to elixir-lang-co...@googlegroups.com.
>>>>> To view this discussion on the web visit
>>>>> https://groups.google.com/d/msgid/elixir-lang-core/8385965e-799d-4cea-bcd5-151d9fee6914n%40googlegroups.com
>>>>> <https://groups.google.com/d/msgid/elixir-lang-core/8385965e-799d-4cea-bcd5-151d9fee6914n%40googlegroups.com?utm_medium=email&utm_source=footer>
>>>>> .
>>>>>
>>>> --
>>> You received this message because you are subscribed to the Google
>>> Groups "elixir-lang-core" group.
>>> To unsubscribe from this group and stop receiving emails from it, send
>>> an email to elixir-lang-co...@googlegroups.com.
>>>
>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/elixir-lang-core/d88a5141-86d8-42b8-ae61-71cf58d644a0n%40googlegroups.com
>>> <https://groups.google.com/d/msgid/elixir-lang-core/d88a5141-86d8-42b8-ae61-71cf58d644a0n%40googlegroups.com?utm_medium=email&utm_source=footer>
>>> .
>>>
>> --
> You received this message because you are subscribed to the Google Groups
> "elixir-lang-core" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to elixir-lang-core+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/elixir-lang-core/a0f6ba5c-1a50-4637-90c9-c3968b443377n%40googlegroups.com
> <https://groups.google.com/d/msgid/elixir-lang-core/a0f6ba5c-1a50-4637-90c9-c3968b443377n%40googlegroups.com?utm_medium=email&utm_source=footer>
> .
>
--
You received this message because you are subscribed to the Google Groups
"elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to elixir-lang-core+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4KaMgqoxWsKxSN4%2BdFhY5PatDHufoD2EoNuSM9noFZE7w%40mail.gmail.com.
