Janus.js handling of track muting/unmuting causes video flashing in Chrome

[ghost]

In janus.js, a media track's mute/unmute behavior is handled as follows:

event.track.onended = function(ev) {
    Janus.log("Remote track muted/removed:", ev);
    if(config.remoteStream) {
        config.remoteStream.removeTrack(ev.target);
        pluginHandle.onremotestream(config.remoteStream);
    }
};
event.track.onmute = event.track.onended;
event.track.onunmute = function(ev) {
    Janus.log("Remote track flowing again:", ev);
    try {
        config.remoteStream.addTrack(ev.target);
        pluginHandle.onremotestream(config.remoteStream);
    } catch(e) {
        Janus.error(e);
    };
};

Chrome signals mute and unmute events whenever video is temporarily suspended, such as when packet loss is occurring. The code above causes the video at the receiver to flash--disappear when the track is removed by onmute, and then re-appear when added back by onunmute. When high packet loss occurs, flashing happens every few seconds.

Firefox does not exhibit this behavior, because it does not raise the mute and unmute events. (Instead, the video just freezes.)

Also, the problem occurs in Chrome with any videoroom settings--it doesn't matter whether transport_wide_cc_ext is enabled or disabled, whether simulcast is turned on or off, etc.

This is straightforward to duplicate in the janus demo room. (We're using clumsy to inject packet loss.)

I changed onmute to use a 5-sec timeout before removing the track (and made corresponding changes in onunmute), and the problem went away.

[reedspool]

Just came here looking for/to report this issue. Great timing :-D Thanks for your work @schoolcoder. I want to confirm what I'm seeing with your description of "flash": The video freezes and changes width and height. Is that what you are seeing? Want to confirm my issue is the same. Will try your remediation now and report.

[ghost]

@reedspool, we are not seeing freezing or size changing. The video track disappears, because it is removed from the media stream by the onmute handler. So our <video> element, where the media stream is displayed, has nothing to show until onunmute is called.

You can see the same effect in the Janus videoroom demo. When video disappears, it displays a message "no remote video source". Then, when onunmute gets called, the video reappears.

[lminiero]

Already discussed several times on the forum, where this belongs. We won't change the code, it's up to you to ignore the events in your web application if you want.

[ghost]

@lminiero, I apologize if this is the wrong forum to discuss bugs in janus.js, but the code in janus.js is simply incorrect.

That's fine if you don't want to fix it, but short of removing the onmute and onunmute event handlers added by janus (which is, to put it politely, a hack), there's no way to ignore it in the web application.

The bug also causes the videoroom demo to showcase Janus functionality poorly relative to other SFUs.

[lminiero]

Just ignore onremotemedia calls where the track has no video, if you had video before, and the effect will be the same as your hack.

[lminiero]

And I don't think honouring events triggered by the browser can be called a "bug". So again, hit the forum if you want to discuss that.

[lminiero]

@schoolcoder To clarify (at my laptop now, so I can elaborate more than I could on the phone), the reason why the event is important is that we need to account for scenarios where renegotiations can actually take video away. If your peer renegotiated his PeerConnection to remove video, or you chose to renegotiate and reject incoming video, this will trigger the onmute callback too, and you DO want it to be called, otherwise the effect will be a frozen video forever. That's why I'm opposed to remove those callbacks as many have suggested: I have to take into account ALL ways Janus can be used, not the few use cases people care about for their own applications.

That said, quoting the last part of your opening message:

I changed onmute to use a 5-sec timeout before removing the track (and made corresponding changes in onunmute), and the problem went away.

Thinking about it, a timed approach does seem like a good compromise: that said, 5 seconds are completely out of the question, for me. If you guys can come up with a reasonable trade-off (possibily coming from empirical evaluations of how long the issue typically happens when not warranted by a renegotiation), we can find a good value for that. Since you actually have a framework in place already in your setup, if you can share a pull request that would be great too :+1: (especially considering this is, to be honest, very low priority for me so I won't do it myself soon).

Reopening with an enhancement tag so that we can take it from there.

[brailateo]

I changed onmute to use a 5-sec timeout before removing the track (and made corresponding changes in onunmute), and the problem went away.

@schoolcoder , it would be fine to give us all the changes, in order to test for our environment! Thanks,

[reedspool]

I believe I executed @schoolcoder's description (let me know if not). My changes begin with > and start on line 1779 of my janus.js.

I also added the mute/unmute event to the onremotestream callback to give more fine-grained control to the application, a pattern I suggest highly for these cases where multi-shot callbacks are used for multiple scenarios.

1779,1780c1779,1785
<             config.remoteStream.removeTrack(ev.target);
<             pluginHandle.onremotestream(config.remoteStream);
---
>             clearTimeout(window.janusHackTimeoutId);
>             window.janusHackTimeoutId = setTimeout(
>               () =>
>               {
>                 config.remoteStream.removeTrack(ev.target);
>               }, 10 * 1000);
>             pluginHandle.onremotestream(config.remoteStream, ev);
1784a1790
>           clearTimeout(window.janusHackTimeoutId);
1788c1794
<             pluginHandle.onremotestream(config.remoteStream);
---
>             pluginHandle.onremotestream(config.remoteStream, ev);

[lminiero]

Planning to merge the PR above soon, so make sure you test before that happens.

[lminiero]

You gotta love how people are so quick to complain, but never as much to provide feedback when we get to it. I guess we'll just merge then.