[Video Capture] Add check to ensure Mac video capture runs on a CFRunLoop based message loop |
||
Issue descriptionThe video capture code for mac, i.e. media/capture/video/mac, has an implicit requirement that it must be run on a CFRunLoop [1] based message loop. Currently, if the code is run on a non-CFRunLoop based message loop, there is no warning or failure. An observed symptom is flakiness of events that are expected to be fired when webcams are plugged in or unplugged. To prevent changes inadvertently leading to the code running on a non-CFRunLoop to go unnoticed, we need to add a check that causes a hard failure in this situation. Ideally, this would be a build-time check, but if that is not possible, a runtime check that fails an integration test could work as well. [1] https://developer.apple.com/documentation/corefoundation/cfrunloop-rht
,
Sep 25
re #1: Thanks, that is a great suggestion. I tried it and it works as a runtime check. Only downside is now that if this CHECK is triggered when the video capture service is run in a utility process, the failure is not very *hard* in that it "only" leads to the symptom of video capture not working at all. However, this should definitely be enough to fail some integration tests in the CQ, so this should do the trick of preventing regression. CL going up for review shortly.
,
Oct 2
For reference, this is a follow-up for Bug 834581
,
Oct 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/917db44faac04a63b42534235e6d47233c3f0ac4 commit 917db44faac04a63b42534235e6d47233c3f0ac4 Author: Christian Fremerey <chfremer@chromium.org> Date: Fri Oct 05 21:10:08 2018 [Video Capture, Mac] Ensure code is running on a CFRunLoop enabled thread This is to prevent regression that only manifests in very subtle issues such as device enumeration becoming unreliable. Bug: 848042 Change-Id: I4044c2494e2a525ca2eeca22bd67698e651360ff Reviewed-on: https://chromium-review.googlesource.com/c/1246147 Reviewed-by: Emircan Uysaler <emircan@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org> Commit-Queue: Christian Fremerey <chfremer@chromium.org> Cr-Commit-Position: refs/heads/master@{#597293} [modify] https://crrev.com/917db44faac04a63b42534235e6d47233c3f0ac4/media/capture/video/mac/video_capture_device_factory_mac.mm [modify] https://crrev.com/917db44faac04a63b42534235e6d47233c3f0ac4/media/capture/video/mac/video_capture_device_factory_mac_unittest.mm [modify] https://crrev.com/917db44faac04a63b42534235e6d47233c3f0ac4/media/capture/video/video_capture_device_unittest.cc
,
Oct 8
|
||
►
Sign in to add a comment |
||
Comment 1 by rsesek@chromium.org
, May 30 2018