1487419 - Share screen doesn't work on Mac with getUserMedia Test Page with multiple monitors

Status: VERIFIED FIXED

(Core :: WebRTC: Audio/Video, defect, P2)

Description

[ovidiu boca[:Ovidiu]]

[Affected versions]:

Nightly 63.0a1
Beta 62

[Affected platforms]:

macOS 10.13

Prerequisites: You need to have 2 monitors

[Steps to reproduce]:

1. Open Firefox
2. Go to https://mozilla.github.io/webrtc-landing/gum_test.html
3. Select "Screen"
4. From the "Screen to share" drop-down select Screen 1 

[Expected result]:
The image from the Screen 1 is captured. 

[Actual result]:
The image from both screens is captured.


Note: This issue is a regression, I run mozregression and here is the push log: 


ast good revision: 89b72c9c5db5dc38c0dd6547eac9b1b1a777c53f
29:20.48 INFO: First bad revision: 0708df4d3b7a8c38dd32f52d3199e5a27a0758f2
29:20.48 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=89b72c9c5db5dc38c0dd6547eac9b1b1a777c53f&tochange=0708df4d3b7a8c38dd32f52d3199e5a27a0758f2

From what I see I think the fix from bug 1409018 created this regression. 
Also when I see the 2 screens the capture is not updated only from on 1 screen.

Comment 1

[ovidiu boca[:Ovidiu]]

Dan please take a look at this and tell me your opinion.

Comment 2

[Dan Minor [:dminor]]

Thank you for your report!

I tested this just now, and I'm seeing the following:
* with no screen attached, the screen share works and updates normally
* with an external screen attached and the display mirrored, screen share works and updates normally
* with an external screen attached an no mirroring, regardless of whether I pick Screen 1 or Screen 2 in the drop down, I always get the laptop screen (Screen 1), not the external screen. Sharing that screen works and updates normally.

For reference, I tested using a 2015 macbook with a retina display, running 10.13.1, and I used my tv as the second monitor.

I'm adjusting the bug title as the problem only seems to occur with multiple monitors attached. Please let me know if you are also seeing problems with just a single screen.

Since I'm not getting the exact same problem you are seeing, I was wondering if you could tell me a bit more about your test system. By two monitors, do you mean a laptop screen and a monitor, or two monitors connected to a desktop, or something else? How do you have your two monitors configured in System Settings? What are the resolutions of your two monitors? Thanks!

Comment 3

[ovidiu boca[:Ovidiu]]

I did a screen recording of the issue, https://streamable.com/fffm6
I have an iMac (21.5-inch, Late 2013) with an extra monitor S22C300 Display (Samsung), my Graphic card is Intel Iris Pro 1536 MB. The resolution of the monitors: iMac - 1920x1080   Samsung monitor 1080p, this is the arrangement of the monitors: https://imgur.com/a/6gA1t5u
From the screen recording, you can see that both monitors are displayed even if I select screen 1 or screen2. 
Please tell me if there is something else that I can do.

Comment 4

[ovidiu boca[:Ovidiu]]

I also tested this on a MacBook Pro Mac OS X 10.13 (13-inch,2017, Two Thunderbolt 3 ports) with an extra monitor Samsung S22C300 (HDMI cable) and the arrangement of the screens is this one: https://imgur.com/a/DL7eSU5 . and it doesn't matter what screen I choose from the drop down (1 or 2) the sharing is performed only for Samsung screen.

Comment 5

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

dan - can you try again?  Particularly the cabling setup, or something close.  This is shipping in 62 and isn't a stop-ship, but we should try to fix and uplift to 63 at least.

Comment 6

[Dan Minor [:dminor]]

I have a simple fix for this, but it's not clear to me why it is required. I'm going spend a bit more time on this before putting it up for review.

Comment 7

[Dan Minor [:dminor]]

Attachment: Bug 1487419 - Save current desktop configuration in ScreenCapturerMac::Init; r=jesup

The fix for Bug 1409018 accidentally removed saving the current desktop
configuration during Init() which causes it to not be set when a different
screen is selected, meaning that regardless of the choice made, only the
first screen is captured.

Comment 8

[Dan Minor [:dminor]]

This fixes the problem I'm able to test directly (laptop + one external screen). I think it will also fix problems with two monitors, but I don't have the hardware to test that.

Comment 9

[ovidiu boca[:Ovidiu]]

I can help you with that testing, please let me know when I can start.

Comment 10

[Andreas Pehrson [:pehrsons]]

Comment on attachment 9006646 [details]
Bug 1487419 - Save current desktop configuration in ScreenCapturerMac::Init; r=jesup

Andreas Pehrson [:pehrsons] has approved the revision.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0700ed6720e5
Save current desktop configuration in ScreenCapturerMac::Init; r=pehrsons

Comment 12

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/0700ed6720e5

Comment 13

[Andreas Pehrson [:pehrsons]]

I've added this as a known issue to the webrtc release notes for 62, https://wiki.mozilla.org/Media/WebRTC/ReleaseNotes/62

Comment 14

[Dan Minor [:dminor]]

(In reply to ovidiu boca[:Ovidiu] from comment #9)
> I can help you with that testing, please let me know when I can start.

This should be on the latest nightly, it would be great if you could test it with your setup. Thanks!

Comment 15

[ovidiu boca[:Ovidiu]]

Thanks Dan for reminding me about this.

I run an exploratory test around this issue on Mac OS X 10.12 with FF Nightly 64.0a1(2018-09-12) and I can't reproduce the initial problem, now the capture of the screens is working. I also found an issue, the quality of the capture from screen 2, not the default iMac monitor, it has some problems, please see the video: https://streamable.com/g6ytn
Please tell me your opinion and also how should we treat this issue, new bug or reopen this one?

Comment 16

[Dan Minor [:dminor]]

(In reply to ovidiu boca[:Ovidiu] from comment #15)
> Thanks Dan for reminding me about this.
> 
> I run an exploratory test around this issue on Mac OS X 10.12 with FF
> Nightly 64.0a1(2018-09-12) and I can't reproduce the initial problem, now
> the capture of the screens is working. I also found an issue, the quality of
> the capture from screen 2, not the default iMac monitor, it has some
> problems, please see the video: https://streamable.com/g6ytn
> Please tell me your opinion and also how should we treat this issue, new bug
> or reopen this one?

Thank you for testing this. I think we should open a new bug for the screen capture quality issue, my first guess is that it is not related to the problem here, so I think it makes sense to investigate it separately.

It might be related to Bug 1409018, do you have time to do a quick regression window around when that landed? I did not see that problem when I tested the fix for this bug, so I'm not sure if I will be able to reproduce it, unless this is a recent regression that would not have shown up last week.

Comment 17

[Dan Minor [:dminor]]

Of course you can't do a regression window, this bug would stop you from being able to see if the problem started at that time.

Comment 18

[ovidiu boca[:Ovidiu]]

I'll put an NI? for myself to do a regression.

Comment 19

[Pascal Chevrel:pascalc]

Dan, should we consider it safe for a 63 beta uplift or can we let it ride the trains with 64? Thanks

Comment 20

[Dan Minor [:dminor]]

Comment on attachment 9006646 [details]
Bug 1487419 - Save current desktop configuration in ScreenCapturerMac::Init; r=jesup

Approval Request Comment
[Feature/Bug causing the regression]:
Bug 1409018
[User impact if declined]:
User is unable to select second monitor for screen capture in OS X
[Is this code covered by automated tests?]:
No, we don't have multiple monitors in automation :/
[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]:
None
[Is the change risky?]:
No
[Why is the change risky/not risky?]:
This is a one line fix that restores previous functionality.
[String changes made/needed]:
None.

Comment 21

[Dan Minor [:dminor]]

(In reply to Pascal Chevrel:pascalc from comment #19)
> Dan, should we consider it safe for a 63 beta uplift or can we let it ride
> the trains with 64? Thanks

Yes, we want this on beta if at all possible. Thanks for reminding me!

Comment 22

[Pascal Chevrel:pascalc]

Comment on attachment 9006646 [details]
Bug 1487419 - Save current desktop configuration in ScreenCapturerMac::Init; r=jesup

Low-risk patch fixing a regression, uplift approved for 63 beta 6. Thanks

Comment 23

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/releases/mozilla-beta/rev/feca63da8d4f

Comment 24

[ovidiu boca[:Ovidiu]]

I verified this issue on Mac OS X 10.12 with the latest FF Nightly 64.0a1(2018-09-13) and FF beta 63.0b6 and I can't reproduce the issue from the description based on that I will mark this as verified fixed. 
Please note that the issue described in comment 15 is still reproducilbe. I will try to find a regression range and file a new bug for it.

Comment 25

[Julien Cristau [:jcristau]]

Do you think this is safe/important enough as a ride-along in 62.0.2?

Comment 26

[Dan Minor [:dminor]]

(In reply to Julien Cristau [:jcristau] from comment #25)
> Do you think this is safe/important enough as a ride-along in 62.0.2?

Yes, that would be great!

Comment 27

[Julien Cristau [:jcristau]]

Comment on attachment 9006646 [details]
Bug 1487419 - Save current desktop configuration in ScreenCapturerMac::Init; r=jesup

OK let's take this for 62.0.2 based on comments 20 and 26.

Comment 28

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-release/rev/5e2fe8c6f242

Comment 29

[ovidiu boca[:Ovidiu]]

I tested this on Mac OS X 10.12 with FF 62.0.2 release candidate and I can confirm the fix.