1322358 - All media lines get disabled in answer SDP when all m-lines in offer are inactive

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect, P2)

Description

[ushunmugan]

Attachment: m-lines-get-disabled.txt

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/55.0.2883.75 Safari/537.36

Steps to reproduce:

During a WebRTC video call, a new offer was received by Firefox, with all m-lines having a=inactive.  After doing setRemoteDescription, an answer was created.


Actual results:

The answer SDP had all m-lines disabled with port numbers set to 0, and had no SSRC attributes.  Note that this behavior is always reproducible.  The testing was done with latest stable version of Firefox (50.0.2).


Expected results:

All m-lines should have valid ports, with a=inactive and SSRC attributes.  This would ensure ICE connection remains, and RTCP is sent and received.

Comment 1

[Nils Ohlmeier [:drno]]

Byron what are your thoughts on this?

Comment 2

[Byron Campen [:bwc]]

Pretty sure this is not the behavior we want. I'll look into it.

Comment 3

[ushunmugan]

Any update on this?  I hope that this issue gets looked at soon and hopefully resolved.  Due to this issue, hold/resume of a call wouldn't work on Firefox with a SIP endpoint, connected via a WebRTC gateway. FYI, the same scenario works fine on Chrome. As a workaround, we had to add a hack on the client side, which modifies a=inactive to a=sendonly in the offer SDP before doing the setRemoteDescription. However, this is not good, as the change is hacky, and the stream isn't really made inactive.

Comment 4

[Byron Campen [:bwc]]

Attachment: Bug 1322358: Don't disable m-sections in answer just because they're inactive.

Review commit: https://reviewboard.mozilla.org/r/107294/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/107294/

Comment 5

[Nils Ohlmeier [:drno]]

Comment on attachment 8830540 [details]
Bug 1322358: Don't disable m-sections in answer just because they're inactive.

https://reviewboard.mozilla.org/r/107294/#review108460

Assuming the answer to my question below is yes.

::: media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
(Diff revision 1)
> -  if (!msection.IsReceiving() && !msection.IsSending()) {
> -    mSdpHelper.DisableMsection(sdp, &msection);
> -    return NS_OK;
> -  }

Do we have code which still handles the case of missing direction attributes (which appears to me the functionality we loose by deleting this)?

Comment 6

[Byron Campen [:bwc]]

Comment on attachment 8830540 [details]
Bug 1322358: Don't disable m-sections in answer just because they're inactive.

https://reviewboard.mozilla.org/r/107294/#review108460

> Do we have code which still handles the case of missing direction attributes (which appears to me the functionality we loose by deleting this)?

We interpret missing direction attributes as a=sendrecv, as in the spec, so we're fine there.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c913bf6e11c4
Don't disable m-sections in answer just because they're inactive. r=drno

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c913bf6e11c4

Comment 9

[ushunmugan]

Is this fixed in Firefox nightly already?  I was testing today with the latest nightly version, 54.0a1 (2017-02-07), and I still see the same issue.

Comment 10

[ushunmugan]

BTW, there seems to be some new video issue in nightly as well as in developer (53.0a2) versions, where video is not played after a renegotiation.  This problem doesn't exist on version 50, or beta 52.0b4.

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

(In reply to ushunmugan from comment #10)
> BTW, there seems to be some new video issue in nightly as well as in
> developer (53.0a2) versions, where video is not played after a
> renegotiation.  This problem doesn't exist on version 50, or beta 52.0b4.

Please file a separate bug; we know renegotiation work in 53, but perhaps there's a bug in a specific case, or perhaps there's a bug in the test you're running.  Major rework of the media and renegotiation code landed in 53.

(In reply to ushunmugan from comment #9)
> Is this fixed in Firefox nightly already?  I was testing today with the
> latest nightly version, 54.0a1 (2017-02-07), and I still see the same issue.

the original bug is supposed to be fixed in Nightly/54 - nils?

Comment 12

[Nils Ohlmeier [:drno]]

(In reply to Randell Jesup [:jesup] from comment #11)
> (In reply to ushunmugan from comment #9)
> > Is this fixed in Firefox nightly already?  I was testing today with the
> > latest nightly version, 54.0a1 (2017-02-07), and I still see the same issue.
> 
> the original bug is supposed to be fixed in Nightly/54 - nils?

I'm not the one who fixed it. But it looks like Byron's fix landed in Nightly. Byron?
So if it's not working I guess we should re-open this bug.

Is there a test page available where we re-produce this issue?

Comment 13

[Byron Campen [:bwc]]

There are currently only three cases where we will disable an m-section, looking at the code:

1. We are creating an offer, have no track to send, and are not configured to offer to receive.
2. We are creating an answer, and the m-section was disabled (port==0 and not bundle-only) in the offer.
3. We are creating an answer, and negotiation has failed for that m-section.

Can I get a test-case that I can run that reproduces this for you?

Comment 14

[Byron Campen [:bwc]]

I have a feeling what we're seeing here is case 1 in the above comment. I _think_ we should just do a=inactive in that case.

Comment 15

[Byron Campen [:bwc]]

Ok, I think the behavior in case 1 is correct; when creating a new offer, if the transceiver is stopped, the m-section gets disabled. We don't have transceivers yet, so it is a little muddy, but removing the local track _and_ not offering to receive seems about the same.

Comment 16

[ushunmugan]

Hi Byron,

Thanks!  I think that it's case #1 too.  I couldn't give you a test script right now, but it should be very easy to reproduce.  After an audio-only call is established, do a setRemoteDescription with an offer SDP that has a valid m-line (with non-zero port) with a=inactive set, and create the answer - the answer will have the m-line disabled with the port set to 0.  Here is an example:

This is the new offer SDP from peer:
v=0
o=xxx 833 3 IN IP4 0.0.0.0
s=-
c=IN IP4 0.0.0.0
t=0 0
a=group:BUNDLE sdparta_0
a=msid-semantic: WMS F9C4D094-2D57-4072-877C-94B6FDB63273
m=audio 36000 RTP/SAVPF 9 101
a=inactive
a=rtcp-mux
a=mid:sdparta_0
a=ssrc:316236972 cname:HOkMnEjWnUih9fX/fyrWM+UR
a=ssrc:316236972 label:a1-D094-2D57-407
a=ssrc:316236972 msid:F9C4D094-2D57-4072-877C-94B6FDB63273 a1-D094-2D57-407
a=ssrc:316236972 mslabel:F9C4D094-2D57-4072-877C-94B6FDB63273
a=ice-ufrag:sCah
a=ice-pwd:WePX71BPcCZghb2sSOIz38
a=rtpmap:9 g722/8000
a=rtpmap:101 telephone-event/8000
a=fmtp:101 0-15
a=fingerprint:sha-256 EB:95:EC:03:8B:21:39:58:81:10:8D:9D:0A:B1:3C:6D:DC:08:9D:59:59:3A:51:8A:BC:88:80:C8:4F:FE:6D:4F

This is the answer SDP created by Firefox nightly 54.0a1:
v=0
o=mozilla...THIS_IS_SDPARTA-54.0a1 3442797379914774747 1 IN IP4 0.0.0.0
s=-
t=0 0
a=fingerprint:sha-256 30:70:F0:B6:ED:2A:36:F9:2B:7D:BC:AE:DD:30:D7:E2:CD:A7:91:C5:56:F3:50:FF:59:0A:E3:32:59:2F:2C:0B
a=ice-options:trickle
a=msid-semantic:WMS *
m=audio 0 RTP/SAVPF 0
c=IN IP4 0.0.0.0
a=inactive
a=rtpmap:0 PCMU/8000

Comment 17

[ushunmugan]

I agree with comment #14 (just set a=inactive but don't disable the m-line), and I am not sure what is exactly meant by comment #15.  Note, with an inactive m-line, RTP should stop, but RTCP should continue to flow.

Comment 18

[ushunmugan]

FYI, re. the media issue mentioned in comment #10, I've created issue 1338005.