[Video Capture, Windows] Some capture devices not working with MediaFoundation |
|||||||||||
Issue descriptionDuring rollout of the new MediaFoundation code path for video capture on Windows, users have reported that certain types of devices stopped working. See comments beginning at https://bugs.chromium.org/p/chromium/issues/detail?id=849636#c9. The rollout has since been rolled back to mitigate the issue until we can find the root cause and have landed a fix. Since https://bugs.chromium.org/p/chromium/issues/detail?id=849636 is for tracking a different issue, i.e. an increase in errors during video capture start that happened around May 1st, let us use this new issue entry to track the work on the MediaFoundation code path.
,
Jul 15
,
Jul 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3772661191b045df5ed2df7aa6fde0a787967fae commit 3772661191b045df5ed2df7aa6fde0a787967fae Author: Réda Housni Alaoui <alaoui.rda@gmail.com> Date: Tue Jul 17 16:28:55 2018 [Video Capture Win] Fallback to DirectShow when MediaFoundation offers no video format Devices like Pinnacle Dazzle are listed in DirectShow with their video formats while listed in MediaFoundation with no available video format at all. This makes the capture factory fallback to DirectShow implementation when it contains at least one video format and its MediaFoundation counterpart has none. Bug: 862395 Change-Id: I5d02f97971d4cf98372e12bb96c0b7ad9cc6d9c3 Reviewed-on: https://chromium-review.googlesource.com/1137817 Commit-Queue: Christian Fremerey <chfremer@chromium.org> Reviewed-by: Christian Fremerey <chfremer@chromium.org> Cr-Commit-Position: refs/heads/master@{#575667} [modify] https://crrev.com/3772661191b045df5ed2df7aa6fde0a787967fae/media/capture/video/win/video_capture_device_factory_win.cc [modify] https://crrev.com/3772661191b045df5ed2df7aa6fde0a787967fae/media/capture/video/win/video_capture_device_factory_win.h [modify] https://crrev.com/3772661191b045df5ed2df7aa6fde0a787967fae/media/capture/video/win/video_capture_device_factory_win_unittest.cc
,
Jul 17
Thank you chfremer@. Can we backport the fix to 68?
,
Jul 17
I was hoping we might still be able to get this into M68, since it the change is quite safe and is essentially a "hotfix" for the feature rollout which is temporarily disable via service-side configuration flag. However, we do need to confirm that things are working on at least one Canary build first, and it looks like M68 stable cut is happening before that can be done. Let's check with the release owner if there is any chance to still get it in.
,
Jul 17
This bug requires manual review: We are only 6 days from stable. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 17
thanks for the fix. We are cutting release candidate today at 2pm PDT for tomorrow's beta, so it will be tough to get this in. Since this is the last beta, my recommendation is to target this fix for M69.
,
Jul 17
abdulsyed@: Hmm, I see. So even if we verify on the next Canary, we would not be able to verify on any M68 beta anymore, leaving a merge directly into the release candidate as the only (probably undesirable) option. It looks like alaoui.rda@gmail.com has verified the change on his local build, and the touched functionality is covered by unit tests as well. If I go ahead and also verify on a local build of tip of tree right now, would you approve a merge into M68 before the 2pm PDT deadline?
,
Jul 17
For what it's worth, I have tested a local build of the latest tip of tree, which includes this fix, and found no regression.
,
Jul 17
confirmed with chfremer@, this is behind a feature flag. It's safe, isolated, and well tested locally. Approved for M68. Branch:3440. But please verify this in Canary and Beta tomorrow.
,
Jul 17
Thank you. I am creating the CL.
,
Jul 17
re #11 no need, I already did the cherry pick into the branch.
,
Jul 17
Hmm looks like I spoke too early. The cherry pick through Gerrit gives me an error because of a merge conflict. I'll see if I can resolve it.
,
Jul 17
Oh cool, just have to wait then :)
,
Jul 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6e3ddfc4b757ca922cb2398db4ad887873210eda commit 6e3ddfc4b757ca922cb2398db4ad887873210eda Author: Christian Fremerey <chfremer@chromium.org> Date: Tue Jul 17 20:36:35 2018 [Video Capture Win] Fallback to DirectShow when MediaFoundation offers no video format Devices like Pinnacle Dazzle are listed in DirectShow with their video formats while listed in MediaFoundation with no available video format at all. This makes the capture factory fallback to DirectShow implementation when it contains at least one video format and its MediaFoundation counterpart has none. TBR=alaoui.rda@gmail.com (cherry picked from commit 3772661191b045df5ed2df7aa6fde0a787967fae) Bug: 862395 Change-Id: I5d02f97971d4cf98372e12bb96c0b7ad9cc6d9c3 Reviewed-on: https://chromium-review.googlesource.com/1137817 Commit-Queue: Christian Fremerey <chfremer@chromium.org> Reviewed-by: Christian Fremerey <chfremer@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#575667} Reviewed-on: https://chromium-review.googlesource.com/1141051 Cr-Commit-Position: refs/branch-heads/3440@{#701} Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733} [modify] https://crrev.com/6e3ddfc4b757ca922cb2398db4ad887873210eda/media/capture/video/win/video_capture_device_factory_win.cc [modify] https://crrev.com/6e3ddfc4b757ca922cb2398db4ad887873210eda/media/capture/video/win/video_capture_device_factory_win.h [modify] https://crrev.com/6e3ddfc4b757ca922cb2398db4ad887873210eda/media/capture/video/win/video_capture_device_factory_win_unittest.cc
,
Jul 17
Cool, the merge was easy to do with git drover. Let's keep a close eye on the beta builder to make sure everything stays green.
,
Jul 17
Thank you chfremer@ Can the 100% rollout of MediaFoundation be resumed on 68?
,
Jul 17
We'll have to get a new approval to re-ramp the rollout of the feature once the code change has reached users with M68. I don't assume we will immediately go to 100%. Most likely it will again be 1% first and then increase from there.
,
Jul 18
Beta builder did succeed with the merge. The first M68 including the fix is 68.0.3440.67, but the latest beta release is still 68.0.3440.59, so we'll have to wait a bit for the newer release. For M69, the first build including the fix is 69.0.3495.0, which is today's Canary. alaoui.rda@gmail.com: Could you please try today's Canary and check that everything works as expected?
,
Jul 18
Looks like the new Beta is now out with version 68.0.3440.68. alaoui.rda@gmail.com: Would you mind giving it a spin when you get a chance?
,
Jul 19
chfremer@ I just tested both latest Canary and latest Beta with the MediaFoundation flag enabled. The fix works as expected.
,
Aug 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0ba070fd460e84e89cabb8af7396f1e28b9ba513 commit 0ba070fd460e84e89cabb8af7396f1e28b9ba513 Author: Christian Fremerey <chfremer@chromium.org> Date: Tue Aug 14 23:04:29 2018 [VideoCapture, Windows] Blacklist certain legacy Empia chips for MediaFoundation Devices using certain legacy Empia chips appear to get listed and report valid formats in both DirectShow and MediaFoundation but only work in DirectShow when actually trying to open them. This CL adds a blacklisting mechanism that forces device models known to exhibit this issue to always get opened with DirectShow instead of MediaFoundation. Test: capture_unittests.exe --gtest_filter=VideoCaptureDeviceFactoryMFWinTest* Bug: 792640, 849636, 862395 Change-Id: Ia70d13a45eb49ed185f27630d0e544f3a484b6f2 Reviewed-on: https://chromium-review.googlesource.com/1174902 Reviewed-by: Emircan Uysaler <emircan@chromium.org> Commit-Queue: Christian Fremerey <chfremer@chromium.org> Cr-Commit-Position: refs/heads/master@{#583068} [modify] https://crrev.com/0ba070fd460e84e89cabb8af7396f1e28b9ba513/media/capture/video/win/video_capture_device_factory_win.cc [modify] https://crrev.com/0ba070fd460e84e89cabb8af7396f1e28b9ba513/media/capture/video/win/video_capture_device_factory_win_unittest.cc
,
Aug 16
Requesting merge of CL from #24 into M69 and maybe even M68. Justification for the merge is that this is a "hotfix" for unblocking the rollout tracked in bug 792640. It is very safe, since it is small and localized and only affects a code path that is rolled out via Finch flag and is currently at 50% in beta and 0% in stable.
,
Aug 16
This bug requires manual review: M69 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 16
Approving merge to M69 branch 3497 based on comment #25. Please merge ASAP. Thank you.
,
Aug 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1abb14e34927da38b26ba3087dcb443847c641ac commit 1abb14e34927da38b26ba3087dcb443847c641ac Author: Christian Fremerey <chfremer@chromium.org> Date: Thu Aug 16 23:22:33 2018 [VideoCapture, Windows] Blacklist certain legacy Empia chips for MediaFoundation Devices using certain legacy Empia chips appear to get listed and report valid formats in both DirectShow and MediaFoundation but only work in DirectShow when actually trying to open them. This CL adds a blacklisting mechanism that forces device models known to exhibit this issue to always get opened with DirectShow instead of MediaFoundation. Test: capture_unittests.exe --gtest_filter=VideoCaptureDeviceFactoryMFWinTest* Bug: 792640, 849636, 862395 Change-Id: Ia70d13a45eb49ed185f27630d0e544f3a484b6f2 Reviewed-on: https://chromium-review.googlesource.com/1174902 Reviewed-by: Emircan Uysaler <emircan@chromium.org> Commit-Queue: Christian Fremerey <chfremer@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#583068}(cherry picked from commit 0ba070fd460e84e89cabb8af7396f1e28b9ba513) Reviewed-on: https://chromium-review.googlesource.com/1179241 Reviewed-by: Christian Fremerey <chfremer@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#679} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/1abb14e34927da38b26ba3087dcb443847c641ac/media/capture/video/win/video_capture_device_factory_win.cc [modify] https://crrev.com/1abb14e34927da38b26ba3087dcb443847c641ac/media/capture/video/win/video_capture_device_factory_win_unittest.cc
,
Aug 27
,
Aug 27
This was actually merged to M69
,
Oct 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6854025abb6278daaa3258117cf8c796ec46bba8 commit 6854025abb6278daaa3258117cf8c796ec46bba8 Author: Christian Fremerey <chfremer@chromium.org> Date: Thu Oct 11 20:46:32 2018 [Video Capture, Win] Update blacklist for MediaFoundation This CL adds a new entry to the blacklist as per new report from https://bugs.chromium.org/p/chromium/issues/detail?id=849636#c61 TBR=emircan@chromium.org Bug: 792640, 849636, 862395 Change-Id: I47a36dce54993f93059fd833f0ca5ea074037339 Reviewed-on: https://chromium-review.googlesource.com/c/1277589 Commit-Queue: Christian Fremerey <chfremer@chromium.org> Reviewed-by: Christian Fremerey <chfremer@chromium.org> Cr-Commit-Position: refs/heads/master@{#598921} [modify] https://crrev.com/6854025abb6278daaa3258117cf8c796ec46bba8/media/capture/video/win/video_capture_device_factory_win.cc |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by alaoui....@gmail.com
, Jul 15