1408256 - max-fs does not work when reusing RTCpeerconnection

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect, P3)

Description

[xpeng]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0
Build ID: 20170926190823

Steps to reproduce:

1. use max-fs to control the resolution in sdp first time
2. reuse the RTCpeerconnection and use the max-fs to control the resolution in sdp

the demo we can not provide right now.  if you can not  find the issue without demo we will provide the demo as soon as possible .


Actual results:

It works in step 1, but do not work in step 2.


Expected results:

max-fs should control the resolution normally  when reusing RTCpeerconnection

Comment 1

[Nils Ohlmeier [:drno]]

Not sure what exactly you mean by "reuse". To me that would be closing a PeerConnection and then try to use it again. But since you can't use a once closed PeerConnection any more I assume "reuse" mean renegotitation of some form. Can you tell us more about what get renegotiated here?
Did a re-offer only try to change the max-fs parameter? Or did the m-section get disabled and then re-enabled later with a different max-fs value?

Did this ever work in a previous release of Firefox?

Comment 2

[xpeng]

the max-fs works well in firefox 55 and before.
the implement of  reusing peerconnection follows the blog: https://hacks.mozilla.org/2015/03/webrtc-in-firefox-38-multistream-and-renegotiation/

Comment 3

[Byron Campen [:bwc]]

Attachment: Bug 1408256: Tests for bug.

Comment 4

[Byron Campen [:bwc]]

Attachment: Bug 1408256: Fix bug where renegotiation would un-set a previously negotiated max-fs.

Depends on D55889

Comment 5

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4620dbc0f12d2e08f32be945a1bed9f20ce5bbc3

Comment 6

[Byron Campen [:bwc]]

Try push never ran anything. Here's another.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e3c90be45234f9162578fed677833232fd9ab7d7

Comment 7

[Byron Campen [:bwc]]

Marking as blocking 1591199, because the patches on that bug fail tests due to this bug.

Comment 8

[Byron Campen [:bwc]]

Seeing some leaks in that try push, trying a push for the base revision here:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=671258ba18df7a5ebb7369bd92d2e733641c307d

Comment 9

[Byron Campen [:bwc]]

Hooray, try push in comment 8 didn't run anything. Getting really sick of that bug.

Comment 10

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=20789787a1b8afcc80fce564722677251e5b9cd9

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bfbff4e80023
Tests for bug. r=dminor
https://hg.mozilla.org/integration/autoland/rev/b2134c0ea6e8
Fix bug where renegotiation would un-set a previously negotiated max-fs. r=dminor

Comment 12

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/bfbff4e80023
https://hg.mozilla.org/mozilla-central/rev/b2134c0ea6e8