feat: add minimal support for streaming plugin

[fcx-mrogez]

This PR add minimal support for the streaming plugin API.

We currently only support:

• create an RTP mountpoint LIVE,ONDEMAND or RTSP are not yet supported
• list available mountpoints
• destroy mountpoint

I tried to follow what was done in other plugins but since I'm no expert in rust, and janus API may be sometime complicate to grasp, some things may need some tweaking especially regarding streaming events.
Anyway, I hope you'll find it useful. I'm not sure I'll have more time to allocate to add more features to this plugin (we only use this subset for now), but I'll try to take into account reviews feedback. Thanks for your work on this useful library :)

[Ghamza-Jd]

Thanks a lot for your contribution! ⭐

I'll review it this evening

[fcx-mrogez]

Upon running some more tests I've encountered the following issue :

if I change the port to 5004 in the streaming example and run it, it will fail with a parsing error. This is unexpected. Given the default configuration of the streaming plugin (in the janus docker image) the port 5004 is already attributed to another mountpoint so it fails to create the mountpoint (as expected). But the returned json is :

    {
       "janus": "success",
       "session_id": 2158724686674557,
       "transaction": "nNbmsbj33zLY",
       "sender": 77797716144085,
       "plugindata": {
          "plugin": "janus.plugin.streaming",
          "data": {
             "streaming": "event",
             "error_code": 456,
             "error": "Can't add 'rtp' stream, error creating data source stream"
          }
       }
    }

The problem is the first line : "janus": "success", it should be "janus": "error" so that serde could properly deserialize the response... (see https://github.com/meetecho/janus-gateway/blob/master/src/plugins/janus_streaming.c#L5507)

To me, this looks like an upstream bug, but changing that upstream could be considered a breaking change and therefore not be accepted. What do you think ?

edit: Seems like videoroom is also affected https://github.com/meetecho/janus-gateway/blob/master/src/plugins/janus_videoroom.c#L8287

[Ghamza-Jd]

@fcx-mrogez Thanks for finding these out!

I'm thinking of creating a plugin response enum that would parse the error variant.

#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Deserialize)]
#[serde(untagged)]
pub enum PluginResponse<T> {
    Err(ErrorRsp),
    Ok(T),
}

#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Deserialize)]
pub struct ErrorRsp {
    pub error_code: u16,
    pub error: String,
}

Then fallible can parse it

pub async fn create_mountpoint_with_config(
    &self,
    options: StreamingCreateOptions,
    timeout: Duration,
) -> JaResult<MountpointCreatedRsp> {
    tracing::info!(plugin = "streaming", "Sending create");
    let mut message: Value = options.try_into()?;
    message["request"] = "create".into();

    let rsp = self
        .handle
        .send_waiton_rsp::<PluginResponse<MountpointCreatedRsp>>(message, timeout)
        .await?;
    match rsp {
        PluginResponse::Err(err) => Err(JaError::JanusTransport(JaTransportError::JanusError {
            code: err.error_code,
            reason: err.error,
        })),
        PluginResponse::Ok(ok) => Ok(ok),
    }
}

That's the fast answer (although JanusError will be incorrect as we could have same error code for a janus connection and a janus plugin, e.g. JANUS_STREAMING_ERROR_CANT_CREATE 456 for the streaming plugin and JANUS_ERROR_MISSING_MANDATORY_ELEMENT 456 for janus connection)

The long answer is were should we have the error, we could make a JaStreamingError that wraps JaError and custom errors (in this case only the error struct) or a common JaPluginError with JaPluginResult, or changing the upstream (not entirely sure about this approach since this is a plugin specific error so I prefer scoping it in the plugin crate or at the handle level)

I don't mind having a breaking change, since this crate is still in active development.

[Ghamza-Jd]

You don't have to worry about the error issue in this PR, we could tackle it in a separate PR, I'm happy to review it regardless of this issue since it's an existing issue

[Ghamza-Jd]

Really nice PR ⭐️
It just requires tests on parsing.
It also triggered some design decisions for the lib to make it more robust and more developer friendly, Thanks!

[Ghamza-Jd]

I've created a draft PR to tackle this issue, and it turns out changing the upstream solved 2 problems, propagating the error events and the error response.

#133

We can merge your PR first or mine, whichever you prefer!

[fcx-mrogez]

Thanks a lot for your review. I've forced pushed the updated branch with event parsing tests as requested.

[Ghamza-Jd]

Great work! ⭐️