Closed Bug 1337777 Opened 8 years ago Closed 7 years ago

Audio/video on Ciscospark is not working

Categories

(Core :: WebRTC, defect, P1)

Product:
Core
Component:
WebRTC
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla54
Tracking Flags:
Tracking Status
firefox51 --- unaffected
firefox52 --- unaffected
firefox53 --- fixed

People

(Reporter: ccomorasu, Assigned: jesup)

References

Details

Attachments

(4 files, 2 obsolete files)

if no receive-SSRC was signaled for video, on the first packet reset the VideoReceiveStream
19.50 KB, patch
bwc
: review+
lizzard
: approval-mozilla-beta+
Details | Diff | Splinter Review
Add test for SDP answer without a=ssrc
9.49 KB, patch
drno
: review+
lizzard
: approval-mozilla-aurora+
lizzard
: approval-mozilla-beta+
Details | Diff | Splinter Review
ensure mSend/RecvStream access is locked
11.37 KB, patch
bwc
: review+
lizzard
: approval-mozilla-beta+
Details | Diff | Splinter Review
issue behavior on Mac vs. Windows 10
1.43 MB, image/jpeg
Details
[Affected versions]: Fx 54.0a1 (2017-02-08) Fx 53.0a2 (2017-02-08) [Affected platforms]: Mac OS X 10.12.3 Ubuntu 14.04 LTS x64 Windows 8.1 x64 Windows 10 x64 [Steps to reproduce]: 1. Launch Firefox. 2. Go to https://web.ciscospark.com/ . 3. Log in with two accounts. 4. Start a video conversation between the accounts. [Expected result]: The conversation goes without any issues. [Actual result]: There is no video or sound. [Regression range]: This seems like a regression and we`ll return with more info as soon as possible. [Additional notes]: a. Between the native app and the browser, the audio for the native app user is working. b. Here is a list of the tested environments: Windows 10 x64 + Mac OS X 10.12 Ubuntu 14.04 x64 + Mac OS X 10.12 Ubuntu 14.04 x64 + Windows 8.1 x64 Ubuntu 14.04 x64 + Native Client (Windows 10 x64) Ubuntu 14.04 x64 + Native Client (Windows 8.1 x64) Native Client (Windows 10 x64) + Windows 10 x64
Likely a branch49 regression, giving this to Jesup to investigate.
Assignee: nobody → rjesup
Rank: 10
Priority: -- → P1
Webrtc.org Branch49 regression. Due to assertions in the Call API, pkerr added code to fail conduit/pipeline creation if the ssrc for a receive stream wasn't specified at creation time. This means that it insisted that the other side specify a=ssrc for at least the m=video section. This was removed as a MUST from JSEP-17 ~4 months ago. CiscoSpark/linus doesn't signal SSRCs. To solve it, we have to either a) create the conduit's VideoReceiveStream lazily when the first packet arrives (at least in the no-ssrc-signaled case), which is what jingle/Chrome does, or b) destroy and recreate the VideoReceiveStream on the first packet, if no SSRC was signaled. Due to the number of assumptions broken by trying to have an active conduit with mRecvStream == null, I decided to go with 'b', and simply use a random ssrc as for initial creation if none was signaled. On the first packet, if no SSRC was signaled, start queuing packets and send a runnable to MainThread to set the SSRC (and rebuild the VideoReceiveStream). On the next packet after the SSRC is set, dump all queued packets into the receive stream. This does have a minor weakness in that the packets are delayed until the next packet comes in.
Blocks: 1250356
Keywords: regressionwindow-wanted
Also, once this is solved I hit a TIAS kbps vs bps issue; see bug 1342727
Depends on: 1342727
Note that this stumbles over the use of the PCHandle as a global when initializing the OpenH264 gmp plugin. MozReview-Commit-ID: Cn714G29vwz
Attachment #8841295 - Flags: review?(docfaraday)
Paul - you can try out the Try build, assuming it builds happy. Also please verify the video bitrate stays within the TIAS boundary (modulo jitter/etc)
Flags: needinfo?(paulej)
Randell, I placed a call using firefox-54.0a1.en-US.win64 (running on Windows 10 Pro). Call looked good. The call was between a desktop machine running FF and laptop running a native Spark client. FF appeared to be emitting most video data (82.64% of it) between at rates between 500Kbps and 1000Kbps. The rest was below 500Kbps. The Spark client was split fairly evenly between sending in the range of 500Kbps - 1Mbps or 1Mbps and 2Mbps during the call. Packet loss was extremely low. Call was routed through two data centers. I do not know what the E2E delay was, but it appeared to be fairly low by my eyes. I do know the delay from the native Spark client to the first data center was generally (81.98% of it) less than 50ms, with 18.02% of the traffic between 50 and 150ms. TIAS advertised by FF was 1000000 for video. The TIAS value presented by Linus to FF was 600000. It appeared from looking at a time series graph that FF was transmitting very close to 600Kbps. However, I'll admit that the chart is very hard to read, because the tic marks are a few hundred Kbps apart. There's a tic mark at 480Kbps and the next one is at 720Kbps. The best I can report positively is that the flow is "about in the middle" between those two tic marks, which would place it about 600Kbps.
Flags: needinfo?(paulej)
Note that this stumbles over the use of the PCHandle as a global when initializing the OpenH264 gmp plugin. MozReview-Commit-ID: Cn714G29vwz * * * [mq]: forward-first-packet MozReview-Commit-ID: BBKUNHnH3AL * * * [mq]: forget MozReview-Commit-ID: Ip5G0AAYjDa
Attachment #8841437 - Flags: review?(docfaraday)
Attachment #8841295 - Attachment is obsolete: true
Attachment #8841295 - Flags: review?(docfaraday)
I updated the patch, as the original version broke down in some screencapture cases since it relied on the next incoming packet to 'kick' the queued packets out. Now it instead sends a second runnable/lambda back to STS thread to release the queued packets.
Comment on attachment 8841437 [details] [diff] [review] if no receive-SSRC was signaled for video, on the first packet reset the VideoReceiveStream Review of attachment 8841437 [details] [diff] [review]: ----------------------------------------------------------------- Wait... webrtc.org doesn't handle the case where the remote end's ssrc changes (without any signalling)? Isn't that kinda broken? Is the latest update we're planning to do any better in this regard? As long as we're performing horrible contortions to work around these bugs, maybe we should consider just rewriting the ssrc on incoming packets to something we arbitrarily selected when the ssrc wasn't signaled... yes it would be gross, but it wouldn't involve a bunch of thread dispatches and queueing gymnastics.
Flags: needinfo?(rjesup)
Attached patch Add test for SDP answer without (obsolete) — — Splinter Review
perhaps should test more no-ssrc cases (incoming offer); feel free to suggest changes. I verified in about: webrtc that it's making calls with answers with no a=ssrc lines.
Attachment #8841685 - Flags: review?(drno)
Comment on attachment 8841685 [details] [diff] [review] Add test for SDP answer without Review of attachment 8841685 [details] [diff] [review]: ----------------------------------------------------------------- I think this is on the right track, but needs some improvements before we land it. ::: dom/media/tests/mochitest/test_peerConnection_noSSRCSignaled.html @@ +10,5 @@ > + title: "Basic video only call without a=ssrc" > + }); > + > + var test; > + function removeSSRC(desc) { I think this helper function should go into sdpUtils.js to avoid people reinventing these. Note: most functions in sdpUtils.js just take desc.sdp as input and return just the SDP as text. @@ +12,5 @@ > + > + var test; > + function removeSSRC(desc) { > + var sdp = desc.sdp.replace(/\r\na=ssrc.*\r\n/, "\r\n"); > + return new mozRTCSessionDescription({ type: desc.type, sdp: sdp }); No 'moz' prefix any more please. @@ +20,5 @@ > + test = new PeerConnectionTest(options); > + test.setMediaConstraints([{video: true}], [{video: true}]); > + test.chain.replace('PC_LOCAL_GET_ANSWER', [ > + function PC_LOCAL_GET_FULL_ANSWER(test) { > + return test.pcRemote.endOfTrickleIce.then(() => { Why is this waiting for the end of candidates indication? I fail to see the dependency here. @@ +26,5 @@ > + test._answer_constraints = test.pcRemote.constraints; > + }); > + } > + ]); > + test.run(); If I understand the original bug report then the bug did not result in signaling or ICE problems, but simply the received video not being rendered, right? So we really need to pull in the canvas painting and verifying trick here to be sure that things actually get rendered now (even though I dislike how often we have to pull in that overhead).
Attachment #8841685 - Flags: review?(drno) → review-
> > + function removeSSRC(desc) { > > I think this helper function should go into sdpUtils.js to avoid people > reinventing these. > Note: most functions in sdpUtils.js just take desc.sdp as input and return > just the SDP as text. Sure. > @@ +12,5 @@ > > + > > + var test; > > + function removeSSRC(desc) { > > + var sdp = desc.sdp.replace(/\r\na=ssrc.*\r\n/, "\r\n"); > > + return new mozRTCSessionDescription({ type: desc.type, sdp: sdp }); > > No 'moz' prefix any more please. Was just cut-and-pasting from nonTrickleICE.js > @@ +20,5 @@ > > + test = new PeerConnectionTest(options); > > + test.setMediaConstraints([{video: true}], [{video: true}]); > > + test.chain.replace('PC_LOCAL_GET_ANSWER', [ > > + function PC_LOCAL_GET_FULL_ANSWER(test) { > > + return test.pcRemote.endOfTrickleIce.then(() => { > > Why is this waiting for the end of candidates indication? > I fail to see the dependency here. Cut-and-paste, and not really having my head in the test chain structure - recycled those neurons. > @@ +26,5 @@ > > + test._answer_constraints = test.pcRemote.constraints; > > + }); > > + } > > + ]); > > + test.run(); > > If I understand the original bug report then the bug did not result in > signaling or ICE problems, but simply the received video not being rendered, > right? > So we really need to pull in the canvas painting and verifying trick here to > be sure that things actually get rendered now (even though I dislike how > often we have to pull in that overhead). Yeah... perhaps use one of the captureStream examples as a base. They were the tests broken by the first patches here, since they tend to not throw a lot of packets around.
> Wait... webrtc.org doesn't handle the case where the remote end's ssrc > changes (without any signalling)? Isn't that kinda broken? Is the latest > update we're planning to do any better in this regard? I doubt it handles it properly or maybe does via a collision callback (but that would cause others to see an unsignaled change)... and it still has the "SSRC 0 means never set" evilness :-( > As long as we're performing horrible contortions to work around these bugs, > maybe we should consider just rewriting the ssrc on incoming packets to > something we arbitrarily selected when the ssrc wasn't signaled... yes it > would be gross, but it wouldn't involve a bunch of thread dispatches and > queueing gymnastics. That would simplify this code a lot. However, this would greatly complicate a bunch more distributed code for handling stats and the like, and what happens in renegotiations. I tried initially (as mentioned above) delaying creating mRecvStream, but that broke a different set of assumptions in our code. That would be closer in concept to the code in jingle that webrtc.org assumes is used.
Signaling the SSRCs in theory allows the clients to avoid collisions, though in conferencing cases this gets more interesting. But un-announced SSRCs (enabled in JSEP in -17 ~ 4 months ago) breaks that ability.
(In reply to Randell Jesup [:jesup] from comment #16) > > Wait... webrtc.org doesn't handle the case where the remote end's ssrc > > changes (without any signalling)? Isn't that kinda broken? Is the latest > > update we're planning to do any better in this regard? > > I doubt it handles it properly or maybe does via a collision callback (but > that would cause others to see an unsignaled change)... and it still has the > "SSRC 0 means never set" evilness :-( Can we check this before we do something this invasive? > > As long as we're performing horrible contortions to work around these bugs, > > maybe we should consider just rewriting the ssrc on incoming packets to > > something we arbitrarily selected when the ssrc wasn't signaled... yes it > > would be gross, but it wouldn't involve a bunch of thread dispatches and > > queueing gymnastics. > > That would simplify this code a lot. However, this would greatly complicate > a bunch more distributed code for handling stats and the like, and what > happens in renegotiations. I tried initially (as mentioned above) delaying > creating mRecvStream, but that broke a different set of assumptions in our > code. That would be closer in concept to the code in jingle that webrtc.org > assumes is used. For my part, I'd be totally happy with the stats having the wrong ssrc on them when someone uses unannounced ssrcs.
(In reply to Randell Jesup [:jesup] from comment #15) > > If I understand the original bug report then the bug did not result in > > signaling or ICE problems, but simply the received video not being rendered, > > right? > > So we really need to pull in the canvas painting and verifying trick here to > > be sure that things actually get rendered now (even though I dislike how > > often we have to pull in that overhead). > > Yeah... perhaps use one of the captureStream examples as a base. They were > the tests broken by the first patches here, since they tend to not throw a > lot of packets around. Fair point. So maybe we need a better way. For example the color on the source canvas changing over time so it keeps generating more video packets?
majorly updated; based on captureStream
Attachment #8841752 - Flags: review?(drno)
Attachment #8841685 - Attachment is obsolete: true
also fixes a (spurious) warning in the gmp interface, and moves the field members locked to be co-located with the mutex
Attachment #8841761 - Flags: review?(docfaraday)
Attachment #8841752 - Attachment description: Add test for SDP answer without → Add test for SDP answer without a=ssrc
(In reply to Byron Campen [:bwc] from comment #18) > > I doubt it handles it properly or maybe does via a collision callback (but > > that would cause others to see an unsignaled change)... and it still has the > > "SSRC 0 means never set" evilness :-( > > Can we check this before we do something this invasive? Not so easy to test... > > > As long as we're performing horrible contortions to work around these bugs, > > > maybe we should consider just rewriting the ssrc on incoming packets to > > > something we arbitrarily selected when the ssrc wasn't signaled... yes it > > > would be gross, but it wouldn't involve a bunch of thread dispatches and > > > queueing gymnastics. > > > > That would simplify this code a lot. However, this would greatly complicate > > a bunch more distributed code for handling stats and the like, and what > > happens in renegotiations. I tried initially (as mentioned above) delaying > > creating mRecvStream, but that broke a different set of assumptions in our > > code. That would be closer in concept to the code in jingle that webrtc.org > > assumes is used. > > For my part, I'd be totally happy with the stats having the wrong ssrc on > them when someone uses unannounced ssrcs. Unannounced SSRCs are totally allowed in the spec (and in fact the big problem here is not handling arbitrary SSRCs in Call (and SSRCs that change!), because Call is based on Plan B).
(In reply to Randell Jesup [:jesup] from comment #22) > (In reply to Byron Campen [:bwc] from comment #18) > > > I doubt it handles it properly or maybe does via a collision callback (but > > > that would cause others to see an unsignaled change)... and it still has the > > > "SSRC 0 means never set" evilness :-( > > > > Can we check this before we do something this invasive? > > Not so easy to test... Actually, unless I'm mistaken, we have a mochitest for this already. The simulcast test works by selectively filtering SSRCs on the receiver side (and changing which SSRCs are filtered as the test runs), before the RTP gets down to webrtc.org. That will look exactly like SSRCs changing mid-session, without any signaling.
Comment on attachment 8841437 [details] [diff] [review] if no receive-SSRC was signaled for video, on the first packet reset the VideoReceiveStream Review of attachment 8841437 [details] [diff] [review]: ----------------------------------------------------------------- We have a mochitest running in CI (test_peerConnection_simulcastOffer.html) that shows that webrtc.org can handle the ssrc of a video stream changing without requiring additional signaling. Let's just write a patch that picks a random recv ssrc (or maybe even a fixed value) when one is not signaled, and rely on webrtc.org to handle what actually arrives. We have good reason to believe that this will work fine. If there is some problem that the mochitest does not detect, we can revisit it then.
Attachment #8841437 - Flags: review?(docfaraday) → review-
Comment on attachment 8841761 [details] [diff] [review] ensure mSend/RecvStream access is locked Review of attachment 8841761 [details] [diff] [review]: ----------------------------------------------------------------- It would be good if we had some thread assertions? Maybe later.
Attachment #8841761 - Flags: review+
> We have a mochitest running in CI (test_peerConnection_simulcastOffer.html) > that shows that webrtc.org can handle the ssrc of a video stream changing > without requiring additional signaling. There's no signaling there, but the underlying webrtc.org code doesn't see SSRC changes. SetRemoteSSRC is called, which effectively destroys the VideoReceiveStream and re-creates it with a new SSRC. And in fact that's what this code does, with the complication of having to send runnables around and buffer packets (it would work without the buffering, but would cause a startup delay since it would have to send an RTCP asking for a full iframe refresh). > Let's just write a patch that picks > a random recv ssrc (or maybe even a fixed value) when one is not signaled, > and rely on webrtc.org to handle what actually arrives. We have good reason > to believe that this will work fine. If there is some problem that the > mochitest does not detect, we can revisit it then. We do need this patch (or an alternative that delays creating mRecvStream, but that has all these complications and more besides). The Call api insists on having a set SSRC(s) at creation time, and does not allow changing it/them. :-( :-( Very plan-B....
Comment on attachment 8841437 [details] [diff] [review] if no receive-SSRC was signaled for video, on the first packet reset the VideoReceiveStream re-requesting review given last comment.
Attachment #8841437 - Flags: review- → review?(docfaraday)
(In reply to Randell Jesup [:jesup] from comment #27) > > We have a mochitest running in CI (test_peerConnection_simulcastOffer.html) > > that shows that webrtc.org can handle the ssrc of a video stream changing > > without requiring additional signaling. > > There's no signaling there, but the underlying webrtc.org code doesn't see > SSRC changes. SetRemoteSSRC is called, which effectively destroys the > VideoReceiveStream and re-creates it with a new SSRC. And in fact that's > what this code does, with the complication of having to send runnables > around and buffer packets (it would work without the buffering, but would > cause a startup delay since it would have to send an RTCP asking for a full > iframe refresh). But does it actually cause any harm when the SSRC changes? The video stream gets rendered, at least according to the mochitest. Are we mainly worried about the stats here? Or is there some internal logic that is going to get really confused? Maybe this could be the source of the intermittents we see on the simulcast test?
Attachment #8841437 - Flags: review?(docfaraday) → review+
(In reply to Byron Campen [:bwc] from comment #29) > > There's no signaling there, but the underlying webrtc.org code doesn't see > > SSRC changes. SetRemoteSSRC is called, which effectively destroys the > > VideoReceiveStream and re-creates it with a new SSRC. And in fact that's > > what this code does, with the complication of having to send runnables > > around and buffer packets (it would work without the buffering, but would > > cause a startup delay since it would have to send an RTCP asking for a full > > iframe refresh). > > But does it actually cause any harm when the SSRC changes? The video stream > gets rendered, at least according to the mochitest. Are we mainly worried > about the stats here? Or is there some internal logic that is going to get > really confused? Maybe this could be the source of the intermittents we see > on the simulcast test? If the incoming SSRC changes, and no one calls SetRemoteSSRC (and thus tears down/recreates the VideoReceiveStream), we will not render anything - we (webrtc.org that is) will drop every packet on the floor. Before this patch, if a=ssrc wasn't including in the SDP, we'd simply fail to create the VideoReceiveStream and error out. If we bent the incoming packets to match the random SSRC we picked to let Create succeed, then we'd have to modify ALL RTCPs to match the SSRC (inbound and outbound), and modify the stats. Plan B sucks. It wouldn't be so bad if the code supported ChangeRemoteSSRC/etc, but it doesn't.
Pushed by rjesup@wgate.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/52bee438b042 if no receive-SSRC was signaled for video, on the first packet reset the VideoReceiveStream r=bwc https://hg.mozilla.org/integration/mozilla-inbound/rev/cfa8acce82af ensure mSend/RecvStream access is locked r=bwc
leave-open until we land the test as well
Keywords: leave-open
Comment on attachment 8841752 [details] [diff] [review] Add test for SDP answer without a=ssrc Review of attachment 8841752 [details] [diff] [review]: ----------------------------------------------------------------- LGTM
Attachment #8841752 - Flags: review?(drno) → review+
Pushed by rjesup@wgate.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/efaf63a68ee2 Add test for SDP answer without a=ssrc r=drno
Keywords: leave-open
Depends on: 1344557
Hello! After some investigation using Firefox Nightly 54.0a1 (2017/03/05/) we found some issues. Below is a list with the results. Ubuntu 16.04 LTS - Windows 10 x64 - Video not working for Windows (web). Windows 10 x64(native app) - Windows 10 x64 - Video not working for Windows(web). Windows 8.1 x64 (web) - Windows 10x64 (web) - Video not working for Windows 8.1 x64 (web). Windows 10x64(web) - Mac OS X 10.12 - Video not working for Windows (web). *note: - video for Windows native app and Ubuntu is correctly displayed. - video for Mac(web) and Ubuntu(web) is correctly displayed. - video for Windows 8.1 x64(web) and Mac OS X 10.12 is correctly displayed. - video for Windows 10x64(native app) and Mac OS X 10.12 is correctly displayed. The issue seems to be Windows related. Let me know if you need more information.
Comment on attachment 8841752 [details] [diff] [review] Add test for SDP answer without a=ssrc Review of attachment 8841752 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/tests/mochitest/test_peerConnection_captureStream_canvas_2d_noSSRC.html @@ +5,5 @@ > + <script type="application/javascript" src="/tests/dom/canvas/test/captureStream_common.js"></script> > +</head> > +<body> > +<pre id="test"> > +<script type="application/javascript;version=1.8"> Please do not use versioned JavaScript anymore (see bug 1342144 for details). Because this test uses neither for-each nor legacy generators, you can just remove the version parameter.
See comment #37.
Flags: needinfo?(rjesup)
See Also: → 1342764
> > +<script type="application/javascript;version=1.8"> > > Please do not use versioned JavaScript anymore (see bug 1342144 for details). > > Because this test uses neither for-each nor legacy generators, you can just > remove the version parameter. This was cloned from another test in the directory... probably many of our tests do this. Please file a bug for cleaning up version usage in dom/media/tests/mochitest as a whole.
Flags: needinfo?(rjesup) → needinfo?(VYV03354)
(In reply to Cristian Comorasu [:CristiComo] from comment #36) > Hello! After some investigation using Firefox Nightly 54.0a1 (2017/03/05/) > we found some issues. Below is a list with the results. > > Ubuntu 16.04 LTS - Windows 10 x64 - Video not working for Windows (web). > Windows 10 x64(native app) - Windows 10 x64 - Video not working for > Windows(web). > Windows 8.1 x64 (web) - Windows 10x64 (web) - Video not working for Windows > 8.1 x64 (web). > Windows 10x64(web) - Mac OS X 10.12 - Video not working for Windows (web). > > *note: - video for Windows native app and Ubuntu is correctly displayed. > - video for Mac(web) and Ubuntu(web) is correctly displayed. > - video for Windows 8.1 x64(web) and Mac OS X 10.12 is correctly > displayed. > - video for Windows 10x64(native app) and Mac OS X 10.12 is correctly > displayed. > > The issue seems to be Windows related. Let me know if you need more > information. I'm afraid I'm confused by this report. What were you testing? Spark? Does "(web)" mean ciscospark.com? What does "video not working for windows (web)" (and similar) mean? Sending video, receiving video, ...? Can you provide about:webrtc logs for one of these? Or xpcom (MOZ_LOG) signaling:5 logging Thanks
Flags: needinfo?(cristian.comorasu)
(In reply to Randell Jesup [:jesup] from comment #39) > > > +<script type="application/javascript;version=1.8"> > > > > Please do not use versioned JavaScript anymore (see bug 1342144 for details). > > > > Because this test uses neither for-each nor legacy generators, you can just > > remove the version parameter. > > This was cloned from another test in the directory... probably many of our > tests do this. Please file a bug for cleaning up version usage in > dom/media/tests/mochitest as a whole. Bug 1342144 was the bug exactly for that purpose. (Did you even read the bug?) Currently only 7 version parameters are present in the tree: https://dxr.mozilla.org/mozilla-central/search?q=/javascript;version=1.&redirect=false Are you sure you copied the test from the latest tree?
Flags: needinfo?(VYV03354) → needinfo?(rjesup)
> Are you sure you copied the test from the latest tree? I did - when I made the patch. Apparently your mass-change to remove them crossed with my new test copied from one you were modifying (I was waiting on review for over a week IIRC).
Flags: needinfo?(rjesup)
Pushed by rjesup@wgate.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/9a09749bb230 remove js version=1.8 from new noSSRC test (really bug 1342144) rs=jesup
Comment on attachment 8841752 [details] [diff] [review] Add test for SDP answer without a=ssrc Test addition to go with the code that landed in 54 (and I'm requesting for 53)
Attachment #8841752 - Flags: approval-mozilla-beta?
Attachment #8841752 - Flags: approval-mozilla-aurora?
Comment on attachment 8841437 [details] [diff] [review] if no receive-SSRC was signaled for video, on the first packet reset the VideoReceiveStream For all patches in the bug: Approval Request Comment [Feature/Bug causing the regression]: webrtc 49 [User impact if declined]: CiscoSpark (and others that don't signal a=ssrc, which isn't required) will fail to do video [Is this code covered by automated tests?]: yes (see patch here) [Has the fix been verified in Nightly?]: yes [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: other patches in this bug [Is the change risky?]: not very [Why is the change risky/not risky?]: leverages existing "recreate stream" logic; the only complexity is that it has to queue packets while reconfiguring and release when done, on the right thread. IN the process of this patch we add locking we found was missing, which should make it more stable than before. [String changes made/needed]: none
Attachment #8841437 - Flags: approval-mozilla-beta?
Attachment #8841761 - Flags: approval-mozilla-beta?
Comment on attachment 8841437 [details] [diff] [review] if no receive-SSRC was signaled for video, on the first packet reset the VideoReceiveStream WebRTC fix, verified in nightly, includes new test coverage. Let's take this for beta 2.
Attachment #8841437 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8841761 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8841752 - Flags: approval-mozilla-beta?
Attachment #8841752 - Flags: approval-mozilla-beta+
Attachment #8841752 - Flags: approval-mozilla-aurora?
Attachment #8841752 - Flags: approval-mozilla-aurora+
Setting qe-verify- based on Jesup's assessment on manual testing needs (see Comment 46) and the fact that this fix has automated coverage. Note that this particular scenario is part of our recurring WebRTC regression testing on aurora.
Flags: qe-verify-
(In reply to Randell Jesup [:jesup] from comment #40) > (In reply to Cristian Comorasu [:CristiComo] from comment #36) > > Hello! After some investigation using Firefox Nightly 54.0a1 (2017/03/05/) > > we found some issues. Below is a list with the results. > > > > Ubuntu 16.04 LTS - Windows 10 x64 - Video not working for Windows (web). > > Windows 10 x64(native app) - Windows 10 x64 - Video not working for > > Windows(web). > > Windows 8.1 x64 (web) - Windows 10x64 (web) - Video not working for Windows > > 8.1 x64 (web). > > Windows 10x64(web) - Mac OS X 10.12 - Video not working for Windows (web). > > > > *note: - video for Windows native app and Ubuntu is correctly displayed. > > - video for Mac(web) and Ubuntu(web) is correctly displayed. > > - video for Windows 8.1 x64(web) and Mac OS X 10.12 is correctly > > displayed. > > - video for Windows 10x64(native app) and Mac OS X 10.12 is correctly > > displayed. > > > > The issue seems to be Windows related. Let me know if you need more > > information. > > I'm afraid I'm confused by this report. What were you testing? Spark? Does > "(web)" mean ciscospark.com? What does "video not working for windows > (web)" (and similar) mean? Sending video, receiving video, ...? We tested the compatibility between https://web.ciscospark.com/ (referred as (web)) in comment 36 and the native app (downloadable here: https://www.ciscospark.com/downloads.html). i.e.:"Ubuntu 16.04 LTS (user X) - Windows 10 x64(user Y) - Video not working for Windows (web)." - user Y not able to see user X, black screen displayed, sound works accordingly - user X has no issues > Can you provide about:webrtc logs for one of these? Or xpcom (MOZ_LOG) > signaling:5 logging Unfortunately I could not provide the logs due to an issue within the log in. I was sent on an infinite loop when entering the e-mail to log in, thus could not reproduce the issue. > Thanks Based on Comment 46 and Comment 51 I will remove myself as a QA contact as this bug does not require manual QA. Let me know if I can help any further. Thank you!
Flags: needinfo?(cristian.comorasu)
QA Contact: cristian.comorasu
Hi everyone, I've been testing WebRTC and this issue just started reproducing again just as in the example from Comment #52. >i.e.:"Ubuntu 16.04 LTS (user X) - Windows 10 x64(user Y) - Video not working for Windows (web)." >- user Y not able to see user X, black screen displayed, sound works accordingly >- user X has no issues I used today's Aurora build (id: 20170321004003) on both Windows 10 x64 and MacOS X 10.11.6. On the Windows platform I can't see the stream sent from Mac. On the Mac side everything looks and sounds good. Setting the flags accordingly. Please ni? me for further information. P.S. This is only reproducible on Aurora builds.
Flags: needinfo?(rjesup)
(In reply to Alexandru Simonca, QA (:asimonca) from comment #53) > Created attachment 8849509 [details] > issue behavior on Mac vs. Windows 10 > > Hi everyone, > I've been testing WebRTC and this issue just started reproducing again just > as in the example from Comment #52. > > >i.e.:"Ubuntu 16.04 LTS (user X) - Windows 10 x64(user Y) - Video not working for Windows (web)." > >- user Y not able to see user X, black screen displayed, sound works accordingly > >- user X has no issues > > I used today's Aurora build (id: 20170321004003) on both Windows 10 x64 and > MacOS X 10.11.6. On the Windows platform I can't see the stream sent from > Mac. On the Mac side everything looks and sounds good. > Setting the flags accordingly. Please ni? me for further information. > > P.S. This is only reproducible on Aurora builds. I just tried this in Aurora 54 to AUrora 54 (Fedora x64 to Mac), creating the call from both directions worked fine. Can you retest? I can see any reason 54 would be different for this issue, and certainly no reason Mac would be different. Thanks
Flags: needinfo?(rjesup) → needinfo?(alexandru.simonca)
One of the peers has to be Windows. The bug is that the audio/video doesn't work on the Windows side.
Flags: needinfo?(alexandru.simonca)
Did things get sorted out here?
Flags: needinfo?(rjesup)
I was on PTO and haven't re-tested with windows 54 I'll look today
I'm not able to make a call between any platforms using http://web.ciscospark.com/, 55.0a1 (2017-05-04). Reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The issue in 55 might not be this one. We noted this issue: https://bugzilla.mozilla.org/show_bug.cgi?id=1361206 Still trying to figure out who misinterpreted the spec relating to the above. Even reading the spec a few times, people are left wondering what the authors meant. We're discussing the issue inside Cisco, as it might be Spark's problem.
Blocks: 1394602
(In reply to Paul Silaghi, QA [:pauly] from comment #58) > I'm not able to make a call between any platforms using > http://web.ciscospark.com/, 55.0a1 (2017-05-04). > Reopening. As Paul pointed out the root cause for this was probably bug 1361206. I recently placed calls over Cisco Spark and they worked fine, thus closing for now.
Status: REOPENED → RESOLVED
Closed: 8 years ago → 7 years ago
Resolution: --- → FIXED
status-firefox54: affected → ---
status-firefox55: affected → ---
Flags: needinfo?(rjesup)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: