Nightly 57.0a1 does not like WebRTC SDP offer it produces

RESOLVED FIXED in Firefox 57

Status

()

defect
P1
normal
Rank:
17
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ibc, Assigned: drno)

Tracking

({regression})

57 Branch
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox55 unaffected, firefox56 unaffected, firefox57 fixed)

Details

Attachments

(2 attachments)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.113 Safari/537.36

Steps to reproduce:

- Create a PeerConnection and add audio and video (getUserMedia).

- Get the video associated RtpSender and call stop() on it.

- Call pc.createOffer().then((offer) => pc.setLocalDescription(offer)).


Actual results:

pc.setLocalDescription() fails with:

"Invalid description, no ice-ufrag attribute"

Well, it's worth noting that such a offer is created by Firefox itself, and when a m= section has been disabled (via videoRtpSender.stop()) Firefox does NOT add ICE lines into such an inactive m= section.


Expected results:

It should work (as it does in Firefox stable).
To reproduce it, just enter into this URL:

https://v2demo.mediasoup.org

(please, reload the page twice so verbose logs are enabled).

Once you have your webcam activated, click on the webcam icon to remove it. That action will invoke webcamRtpSender.stop() followed by a pc.setLocalDescription() that fails with the error above.

Again: this does NOT happen in Firefox stable.
To clarify, I meant:

"That action will invoke webcamRtpSender.stop() followed by a pc.createOffer() and a corresponding pc.setLocalDescription(offer) that fails with the error above."
Component: General → WebRTC
Component: WebRTC → WebRTC: Signaling
Assignee: nobody → drno
Depends on: 1392745
Keywords: regression
Comment on attachment 8906093 [details]
Bug 1396974: stop bundeling inactive m-sections.

https://reviewboard.mozilla.org/r/177826/#review182906

a=inactive shouldn't prevent bundle on its own. We should avoid bundling _disabled_ m-sections though. I've actually fixed this in my transceivers work; I thought I had messed this up with my patches!
Attachment #8906093 - Flags: review?(docfaraday) → review-
Comment on attachment 8906094 [details]
Bug 1396974: make tests ensure no bundle-only is present in inactive m-swctions.

https://reviewboard.mozilla.org/r/177828/#review182908
Attachment #8906094 - Flags: review?(docfaraday) → review+
Rank: 17
Priority: -- → P1
Comment on attachment 8906093 [details]
Bug 1396974: stop bundeling inactive m-sections.

https://reviewboard.mozilla.org/r/177826/#review182906

Fair enough. And great that this going to be fixed with Transceivers. But I butchered this in 57. As the Transceivers are not likely to land in 57 I think we should fix this somehow in 57. Plus this patch only restores the old behavior (before I landed bug 1392745 disabled m-sections never got an mid set and so they got included in bundles).

Now the reason I did not use MsectionIsDisabled() here is that MsectionIsDisabled() checks the bundle-only attribute, which is only going to be set by this code here. Should I then just check for port == 0 here instead of inactive?
Comment on attachment 8906093 [details]
Bug 1396974: stop bundeling inactive m-sections.

https://reviewboard.mozilla.org/r/177826/#review182906

a=bundle-only should not be set here, so you should get the same result either way. I agree we should go ahead and fix now.
Comment on attachment 8906093 [details]
Bug 1396974: stop bundeling inactive m-sections.

https://reviewboard.mozilla.org/r/177826/#review183916
Attachment #8906093 - Flags: review?(docfaraday) → review+
Pushed by drno@ohlmeier.org:
https://hg.mozilla.org/integration/autoland/rev/ae71d517c443
stop bundeling inactive m-sections. r=bwc
https://hg.mozilla.org/integration/autoland/rev/06c3517b0f33
make tests ensure no bundle-only is present in inactive m-swctions. r=bwc
https://hg.mozilla.org/mozilla-central/rev/ae71d517c443
https://hg.mozilla.org/mozilla-central/rev/06c3517b0f33
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.