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

Mute/unmute event on subscription not fired when muting audio #1037

Open
philipgiuliani opened this issue Jul 8, 2021 · 14 comments
Open

Mute/unmute event on subscription not fired when muting audio #1037

philipgiuliani opened this issue Jul 8, 2021 · 14 comments

Comments

@philipgiuliani
Copy link
Contributor

For debugging purposes, I have currently added the following subscriptions:

subscription.addEventListener("mute", console.debug);
subscription.addEventListener("unmute", console.debug);

subscription.stream.getAudioTracks().forEach((t) => {
    t.addEventListener("mute", console.debug);
    t.addEventListener("unmute", console.debug);
})

subscription.stream.getVideoTracks().forEach((t) => {
    t.addEventListener("mute", console.debug);
    t.addEventListener("unmute", console.debug);
})

When muting the audio channel, the subscription's event listener is never fired. We have also tried to mute the stream with the OWT Management API. We always have the same result. Muting video works, muting audio doesn't.

// subscription's listeners are called
this.publication.mute(Owt.Base.TrackKind.VIDEO);

// subscription's listeners are not called
this.publication.mute(Owt.Base.TrackKind.AUDIO);
@starwarfan
Copy link
Collaborator

Hello,
which branch has this issue?
On 5.0/4.3 branch, we've tried
subscription.addEventListener("mute", console.debug); subscription.addEventListener("unmute", console.debug);
to add event listener, use
subscription.mute(trackKind) subscription.unmute(trackKind)
to mute/unmute a subscription. There're events in this scenario.

@philipgiuliani
Copy link
Contributor Author

Hi @starwarfan,
we are working on OWT 5.0.

I see that you are calling subscription.mute(trackKind). We have expected that publication.mute(trackKind) would call the mute event on all subscriptions. Its actually also working like this when muting the Video-Track.

We would use the feature to show other participants that this stream has muted its audio.

@starwarfan
Copy link
Collaborator

Hi @philipgiuliani,
I tried publication.mute(trackKind) for audio, and I see mute/unmute event triggered on related subscriptions.
I used 2 web client, each publishes 1 stream with audio & video, and subscribes all the forwarded streams (2).

I found an issue which will cause duplicate events (open-webrtc-toolkit/owt-client-javascript#514). But I did not see this missing event issue. It will be helpful if you provide detail steps to produce it.

@philipgiuliani
Copy link
Contributor Author

Hi @starwarfan ,
did you test it with Conference or P2P mode? We are using Conference mode in our environment. I will try to make an open source example project based on the owt-client-javascript's included example.

@philipgiuliani
Copy link
Contributor Author

I have added the neccessary events to the JS demo applications: philipgiuliani/owt-client-javascript@80e0130

It's just an ugly, quick way to demonstrate the problem. If any of the users call publicationGlobal.mute("audio") or publicationGlobal.mute("video") in the console, no listener will be called. I have connected the demo to our own OWT Conference server.

Contrary to my first assumption, the mute event for the video is only called on the MediaStreamTrack.

Hope I could help!

@starwarfan
Copy link
Collaborator

starwarfan commented Jul 21, 2021

Hi, @philipgiuliani
I tried your demo and find the problem - OWT 5.0 JS SDK does not support subscribing same stream multiple times.
So if you want to get 'mute'/'unmute' event, you need avoid subscribing twice, and addEventListener for subscription in the subscribe callback. (https://github.com/philipgiuliani/owt-client-javascript/blob/80e013019513572e1428165025eebe67b3f11c60/src/samples/conference/public/scripts/index.js#L116) and (https://github.com/philipgiuliani/owt-client-javascript/blob/80e013019513572e1428165025eebe67b3f11c60/src/samples/conference/public/scripts/index.js#L123)

@philipgiuliani
Copy link
Contributor Author

Thats what I actually did in my local project but not in the provided example. The example only subscribes to your own stream and to the mixed stream. So my demo technically only has 1 subscription on all other users except yourself (as seen in the console by the logs).

@starwarfan
Copy link
Collaborator

starwarfan commented Jul 23, 2021

image
My experiment on 5.0 with modified index.js, with url parameters(?forward=true)(https://github.com/starwarfan/oms-client-javascript-1/blob/5.0-sample/src/samples/conference/public/scripts/index.js).

@philipgiuliani
Copy link
Contributor Author

Ok finally we have some progress! Thanks for your time. So I checked out your branch and was able to reproduce it locally. After copying your Owt.js file into my project it also worked without any problem.

I think it works because you used an older version of Owt.js. It doesn't seem to be the recent version of Owt 5.0 becuase the last commit in your 5.0-sample branch is from October 2019 🤔

Anyways, as soon as I switch to the latest release of Owt.js it stops working.

@philipgiuliani
Copy link
Contributor Author

Ok after testing one commit after another I was able to see that it doesn't work after this commit: open-webrtc-toolkit/owt-client-javascript@7d31677

With the previous commit the mute/unmute event still works 👍

@philipgiuliani
Copy link
Contributor Author

I did some debugging and the problem is that _audioTrackId and _videoTrackId is actually null here: open-webrtc-toolkit/owt-client-javascript@7d31677#diff-507006c8ac75c6cf7820f145d71c7ffb93510982a8ee134180a2d3df6ea2e090R1051

Screenshot 2021-07-23 at 14 10 43

@philipgiuliani
Copy link
Contributor Author

This patch will fix the issue, but I am not sure if checking just transport.id is enough.

diff --git a/src/sdk/conference/channel.js b/src/sdk/conference/channel.js
index 168df38..95f0ea1 100644
--- a/src/sdk/conference/channel.js
+++ b/src/sdk/conference/channel.js
@@ -1120,8 +1120,9 @@ export class ConferencePeerConnectionChannel extends EventDispatcher {
     if (this._publications.has(message.id)) {
       eventTargets.push(this._publications.get(message.id));
     }
-    for (const subscription of this._subscriptions) {
-      if (message.id === subscription._audioTrackId ||
+    for (const [_key, subscription] of this._subscriptions) {
+      if (message.id === subscription.transport.id ||
+          message.id === subscription._audioTrackId ||
           message.id === subscription._videoTrackId) {
         eventTargets.push(subscription);
       }

@starwarfan
Copy link
Collaborator

Hi,
Thanks for your patch. Because latest master branch can be unstable, I recommend that you can test on 5.0.x branch or a fixed commit.

@Phillipip
Copy link

Phillipip commented Oct 26, 2022

Could the patch help here too? open-webrtc-toolkit/owt-client-javascript#559
We don't actually have a problem with muting, but the activeaudioinputchange events are not transmitted via the websocket, so they cannot be triggered by the owt.js either.

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

No branches or pull requests

3 participants