Issue metadata
Sign in to add a comment
|
Desktop stream on Mac is really buggy.
Reported by
n...@appkat.com,
Dec 18 2016
|
||||||||||||||||||||||
Issue description
UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.28 Safari/537.36
Steps to reproduce the problem:
1. Get the screen stream.
chrome.desktopCapture.chooseDesktopMedia(["screen"], null, function(streamId) {
if(streamId) {
navigator.webkitGetUserMedia({
audio: false,
video: {
mandatory: {
chromeMediaSource: "desktop",
chromeMediaSourceId: streamId,
minWidth: 1280,
maxWidth: 1280,
minHeight: 760,
maxHeight: 760,
minFrameRate: 3,
maxFrameRate: 3
},
}
2. Show the screen stream in a video tag.
The stream looks messed up, especially in the top half of the screen.
Works fine on Windows. Broken on Mac.
What is the expected behavior?
Stream should be clean.
What went wrong?
The stream has a weird line through it and the screen isn't rendered correctly, even though it looks fine in the browser, the streams shows it as if it's still rendering.
Did this work before? Yes
Does this work in other browsers? Yes
Chrome version: 56.0.2924.28 Channel: beta
OS Version: OS X 10.11.6
Flash Version: Shockwave Flash 24.0 r0
,
Dec 19 2016
Still on Mac. Works a lot smoother when doing one window, but if doing entire screen, the issue shows up again.
,
Dec 19 2016
,
Dec 19 2016
Could you please provide a sample testcase/HTML file to reproduce the issue from test team end.
,
Dec 19 2016
,
Dec 19 2016
I don't see the problem with Stable M55, but do see this in Canary. My MBP is 10.12.2. According to this report, it happens in M56. So it appears as a regression between M55 and M56. Will do a bisect ASAP.
,
Dec 19 2016
By bisecting, here may be the story, most probably: In the roll of https://chromium.googlesource.com/chromium/src/+/c3f9bded0cf8d72c9a7b589466fe455ba1f81dc3, the cl https://codereview.webrtc.org/2391743004 caused screen capture on mac freezing. Then in the roll of https://chromium.googlesource.com/chromium/src/+/7f03772199ab446401da40c043ebd8aa479a0be6, the cl https://codereview.webrtc.org/2417603002 fixed the above issue but the screen capture becomes buggy(moving any window around during capturing, you can see the residue in local preview.) The bisect result of above two ranges are: - https://chromium.googlesource.com/chromium/src/+log/ab78fefbe385452fae85587b505bcc1bac60a17f..52de7c17a1f9e4b3a2ed57d41afd131c0f4f8e69 - https://chromium.googlesource.com/chromium/src/+log/fbb8975f1693f4fb33d54f62bbef19a70a4bc724..fdf63b271e08ef37cab00882d13e53b0d52e2338 erikchen@, please double check.
,
Dec 19 2016
braveyao, what do you mean by "but the screen capture becomes buggy(moving any window around during capturing, you can see the residue in local preview". How did you determine this? What exactly are you bisecting? My CLs moved us from an old, deprecated API to the modern replacement. If there's a problem, either: 1) We're not using the new API correctly. 2) The new API is buggy. I'm a little wary of how we're handling dirty rects [we are using kCGDisplayStreamUpdateDirtyRects instead of kCGDisplayStreamUpdateMovedRects and kCGDisplayStreamUpdateRefreshedRects, but sergeyu@ believes that this is fine]. I can't think of anything else that has significantly changed.
,
Dec 19 2016
A pic tells more than a thousand words. The attachments shows what it looks like when I moved the window picker from middle to left-bottom corner. I'm not sure the exact cl that causes this problem. But the bisect result is good though. Tested with demo: chromium/src/chrome/common/extensions/docs/examples/api/desktopCapture PS: this demo only works with Canary now.
,
Dec 19 2016
I built ToT and ran the demo on macOS - no issues. Can you be more specific about the configuration you used to repro the issue? Note that ToT includes several more fixes: """ commit feec627c2317df1b9059b4dff4122e579d4619cd Author: erikchen <erikchen@chromium.org> Date: Tue Nov 15 10:24:43 2016 -0800 mac: Fix screen capture on secondary displays. ... commit 8d5b18ee1c70e3f3255be6b8d1445e40e74a775a Author: erikchen <erikchen@chromium.org> Date: Wed Oct 26 11:10:24 2016 -0700 Change destruction order to fix potential invalid pointer dereference. """ so any bisect you do that doesn't include those two fixes isn't going to provide very meaningful results.
,
Dec 19 2016
The Canary still shows this problem, 57.0.2956.0(#439388). Also my Chromium workspace (master@{#439253} )can also reproduce it.
Maybe it's platform specific?
My MBP is : MBP Retina, 15-inch, early 2013, OSX10.12.2. Graphics Intel HD 4000 1536MB.
What's yours?
,
Dec 19 2016
I was able to reproduce the issue on ToT on a MBP. Reverting all my CLs fixes the problem. Maybe a retina vs non-retina issue?
,
Dec 19 2016
,
Dec 20 2016
Yes it's true. No problem on MBP+non-Retina with Canary.
,
Dec 20 2016
I have a patch that will fix the problem. I'm kind of surprised that this ever worked to begin with, since the old logic [prior to my refactor] was also broken. I'm also surprised that this is only broken on some devices. When you look at the image, it looks like the screen capturer is refreshing the destination rect when the window is dragged, but not the source rect. Changing the logic of CGDisplayStreamFrameAvailableHandler so that it refreshes both the source/destination rect fixes the issue.
,
Dec 20 2016
CL up for review: https://codereview.chromium.org/2588973002/
,
Dec 21 2016
The following revision refers to this bug: https://chromium.googlesource.com/external/webrtc.git/+/494dff4c079910f0033e3ed63c975c791d6ded37 commit 494dff4c079910f0033e3ed63c975c791d6ded37 Author: erikchen <erikchen@chromium.org> Date: Wed Dec 21 01:00:22 2016 Fix a screen capture issue on retina macOS devices. The CGDisplayStream API returns rects in physical pixel coordinates, not Density-Independent Pixel coordinates. The code was incorrectly re-applying the dip_to_pixel scaling. BUG= chromium:675490 Review-Url: https://codereview.webrtc.org/2588973002 Cr-Commit-Position: refs/heads/master@{#15720} [modify] https://crrev.com/494dff4c079910f0033e3ed63c975c791d6ded37/webrtc/modules/desktop_capture/screen_capturer_mac.mm
,
Dec 21 2016
Can someone from the webrtc team take care of merging this to M-56?
,
Dec 22 2016
Your change meets the bar and is auto-approved for M56 (branch: 2924)
,
Dec 26 2016
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/external/webrtc.git/+/f4f8de6631c1b05be8bb57e622535e2ec73d89fb commit f4f8de6631c1b05be8bb57e622535e2ec73d89fb Author: Sergey Ulanov <sergeyu@chromium.org> Date: Tue Dec 27 19:45:00 2016 Fix a screen capture issue on retina macOS devices. The CGDisplayStream API returns rects in physical pixel coordinates, not Density-Independent Pixel coordinates. The code was incorrectly re-applying the dip_to_pixel scaling. BUG= chromium:675490 Review-Url: https://codereview.webrtc.org/2588973002 Cr-Commit-Position: refs/heads/master@{#15720} (cherry picked from commit 494dff4c079910f0033e3ed63c975c791d6ded37) Review-Url: https://codereview.webrtc.org/2607533003 . Cr-Commit-Position: refs/branch-heads/56@{#11} Cr-Branched-From: 613152af11d6e9a4d046af3c48a7be7642dfcc68-refs/heads/master@{#15101} [modify] https://crrev.com/f4f8de6631c1b05be8bb57e622535e2ec73d89fb/webrtc/modules/desktop_capture/screen_capturer_mac.mm
,
Dec 27 2016
Thanks sergeyu. I don't know if DEPS need to be rolled, but I'll assume this bug is fixed for now.
,
Dec 27 2016
56 chrome branch pulls the head of the 56 webrtc branch, so DEPS don't need to be rolled.
,
Dec 29 2016
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 3 2017
Thanks for merging the CL, if there is no pending work please remove Merge-Approved-56 label.
,
Jan 4 2017
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by ajha@chromium.org
, Dec 18 2016