Closed Bug 1545855 Opened 1 year ago Closed 3 months ago

Need to fire mute events when a RTCP BYE is received, or an RTCP timeout happens

Categories

(Core :: WebRTC: Audio/Video, defect, P3)

defect

Tracking

()

RESOLVED DUPLICATE of bug 1595479

People

(Reporter: bwc, Assigned: bwc)

References

Details

Attachments

(1 file)

I guess for timeouts we'd be following https://tools.ietf.org/html/rfc3550#section-6.3.5

Does webrtc.org give us an API that will tell us when either of these things happens?

^

Flags: needinfo?(na-g)
Flags: needinfo?(dminor)

Bugbug thinks this bug is a task, but please change it back in case of error.

Type: defect → task
Assignee: nobody → docfaraday
Type: task → defect

I can't find any callbacks on the WebRTC (call) interface for these events.

Flags: needinfo?(na-g)

It looks like Bye is handled internally by the rtp/rtcp stack and not propagated anywhere [1]. Similarly timeouts seem to only result in warnings [2]. I'm not super familiar with the rtp_rtcp module so maybe I've missed something, but it really doesn't look like anything is getting propagated back up to an interface that would let us register for a notification.

[1] https://searchfox.org/mozilla-central/rev/f46e2bf881d522a440b30cbf5cf8d76fc212eaf4/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc#670
[2] https://searchfox.org/mozilla-central/rev/f46e2bf881d522a440b30cbf5cf8d76fc212eaf4/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc#210

Flags: needinfo?(dminor)

Is there anything in the latest webrtc.org that would let us do this?

(In reply to Byron Campen [:bwc] from comment #7)

Is there anything in the latest webrtc.org that would let us do this?

The code I referenced above is more or less unchanged on tip of webrtc.org. Is this for a test that is passing on Chromium?

I do not know yet. Just trying to figure out whether we have a bug that we can mark this as blocking on.

Priority: -- → P2

Marking this leave-open, because I am adding tests that fail.

Keywords: leave-open

Need to rebase it looks like.

Pushed by bcampen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bd72af1908de
Modify a test to check that a mute event fires when the remote PC is closed, and a similar test for transceiver.stop() r=jib
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/16698 for changes under testing/web-platform/tests

The leave-open keyword is there and there is no activity for 6 months.
:bwc, maybe it's time to close this bug?

Flags: needinfo?(docfaraday)

Nah, this is still a problem. I will de-prioritize it though.

Flags: needinfo?(docfaraday)
Priority: P2 → P4

The mute event logic on the remote end is critical to get right. Without at least one browser implementing the spec, or I fear Chrome's broken mute behavior might end up becoming the de facto standard.

P4 means we have no plan to implement this, but will review patches. I'd say we want P3 at minimum. Any reason it shouldn't be P2?

Flags: needinfo?(docfaraday)
Priority: P4 → P3

Do we intend to make the necessary modification to webrtc.org to make this happen?

Flags: needinfo?(docfaraday)

webrtc.org does not expose RTCP BYE?

That might explain Chrome's punting on it. They implement some logic where they fire mute and unmute based on incoming data, which is not what the spec says at all.

This makes it hard for apps to deduce anything meaningful from the mute/unmute events from negotiation (SRD/SLD). So that's a mess. The mess would be much reduced if the mute event was to spec, and could only fired from actual network issues (actual timeout or BYE), not content.

Do we have anyone who could do this?

Flags: needinfo?(dminor)
Flags: needinfo?(na-g)

I put out a feeler in the crbug as well.

There are a number of observers in the RTCPReceiver code. We could perhaps piggy back on one of the existing ones or add a new one to propagate byes/timeouts. That could get us up to the audio stream or channel level pretty easily. From there we could probably wire something up to Call and then to the AudioConduit. The audio code has changed a lot since the last time we did an update, we caught them part way through a refactor, so I think it would be difficult to rebase a local fix and try to upstream it. We should see if they are interested in fixing this before we try this on our own.

Flags: needinfo?(dminor)

I worry we may have some "at risk" behavior here otherwise: https://w3c.github.io/webrtc-pc/#mediastreamtrack

Note that "mute" in this context applies to audio and video tracks.

(In reply to Jan-Ivar Bruaroey [:jib] (needinfo? me) from comment #24)

Note that "mute" in this context applies to audio and video tracks.

The RTCPReceiver ends up being owned by the audio channel and the video send stream (at least in our version of webrtc.org) so going through the channel/and or streams would be the natural way to propagate information back up through the conduits. I didn't realize this applied to video tracks as well, but I think the implementation would end up being similar in that case.

Blocks: 1595479

I discussed this with drno and he thinks it is worthwhile to maintain a local patch on our side to preserve the spec behaviour. Hopefully this will be fixed upstream and we can drop our version with the next webrtc.org update. I filed Bug 1595479 to track doing this, I'll resolve this bug as fixed.

Status: NEW → RESOLVED
Closed: 3 months ago
Flags: needinfo?(na-g)
Keywords: leave-open
Resolution: --- → FIXED

Great! Since all that landed here was a test, resolving as duplicate rather than fixed.

Resolution: FIXED → DUPLICATE
Duplicate of bug: 1595479
You need to log in before you can comment on or make changes to this bug.