Skip to content

vlcrs-macros: change how capability trait works

Draft: depends on !5715 (merged) and !5716 (merged).

The previous design for the capability trait was working like this:

1/ User was defining a manifest including

module!{
    type: ModuleImplType (CapabilityTrait),
    ...
}

where CapabilityTrait was the general trait being implemented and exposing the module interface, and ModuleImplType the type representing the module implementation from the implementor.

2/ Core was defining the CapabilityTrait with a loader

trait CapabilityTrait {
    type Loader = ...;
    type Activate = ...;
    type Deactivate = ...;

    /* Rest of the required implementation for modules */
}

where Activate and Deactivate are the types for the module callbacks and Loader is a type providing those functions to setup the callbacks.

It was working well to define the manifest and setup the capability, since we could have monomorphized activation functions using the real type of the module implementation by typing the loader with Self.

unsafe extern "C"
fn activate_filter<T: CapabilityTrait>(
    obj: *mut vlc_filter_t,
    valid: &mut boo
) -> c_int {
    T::open(obj, valid);
    0
}

However, it made the virtual tables for operations impossible to write since they could not be monomorphized. Indeed, it is not possible to write the following because only one static/const variable exists for every generic types given:

fn activate<T: EncoderCapability>(obj: *mut encoder_t) {
    static ENCODER_OPS : vlc_encoder_operations = vlc_encoder_operations{
        encode_video: encode_video_impl::<T>
    };
    (*obj).sys = &ENCODER_OPS;
}

Instead, dynamic dispatch needs to be used to forward from the trait implementation directly, but it would mean writing:

fn encode_video_impl(encoder: *mut encoder_t) {
    let implementation = (*encoder).sys
        as *mut Box<dyn EncoderCapability<
            Loader=EncoderCapabilityLoader,
            Activate=vlc_activate_encoder,
            Deactivate=vlc_deactivate_encoder>>;
    (*implementation).encode_video(...);
} 

Which completely defeat the purpose of the associated default setup for the capability trait.

By summarizing the situation, we can find the following graph:

        VLC module loading  ____
                                \
                                 v
+------------+     (3)      +------------+
| Capability |<-------------|  Activate  |
| trait      |              |  Functions |
+------------+              +------------+
      ^       \                    |
      |        \   (2)             |
      | (1)     ---------.         | (4)
      |                   \        |
      |                    \       v
+------------+              +------------+
|   Loader   |     (5)      |   Module   |
|   Vtable   |------------->|   Impl     |
+------------+              +------------+
       ^
        \___ VLC module API

(1): p_sys, we don't know the real type
(2): implements the trait
(3): <T: CapabilityTrait>, uses the real type
(4): <T: CapabilityTrait>, uses the real type
(5): dyn CapabilityTrait, dynamic dispatch

This commit changes the definition of new capabilities and the plugin manifest entry, asking the user to provide the module loader instead of the capability trait.

module!{
    type: ModuleImplType (CapabilityTraitLoader),
    ...
}

It allows removing Activate, Deactivate and Loader from the interface exposed by the capability trait, removing uneeded details for the end users, and allowing to require the ModuleProtocol trait on those loaders, as well as getting a real type and not a trait in the module macros. It also removes the dependency to associated_type_defaults for code creating new capability as only vlcrs-plugin/vlcrs-core will depend on this feature.

This protocol has the defined associated types Activate and Deactivate moved towards the implementation of the trait rather than on the trait itself. In the end, users can now use dyn CapabilityTrait without specifying Loader, Activate and Deactivate types, effectively separating the VLC core interface from the exposed Rust API.

The implementation has been tested against the re-implementation of rav1e decoder in Rust, to check whether it was possible to write the virtual tables and activation functions correctly. Unfortunately, the lack of the correct type in the virtual tables means that we can only use the trait object, which require an additional level of boxing or additional complexity to have thiner trait objects, but that's not visible to the final plugin and can be changed later.

Merge request reports