Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notify speaking participant #2172

Merged
merged 1 commit into from
May 26, 2020
Merged

Notify speaking participant #2172

merged 1 commit into from
May 26, 2020

Conversation

maxboehm
Copy link
Contributor

This PR implements an addditional boolean for the function 'janus_videoroom_notify_participants'. It allows the caller to specify, whether also the source participant of the event will be notified about it.

We created this PR because we want a consistent and cpu-friendly (vs. javascript detection like hark) way to notify the participants of a videoroom who is speaking.

Right now, only other participants are notified who is speaking, but not the speakter itself. In order to notify also the speaker, we would have to implement a separate client-side logic which probably will not behave the exact same way as the janus logic.

Notify the speak itself, so we can reduce the javascript code on speaker side
@lminiero
Copy link
Member

Thanks! I'll look at this tomorrow.

@lminiero
Copy link
Member

Looks good, merging 👍

@lminiero lminiero merged commit acd414c into meetecho:master May 26, 2020
@lminiero
Copy link
Member

I applied the same change to the AudioBridge: now actually that plugin always passes TRUE to its notification function, but that's because we currently only use it for these talk events and for announcements starting/stopping. I'm keeping the flexible signature in case we come up with new events in the future that might exclude participants.

@maxboehm
Copy link
Contributor Author

Thank you, Lorenzo!

Just a few words for documentation:

  1. Started using the janus audio_level_event, but we somehow did not get it running (It was in the early days)
  2. Used hark on each client to listen to the stream of each participant. This had dramatic performance drawbacks as the number of participants (in our case) can be rather high (> 40)
  3. Used hark on each client analyzing his own audio stream and broadcasting the 'speak start' and 'speak stop' event via data channel. This led to some poor behaviour with latency (you hear and see the person talking, but the event of notification is much later) and still some performance issues (if you don't need to do some JS on a smartphone, just don't do it.)
  4. Now we are back to 1. (learning curve) but pushed a PR so that everyone gets notified in the same fashion, previously, the person emitting the speaking event was explictely skipped.

BTW. We are using following parameters for notification:

        audiolevel_event = true
        audio_active_packets = 15 # How often do we get notified
        audio_level_average = 70 # At which level do we get notified

We are still working on that parameters and I will keep you updated. Our goal is to have an immediate as possible feedback to the user in case if somebody is speaking as well if somebody has some echo or random noise issues.

@srisub1
Copy link

srisub1 commented Jul 5, 2020

Thanks for implementing this! But how do i use it in Javascript. how do i add this one config (whats the name).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants