meetecho / janus-gateway

Janus WebRTC Server
https://janus.conf.meetecho.com
GNU General Public License v3.0
8.15k stars 2.47k forks source link

[1.x] Streaming plugin does not send "started" event to event handlers #3335

Closed joshuaRosales closed 6 months ago

joshuaRosales commented 6 months ago

What version of Janus is this happening on? I'm currently using this version (from version.c):

#include "version.h"
const char *janus_build_git_sha = "40706c1e63c333ebc6f995829c3acbe45a7cc0d7";
const char *janus_build_git_time = "Mon Feb 26 19:13:13 UTC 2024";
int janus_version = 1103;
const char *janus_version_string = "1.1.3";
const char *libnice_version_string = "0.1.21";

I think the issue is here, which is in the latest version of the code. https://github.com/meetecho/janus-gateway/blob/b98e3bb91bd728ce21f6fd56519a303f2775f755/src/plugins/janus_streaming.c#L5606C1-L5615C41

Have you tested a more recent version of Janus too? No -- but the code region of interest is the same, it looks like.

Was this working before? I don't think so, but I haven't gone through all the past versions of Janus.

Additional context The problem is that the Janus Streaming Plugin doesn't send an event to event handlers for the "started" event, when a user is watching. It does give an event to the client, but not to eventhandlers.

Interestingly, the Streaming Plugin does send an event to the event handlers (and the client) for the "starting" event. https://github.com/meetecho/janus-gateway/blob/b98e3bb91bd728ce21f6fd56519a303f2775f755/src/plugins/janus_streaming.c#L6629C1-L6637C5

I was able to make it work by simply copy and pasting a snippet similar to the "starting" event but for "started":

    /* Also notify event handlers */
    if(notify_events && gateway->events_is_enabled()) {
        json_t *info = json_object();
        json_object_set_new(info, "status", json_string("started"));
        if(session->mountpoint != NULL)
            json_object_set_new(info, "id", string_ids ?
                json_string(session->mountpoint->id_str) :json_integer(session->mountpoint->id));
        gateway->notify_event(&janus_streaming_plugin, session->handle, info);
    }

at this line: https://github.com/meetecho/janus-gateway/blob/b98e3bb91bd728ce21f6fd56519a303f2775f755/src/plugins/janus_streaming.c#L5614

I'm not sure if this is the best solution, but it seems to work, and I can now know when the media stream has started.

Is it possible to update the code so event handlers will be notified for both "starting" and "started" (not just "starting", as it currently does)?

Thanks!

lminiero commented 6 months ago

I can't remember if there was a reason why we're not sending that event already, but where you added would be where I would have put it too. I'll have a quick look at the code, and push a commit if I don't seen problems.

lminiero commented 6 months ago

Done in the commit above. I also fine tuned the existing "starting" event, since it may have already "started" at that point (as the regular event already reflected).