Closed
Bug 1432793
Opened 7 years ago
Closed 7 years ago
Crash in mozalloc_abort | abort | webrtc::ViEEncoder::ReconfigureEncoder
Categories
(Core :: WebRTC, defect, P2)
Tracking
()
VERIFIED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox58 | --- | wontfix |
firefox59 | --- | wontfix |
firefox60 | --- | verified |
firefox61 | --- | verified |
People
(Reporter: marcia, Assigned: dminor)
References
(Blocks 3 open bugs)
Details
(Keywords: crash, regression)
Crash Data
Attachments
(4 files)
This bug was filed from the Socorro interface and is
report bp-f052cd3b-5369-4b4b-975c-597490180124.
=============================================================
Spotted while looking at nightly Mac and Linux crash data: http://bit.ly/2DzYYHI. Not super high volume, but affects 58/59/60.
Top 10 frames of crashing thread:
0 libmozglue.dylib mozalloc_abort memory/mozalloc/mozalloc_abort.cpp:33
1 libmozglue.dylib abort memory/mozalloc/mozalloc_abort.cpp:80
2 XUL rtc::FatalMessage::~FatalMessage media/webrtc/trunk/webrtc/base/checks.cc:109
3 XUL rtc::FatalMessage::~FatalMessage media/webrtc/trunk/webrtc/base/checks.cc:102
4 XUL webrtc::ViEEncoder::ReconfigureEncoder media/webrtc/trunk/webrtc/video/vie_encoder.cc:424
5 XUL webrtc::ViEEncoder::EncodeVideoFrame media/webrtc/trunk/webrtc/video/vie_encoder.cc:553
6 XUL webrtc::ViEEncoder::EncodeTask::Run media/webrtc/trunk/webrtc/video/vie_encoder.cc:105
7 XUL rtc::TaskQueue::TaskContext::RunTask media/webrtc/trunk/webrtc/base/task_queue_gcd.cc:53
8 @0x7fff730cdd4f
9 @0x7fff730e220b
=============================================================
Comment 1•7 years ago
|
||
P2 for being low rate but unexpected until we know more.
Randell, weren't you looking at just this code recently?
Rank: 15
Flags: needinfo?(rjesup)
Priority: -- → P2
Comment 2•7 years ago
|
||
I changed the RTC_DCHECK() into an RTC_CHECK() (release assert), which triggered this. Likely these configs were failing before -- and the code would happily continue after failing to init the codec (which means no media would flow, and in some cases it might fail elsewhere. Also, if it was vp8 failing to init, in some cases it might have been more problematic than this, so RTC_CHECK was a safety.
Knowing more about what caused it would help a ton (codec, config, etc)
Interestingly, only crashing on Linux and Mac, not windows. And most but not all appear to be Hangouts (one meet.jit.si). One is repeated a few hours apart, which implies a repeatable action caused it (screensharing, or some resolution change triggering a reconfig, etc). Looks more like a reconfig in-call than a start-of-stream config (screensharing), but that's very much a guess.
Flags: needinfo?(rjesup)
Updated•7 years ago
|
Comment 3•7 years ago
|
||
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:58.0) Gecko/20100101 Firefox/58.0
This is 100% reproducible for us, using the following steps:
[Steps to reproduce]:
1. Login to "hangouts.google.com".
2. Click the "Video Call" button to start a video call.
3. Click on the "three dots" button at the top right part of the screen and choose the "Share Screen" option.
4. Choose the "Full Screen" option and observe.
[Crash Reports]:
Firefox: https://crash-stats.mozilla.com/report/index/e496541a-c166-4916-bf71-fee0b1180306
Nightly: https://crash-stats.mozilla.com/report/index/42e2abcb-d89b-4c69-8b8f-423cd0180306
[Notes]:
- The issue is not reproducible if you receive a shared screen through the video call session.
- Attached about:support info: https://pastebin.mozilla.org/9079241
Updated•7 years ago
|
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → dminor
Assignee | ||
Comment 4•7 years ago
|
||
(In reply to Emil Pasca [:emilpasca], Desktop Engineering QA from comment #3)
> User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:58.0) Gecko/20100101
> Firefox/58.0
>
> This is 100% reproducible for us, using the following steps:
>
> [Steps to reproduce]:
> 1. Login to "hangouts.google.com".
> 2. Click the "Video Call" button to start a video call.
> 3. Click on the "three dots" button at the top right part of the screen and
> choose the "Share Screen" option.
> 4. Choose the "Full Screen" option and observe.
>
> [Crash Reports]:
> Firefox:
> https://crash-stats.mozilla.com/report/index/e496541a-c166-4916-bf71-
> fee0b1180306
> Nightly:
> https://crash-stats.mozilla.com/report/index/42e2abcb-d89b-4c69-8b8f-
> 423cd0180306
>
> [Notes]:
> - The issue is not reproducible if you receive a shared screen through the
> video call session.
> - Attached about:support info: https://pastebin.mozilla.org/9079241
Hi Emil,
I'm having difficulty reproducing this on Firefox/60.0 on Linux. I was wondering if it would be possible for you to try again using Nightly? There have been a number of screensharing issues with hangouts and I wanted to see if you were possibly hitting one in 58 that has subsequently been fixed in 60 which would explain why this is not 100% reproducible for me. Thank you,
Dan
Flags: needinfo?(emil.pasca)
Comment 5•7 years ago
|
||
Hi Dan,
I've just retested the issue on Ubuntu 16.04 x64 with the latest Nightly (60.0a1-20180307220100) with the same STR and the tab crashes, here's the crash report:
https://crash-stats.mozilla.com/report/index/12d724c4-e1ba-48c4-8202-9608d0180308
I would like to mention that I've tested this on a lower-end machine, I've added the about:support info below if it helps.
If there's anything else you would like me to test in order to help investigate this further, do let me know.
Application Basics
------------------
Name: Firefox
Version: 60.0a1
Build ID: 20180307220100
Update Channel: nightly
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/60.0
OS: Linux 4.4.0-116-generic
Multiprocess Windows: 1/1 (Enabled by default)
Web Content Processes: 3/4
Stylo: content = true (enabled by default), chrome = true (enabled by default)
Enterprise Policies: Inactive
Google Key: Found
Mozilla Location Service Key: Found
Safe Mode: false
Crash Reports for the Last 3 Days
---------------------------------
Report ID: bp-12d724c4-e1ba-48c4-8202-9608d0180308
Submitted: 1 minute ago
All Crash Reports
Nightly Features
----------------
Name: Activity Stream
Version: 2018.03.01.1281-6a7c8294
ID: activity-stream@mozilla.org
Name: Application Update Service Helper
Version: 2.0
ID: aushelper@mozilla.org
Name: Firefox Screenshots
Version: 30.0.0
ID: screenshots@mozilla.org
Name: Follow-on Search Telemetry
Version: 0.9.6
ID: followonsearch@mozilla.com
Name: Form Autofill
Version: 1.0
ID: formautofill@mozilla.org
Name: Photon onboarding
Version: 1.0
ID: onboarding@mozilla.org
Name: Pocket
Version: 1.0.5
ID: firefox@getpocket.com
Name: Presentation
Version: 1.0.0
ID: presentation@mozilla.org
Name: Web Compat
Version: 1.1
ID: webcompat@mozilla.org
Name: WebCompat Reporter
Version: 1.0.0
ID: webcompat-reporter@mozilla.org
Extensions
----------
Security Software
----------------- Type:
Type:
Type:
Graphics
--------
Features
Compositing: Basic
Asynchronous Pan/Zoom: wheel input enabled; scrollbar drag enabled; keyboard enabled; autoscroll enabled
WebGL 1 Driver WSI Info: GLX 1.4 GLX_VENDOR(client): Mesa Project and SGI GLX_VENDOR(server): SGI Extensions: GLX_ARB_create_context GLX_ARB_create_context_profile GLX_ARB_create_context_robustness GLX_ARB_fbconfig_float GLX_ARB_framebuffer_sRGB GLX_ARB_get_proc_address GLX_ARB_multisample GLX_EXT_import_context GLX_EXT_visual_info GLX_EXT_visual_rating GLX_EXT_fbconfig_packed_float G
Flags: needinfo?(emil.pasca)
Assignee | ||
Comment 6•7 years ago
|
||
Hi Emil,
Thanks for your help on this. Unfortunately, I'm still having problems getting this to reproduce. I've tried on a few different systems without much luck.
I've added some extra debugging information and pushed to try here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7cb676f46f1b442e7335372ced8c434df4b379cb
You can grab an archive of that version of firefox from here:
https://queue.taskcluster.net/v1/task/OqVVOK_CQyCFNnM7N8qowA/runs/0/artifacts/public/build/target.tar.bz2
I was wondering if you could try using that version and seeing if it reproduces for you? I've added the extra debugging as printf statements, so you should just be able to copy+paste the console output after it runs and attach it to the bug.
The other thing that would be helpful would be grabbing a copy of about:webrtc from before you start sharing your screen.
Thank you for your help!
Dan
Flags: needinfo?(emil.pasca)
Comment 7•7 years ago
|
||
I have retested this issue on the provided try-build from comment 6. The issue is still reproducible, below are the Browser Console error messages.
I've also tried to provide the WebRTC log from about:webrtc, however, after starting the debugger and stopping it, the about:webrtc page is empty, no stats are loaded in the page and also when navigating to "temp/WebRTC.log", the log is empty. Am I missing a step or something?
As a side note, the issue is reproducible also when using a meet call and attempt to share just a window, rather than the entire desktop.
Browser Console error messages on Ubuntu 16.04 x64 ------------------------
this is undefined UTEventReporting.jsm:51
Content Security Policy: Ignoring “'unsafe-inline'” within script-src: ‘strict-dynamic’ specified (unknown)
Content Security Policy: Ignoring “https:” within script-src: ‘strict-dynamic’ specified (unknown)
Content Security Policy: Ignoring “http:” within script-src: ‘strict-dynamic’ specified (unknown)
WARNING! rs=AL5CKSGKzmqFOQsi1kxEMltKH41GGPGj9Q:278:242
Using this console may allow attackers to impersonate you and steal your information using an attack called Self-XSS.
Do not enter or paste code that you do not understand. rs=AL5CKSGKzmqFOQsi1kxEMltKH41GGPGj9Q:278:242
Attempt to set a forbidden header was denied: Connection 702790028-lcs_client_bin.js:99:385
Attempt to set a forbidden header was denied: Connection 702790028-lcs_client_bin.js:99:385
Content Security Policy: Ignoring ‘x-frame-options’ because of ‘frame-ancestors’ directive. (unknown)
The character encoding of a framed document was not declared. The document may appear different if viewed without the document framing it. hscv
URL.createObjectURL(MediaStream) is deprecated and will be removed soon. webrtcUI.jsm:674:24
browser.ownerGlobal is null ContentCrashHandlers.jsm:178
Browser Console error messages on Debian 4.9.0-4-amd64 ------------------------------
Content Security Policy: Ignoring “'unsafe-inline'” within script-src: ‘strict-dynamic’ specified (unknown)
Content Security Policy: Ignoring “https:” within script-src: ‘strict-dynamic’ specified (unknown)
Content Security Policy: Ignoring “http:” within script-src: ‘strict-dynamic’ specified (unknown)
Attempt to set a forbidden header was denied: Connection 702790028-lcs_client_bin.js:99:385
Attempt to set a forbidden header was denied: Connection 702790028-lcs_client_bin.js:99:385
Content Security Policy: Ignoring ‘x-frame-options’ because of ‘frame-ancestors’ directive. (unknown)
WARNING! rs=AL5CKSGKzmqFOQsi1kxEMltKH41GGPGj9Q:278:242
Using this console may allow attackers to impersonate you and steal your information using an attack called Self-XSS.
Do not enter or paste code that you do not understand. rs=AL5CKSGKzmqFOQsi1kxEMltKH41GGPGj9Q:278:242
The character encoding of a framed document was not declared. The document may appear different if viewed without the document framing it. hscv
URL.createObjectURL(MediaStream) is deprecated and will be removed soon. webrtcUI.jsm:674:24
browser.ownerGlobal is null
Flags: needinfo?(emil.pasca)
Assignee | ||
Comment 8•7 years ago
|
||
Hi Emil,
Sorry, I wasn't looking for the Browser Console output, but the Linux terminal output, so if you open the terminal on Ubuntu, run firefox from the command line, the output that you see there is what I'm looking for. I should have been more clear - I realize you probably get a lot more people asking for the browser console.
As for about:webrtc, it makes sense that would be empty. When I asked, I wasn't sure if another person had to be in the call for this to reproduce, but I've since learned that it will reproduce when you share the screen even if no one else is on the call.
This has crashed for me now, once on meet.google.com, and once on hangouts. I think the problem is we're returning an error from this function [1] but as you can see, there are lot of possible things that would cause an error there and without it crashing for me consistently it's hard for me to determine which one is the problem.
Thank you for your help,
Dan
[1] https://searchfox.org/mozilla-central/rev/a70da6775d5341a9cca86bf1572a5e3534909153/media/webrtc/trunk/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc#290
Flags: needinfo?(emil.pasca)
Assignee | ||
Comment 9•7 years ago
|
||
nevermind, finally consistently reproducing for me!
Flags: needinfo?(emil.pasca)
Assignee | ||
Comment 10•7 years ago
|
||
The failure I'm seeing is here [1] while sharing a window on meet.google.com. It's checking aspect ratios. I'm guessing I had problems getting this to reproduce because I was choosing windows that had an aspect ratio which happened to result in working simulcast resolutions.
[1] https://searchfox.org/mozilla-central/rev/a70da6775d5341a9cca86bf1572a5e3534909153/media/webrtc/trunk/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc#88
Comment 11•7 years ago
|
||
I think my (still local) patches on bug 1253499 would help with this. The ones attached to the bug don't handle simulcast layers, but I think I have some that do lying around. The way we choose resolutions for simulcast right now is... rudimentary. Something like `layer.width = width << layer.index` and likewise for height. So with an odd width or height your aspect ratio becomes off and we'll hit this assert.
Comment 12•7 years ago
|
||
Interesting about the aspect ratio. That would explain why it more frequently triggers with windows than desktops. I have been trying to change the whole system to disable simulcast while screencasting, without much success. Screencast content is basically unviewable if pre-scaled.
Assignee | ||
Comment 13•7 years ago
|
||
I'm not sure why we have our own code for choosing simulcast resolutions rather than using what is available in [1] which presumably is what is used by Chrome. The interesting bit there is that simulcast layers while screensharing are limited to 1 or 2 (if an experiment is enabled) in the version of the webrtc.org code we're currently using, presumably to avoid aspect ratio issues like we're seeing.
I'm planning to limit the number of simulcast streams to those which result in valid resolutions as a workaround for the current problem. Longer term, I think it would make sense to borrow what we can from [1] rather than maintaining our own algorithm which I don't think is getting us much in this case.
[1] https://searchfox.org/mozilla-central/source/media/webrtc/trunk/webrtc/media/engine/simulcast.cc
Assignee | ||
Comment 14•7 years ago
|
||
I investigated using the cricket::GetSimulcastConfig function I mentioned above. That will definitely work, but it offers a much more restrictive model than what we currently have. For instance, the only allow a fixed set of simulcast resolutions [1] and I had to add a row to that table to make our unit tests work with two streams at a 50x50 resolution. That also throws away a lot of the flexibility that we allow during negotation, e.g. a lot of the fields at [2] would end up being ignored. The advantage of using GetSimulcastConfig is that we would no longer be generating configurations that cause assertions elsewhere in the video encoding stack. We'd also match the behaviour of Chrome which might be helpful for developers. The checks in the vp8 encoder limit how far we can diverge from the webrtc.org simulcast behaviour anyway.
The other approach I've looked into is to limit the number of layers to prevent problems. If I only allow a single simulcast layer when screensharing, and limit the number of layers so that we don't attempt to create a smaller resolution layer from a layer with odd dimensions then that fixes the problems with screensharing simulcast on meet.google.com. Interestingly, just limiting to a single simulcast layer when screensharing is not sufficient to fix the problem as it will crash when switching back from screenshare to camera, it looks like the request is made for camera but with the dimensions from the previous window being shared. And, as Andreas pointed out, allowing window sharing with more than one layer will crash if someone resizes the window to have an odd dimension.
I don't have the background on the decisions made during our implementation of simulcast to have a strong opinion on the best way ahead here. Maybe there's a better fix that I haven't considered. :jib, :perhsons, thoughts on this? Thanks!
[1] https://searchfox.org/mozilla-central/rev/877c99c523a054419ec964d4dfb3f0cadac9d497/media/webrtc/trunk/webrtc/media/engine/simulcast.cc#43
[2] https://searchfox.org/mozilla-central/rev/877c99c523a054419ec964d4dfb3f0cadac9d497/media/webrtc/signaling/src/common/EncodingConstraints.h#14
Flags: needinfo?(jib)
Flags: needinfo?(apehrson)
Comment 15•7 years ago
|
||
Unfortunately I don't know much of our simulcast impl history. Jesup maybe?
Flags: needinfo?(apehrson) → needinfo?(rjesup)
Comment 16•7 years ago
|
||
Since Byron has spend quite some time on our simulcast implementation he might be able to contribute information here.
Flags: needinfo?(docfaraday)
Comment 17•7 years ago
|
||
(In reply to Dan Minor [:dminor] from comment #10)
> The failure I'm seeing is here [1] while sharing a window on
> meet.google.com. It's checking aspect ratios. I'm guessing I had problems
> getting this to reproduce because I was choosing windows that had an aspect
> ratio which happened to result in working simulcast resolutions.
>
> [1]
> https://searchfox.org/mozilla-central/rev/
> a70da6775d5341a9cca86bf1572a5e3534909153/media/webrtc/trunk/webrtc/modules/
> video_coding/codecs/vp8/vp8_impl.cc#88
Back when I first worked on this stuff, I had code that would ensure that we preserved the aspect ratio exactly, but it appears to have been commented out during the 57 update:
https://searchfox.org/mozilla-central/source/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp#612
Flags: needinfo?(docfaraday)
Assignee | ||
Comment 18•7 years ago
|
||
I think the best way ahead here is to switch to using cricket::GetSimulcastConfig. We lose some flexibility but gain safety and consistency with Chrome. I'll clean up and post my patches for review shortly.
Updated•7 years ago
|
Assignee | ||
Comment 20•7 years ago
|
||
(In reply to Dan Minor [:dminor] from comment #18)
> I think the best way ahead here is to switch to using
> cricket::GetSimulcastConfig. We lose some flexibility but gain safety and
> consistency with Chrome. I'll clean up and post my patches for review
> shortly.
I discussed this again with Nils and it looks like we are looking for something that we can easily uplift to 60 for hangouts/meet, so I'm going to go with the workaround instead. I'll file a separate bug for switching to cricket::GetSimulcastConfig.
Flags: needinfo?(rjesup)
Assignee | ||
Comment 21•7 years ago
|
||
(In reply to Byron Campen [:bwc] from comment #17)
> (In reply to Dan Minor [:dminor] from comment #10)
> > The failure I'm seeing is here [1] while sharing a window on
> > meet.google.com. It's checking aspect ratios. I'm guessing I had problems
> > getting this to reproduce because I was choosing windows that had an aspect
> > ratio which happened to result in working simulcast resolutions.
> >
> > [1]
> > https://searchfox.org/mozilla-central/rev/
> > a70da6775d5341a9cca86bf1572a5e3534909153/media/webrtc/trunk/webrtc/modules/
> > video_coding/codecs/vp8/vp8_impl.cc#88
>
> Back when I first worked on this stuff, I had code that would ensure that we
> preserved the aspect ratio exactly, but it appears to have been commented
> out during the 57 update:
>
> https://searchfox.org/mozilla-central/source/media/webrtc/signaling/src/
> media-conduit/VideoConduit.cpp#612
The comments in VideoConduit suggest that each layer must both have the same aspect ratio and also be exactly half the size of the previous layer. As far as I can tell, this size constraint is not actually enforced by branch 57. I did a quick experiment hard coding some layers to be a third of the previous layer's size and that worked fine on meet.google.com.
That said, the webrtc.org code in branch 57 always generates layers that are half the size of the preceding one when not screensharing [1] and when we update to branch 64, a check will be added that each layer is half the size of the previous one in the vp8 encoder [2], so I don't think re-enabling this code gains us much at this point.
Resizing a shared window does not trigger another call to CreateEncoderStreams so using ConstrainPreservingAspectRatioExact won't prevent us from crashing if the user resizes the window to an odd dimension after it is shared.
[1] https://searchfox.org/mozilla-central/source/media/webrtc/trunk/webrtc/media/engine/simulcast.cc#234
[2] https://cs.chromium.org/chromium/src/third_party/webrtc/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc?q=ValidSimu&sq=package:chromium&l=90
Assignee | ||
Comment 22•7 years ago
|
||
This is how I reproduce this:
1. Login to hangouts.google.com using your mozilla ldap account.
2. Go to meet.google.com, start a meeting. (I was not able to use meet from Firefox unless I was authenticated using my ldap account.)
3. Select presentation, share a window.
4. Resize the window a few times.
5. Stop presenting and switch back to camera.
Unpatched Firefox will crash at 3 or 4 depending upon the resolution of the window being shared. I also hit crashes at 5 with an earlier version of the patch.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 24•7 years ago
|
||
I'll write a mochitest for this as well, but I thought it made sense to get the patch reviewed since it is holding up hangouts/meet and is easily verifiable without a test.
Flags: in-testsuite?
Assignee | ||
Comment 25•7 years ago
|
||
Comment 26•7 years ago
|
||
mozreview-review |
Comment on attachment 8964841 [details]
Bug 1432793 - Force screensharing simulcast to one layer and stop generating layers once an odd width and height are found;
https://reviewboard.mozilla.org/r/233592/#review239244
::: media/webrtc/signaling/src/media-conduit/VideoConduit.cpp:589
(Diff revision 1)
> + // Disallow odd width and height, they will cause aspect ratio checks to
> + // fail in the webrtc.org code. We can hit transient states after window
> + // sharing ends where odd resolutions are requested for the camera.
> + for (size_t i = 1; i < streamCount; ++i) {
> + int ith_width = width >> (i - 1);
> + int ith_height = height >> (i - 1);
> + if (ith_width % 2 == 1 || ith_height % 2 == 1) {
> + streamCount = i;
> + break;
> + }
> + }
See if you can use this:
https://searchfox.org/mozilla-central/source/mfbt/MathAlgorithms.h#328
If you have to monkey around with include paths in the build system, it probably isn't worth doing, since (hopefully) this code won't need to stick around for long.
Attachment #8964841 -
Flags: review?(docfaraday) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 30•7 years ago
|
||
Pushed by dminor@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/847e9f622eb7
Force screensharing simulcast to one layer and stop generating layers once an odd width and height are found; r=bwc
Comment 31•7 years ago
|
||
Backout by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/60a9050fb70f
Backed out changeset 847e9f622eb7 for build bustages on VideoConduit.cpp. CLOSED TREE
Comment 32•7 years ago
|
||
mozreview-review |
Comment on attachment 8965011 [details]
Bug 1432793 - Add mochitest for odd simulcast resolutions;
https://reviewboard.mozilla.org/r/233754/#review239464
Attachment #8965011 -
Flags: review?(docfaraday) → review+
Comment 33•7 years ago
|
||
mozreview-review |
Comment on attachment 8965012 [details]
Bug 1432793 - Add gtests for simulcast screenshare and odd resolutions;
https://reviewboard.mozilla.org/r/233756/#review239462
r+ with fixes
::: media/webrtc/signaling/gtest/videoconduit_unittests.cpp:745
(Diff revision 1)
> + EncodingConstraints constraints;
> + VideoCodecConfig::SimulcastEncoding encoding;
> + VideoCodecConfig::SimulcastEncoding encoding2;
> + VideoCodecConfig::SimulcastEncoding encoding3;
> + encoding2.constraints.scaleDownBy = 2;
> + encoding2.constraints.scaleDownBy = 4;
encoding3?
::: media/webrtc/signaling/gtest/videoconduit_unittests.cpp:768
(Diff revision 1)
> + EncodingConstraints constraints;
> + VideoCodecConfig::SimulcastEncoding encoding;
> + VideoCodecConfig::SimulcastEncoding encoding2;
> + VideoCodecConfig::SimulcastEncoding encoding3;
> + encoding2.constraints.scaleDownBy = 2;
> + encoding2.constraints.scaleDownBy = 4;
encoding3?
Attachment #8965012 -
Flags: review?(docfaraday) → review+
Assignee | ||
Comment 34•7 years ago
|
||
Try run with build fix and tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e771797133e81efcec824f6231dcae9b16baf948
Comment 35•7 years ago
|
||
Pushed by dminor@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c5a16ba81b7
Force screensharing simulcast to one layer and stop generating layers once an odd width and height are found; r=bwc
https://hg.mozilla.org/integration/mozilla-inbound/rev/f1929e2b77a9
Add mochitest for odd simulcast resolutions; r=bwc
https://hg.mozilla.org/integration/mozilla-inbound/rev/620a4eba9389
Add gtests for simulcast screenshare and odd resolutions; r=bwc
Assignee | ||
Comment 36•7 years ago
|
||
Comment on attachment 8964841 [details]
Bug 1432793 - Force screensharing simulcast to one layer and stop generating layers once an odd width and height are found;
Approval Request Comment
[Feature/Bug causing the regression]:
I believe this has been a problem since we first implemented simulcast (Bug 1210170).
[User impact if declined]:
This is blocking hangouts/meet supporting Firefox and causes a browser crash.
[Is this code covered by automated tests?]:
New tests have been added in the other patches on this bug.
[Has the fix been verified in Nightly?]:
My push looks fine on inbound but has not yet been merged into central.
[Needs manual test from QE? If yes, steps to reproduce]:
Yes, see comment 22.
[List of other uplifts needed for the feature/fix]:
None.
[Is the change risky?]:
No.
[Why is the change risky/not risky?]:
This only impacts simulcast with odd resolutions. Since cameras and screens typically have even resolutions, this should only change behaviour when window sharing. Since window sharing simulcast was basically completely broken anyway, this should not make things worse.
[String changes made/needed]:
Attachment #8964841 -
Flags: approval-mozilla-beta?
Comment 37•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5c5a16ba81b7
https://hg.mozilla.org/mozilla-central/rev/f1929e2b77a9
https://hg.mozilla.org/mozilla-central/rev/620a4eba9389
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Assignee | ||
Updated•7 years ago
|
Flags: in-testsuite? → in-testsuite+
Comment 38•7 years ago
|
||
Comment on attachment 8964841 [details]
Bug 1432793 - Force screensharing simulcast to one layer and stop generating layers once an odd width and height are found;
Fix needed for Hangouts/Meet. Approved for 60.0b11.
Attachment #8964841 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 39•7 years ago
|
||
bugherder uplift |
Updated•7 years ago
|
Flags: qe-verify+
Comment 40•7 years ago
|
||
The issue is still reproducible on Windows 10 (x64) and macOS 10.12 using latest Nightly (2018-04-11) and Firefox 60.0b11 using the following steps:
Precondition:
Make sure a folder is opened in background and not minimized (it will be needed later)
Steps:
1. Start Firefox.
2. Login to hangouts.google.com.
3. Go to meet.google.com (you have to be logged in with mozilla ldap account, or else it will say that the page is unsupported on Firefox).
4. Click on Start a new meeting.
5. Click on Start Meeting.
6. Click on Present Now and select a Window.
7. The folder from step 1 will be selected for screenshare.
8. After the screenshare started, minimize the folder.
On Ubuntu 16.04 (x64) this issue is not reproducible.
Here is a crash log from macOS: https://crash-stats.mozilla.com/report/index/04613a3b-74fd-4a96-8870-f21120180412#tab-details
Hi Dan, can you have a look into this?
Should I file a new bug for this issue?
I attached a screencast with the issue too.
Flags: needinfo?(dminor)
Assignee | ||
Comment 41•7 years ago
|
||
Hi Sascha,
I'll look into this right away. Since this doesn't affect Ubuntu, I suspect this is a separate, but closely related bug. The crash signature just means that reconfiguring the encoder has failed but unfortunately gives no information on what went wrong. Thank you for catching this.
Flags: needinfo?(dminor)
Assignee | ||
Comment 42•7 years ago
|
||
I filed Bug 1453740 for this. A minimized window has a resolution of 1x1 pixels, which causes a separate check in the vp8 codec to fail.
Assignee | ||
Updated•7 years ago
|
Blocks: Screensharing
Comment 43•7 years ago
|
||
Since this bug is still reproducible, and it depends on Bug 1453740, I cannot verify it until it is fixed.
Depends on: 1453740
Comment 44•7 years ago
|
||
This issue is no longer reproduced by following the steps mentioned in Comment 22 and Bug 1453740 was verified fixed.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in
before you can comment on or make changes to this bug.
Description
•