New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 730068 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: 2017-06-28
OS: Windows
Pri: 2
Type: Bug

Blocked on:
issue 735576
issue 791610

Blocking:
issue 807293


Participants' hotlists:
Hotlist-1


Sign in to add a comment

ImageCapture on Surface Rear Camera (Win) is not using the highest available resolution

Reported by alaoui....@gmail.com, Jun 6 2017

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.86 Safari/537.36

Steps to reproduce the problem:
1. Find a windows 10 computer with a camera device able to take a picture at higher resolution than the video resolution (i.e. Surface Pro 4 Rear Camera which can preview a video at 1080p and can take a shoot at 3264 x 2448)
2. Take a photo at the highest possible resolution with the experimental takePhoto() function

What is the expected behavior?
The photo should have been took at the highest possible resolution

What went wrong?
The photo is taken at the highest video resolution

Did this work before? N/A 

Does this work in other browsers? N/A

Chrome version: 59.0.3071.86  Channel: stable
OS Version: 10
Flash Version: 

I created/tweaked some c++ code to verify the following assertions.
ATMT = According to my tests.

ATMT, the Surface Pro 4 Rear Camera can take video up to 1080p and photo up to 3264 x 2448.

I think the issue is in the MediaFoundation implementation.

There is 2 ways to take a picture in MediaFoundation:
1 - Using directly IMFSourceReader::ReadSample, which captures a frame from video stream
2 - Using IMFCaptureEngine::TakePhoto which captures a still picture

Method 1 seems to be the only one used by Chromium in MF implementation. 
ATMT, method 1 limits frame grabbing to the highest video resolution (in my case 1080p on surface pro 4 rear camera).

ATMT, method 2 allows to take a photo at 3264 x 2448.

I think that MF implementation should at least implement takePhoto() using the IMFCaptureEngine, at best use the IMFCaptureEngine event for the webrtc video part.

If you want some code snippets, I can post them.

I am prone to deliver a patch if you give me some instructions about the potential unit tests and if you are not in a hurry :)
 
Labels: Needs-Triage-M59
Labels: pre-stable-59.0.3071.86
Cc: mcasas@chromium.org
Components: -Blink>Media Blink>ImageCapture
Summary: ImageCapture on Surface Rear Camera (Win) is not using the highest available resolution (was: ImageCapture on Surface Rear Camera is not using the highest available resolution)
alaoui.rda@ thanks for the report and suggestions; before diving into 
how to add this support, can you confirm that MediaFoundation is being
used?

I'm thinking about line [1] that IIUC is routing all Win video capture
to use Direct Show; moreover, VideoCaptureDeviceMFWin class has no
takePhoto() method.

[1] https://cs.chromium.org/chromium/src/media/capture/video/win/video_capture_device_factory_win.cc?sq=package:chromium&dr=CSs&l=409

Labels: Needs-Feedback

Comment 6 by mcasas@chromium.org, Jun 21 2017

NextAction: 2017-06-28
Hello mcasas,

Sorry for the late response.
I will check that MediaFoundation is used this week end.

Regards
Project Member

Comment 8 by sheriffbot@chromium.org, Jun 23 2017

Cc: ranjitkan@chromium.org
Labels: -Needs-Feedback
Thank you for providing more feedback. Adding requester "ranjitkan@chromium.org" to the cc list and removing "Needs-Feedback" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
The Surface disk space is not big enough to build chromium on it.
Despite this I tried to put a breakpoint in video_capture_device_factory_win.cc after pointing Visual Studio 2015 to the chromium symbol server without success. The breakpoint is not recognized.

Do I need to build the full chromium on the Surface to put a breakpoint on this class? Is there another way?
Cc: jbanavatu@chromium.org
Labels: TE-NeedsTriageHelp
Unable to triage this issue from TE end. Hence adding TE-NeedsTriageHelp label for further triage.
mcasas@ As per comment #9, could you please help in further triaging this issue.

Thanks!!
The NextAction date has arrived: 2017-06-28
Same for my Android phone. It can do 4160x3120 but the image I get from takePhone is 800x600
When I explicitly set correct width and height, it works:
const caps = await (imageCapture.getPhotoCapabilities() as Promise<any>)
const options = {
      focusMode: 'auto',
      imageHeight: caps.imageHeight.max,
      imageWidth: caps.imageHeight.max
}

imageCapture.takePhoto(options)
I am currently debugging on a surface using https://googlechrome.github.io/samples/image-capture/grab-frame-take-photo.html.

Directshow is always used for the video stream unless flag "--force-mediafoundation" is passed to chrome shortcut.

After passing "--force-mediafoundation", media foundation implementation is used but the result is the same. video + frame grab still works but "take photo" doesn't work at all. Live output returns "{}".

Please note that "take photo" doesn't work in both cases (directshow or MF).
Cc: chfremer@chromium.org
Status: Available (was: Unconfirmed)
Windows takePhoto() code is just grabbing the next-available-captured frame, encoding
it and sending it as the taken photo [1]; it doesn't reconfigure the device for higher
resolution ATM, neither via DirectShow nor MediaFoundation -- and also note that MF 
is not used right now anywhere AFAIK.

So this issue is legit and should be addressed, but I won't have the cycles to
follow it through; Ic an review patches and provide guidance though. 
chfremer@ can you work on this?

[1] https://cs.chromium.org/chromium/src/media/capture/video/win/video_capture_device_win.cc?q=video+capture+device+win&sq=package:chromium&dr=CSs&l=606
Hello,

I have a functional patch (for media foundation only) almost ready.
Should I open a pull request?
alaoui.rda@ ATM Media Foundation is not used unless the specific
flag forces it, so it might now help much, unless we have plans to
migrate to MF soon, chfremer@ ?
mcasas, I would prefer to avoid the flag usage.

But what I know is that event DirectShow on still PIN won't allow me to reach the 4K resolution on the surface rear camera. I get this from a Microsoft tech expert.

So even behind the flag, fixed media foundation implementation will be tremendously useful to me :)
even*
I had to sync with the master to create my code review.
Since this sync, flag force-mediafoundation is broken.
The flag is also broken in the latest official chrome release (62).

I created https://bugs.chromium.org/p/chromium/issues/detail?id=777880 about this bug.
I agree with mcasas@ that landing changes to the Media Foundation based implementation of media::VideoCaptureDevice is questionable unless we plan to actually roll it out to users.

I would like to keep the CL from #20 on hold until we have a decision.

alaoui.rda@: You seem to have some valuable insights into why a Media Foundation may come out superior to what can be achieved with DirectShow. Could you please share these insights in issue entry for making the decision? See here: https://bugs.chromium.org/p/chromium/issues/detail?id=735576
What's happening now?
Don't forgive this please :)
forget*, I need a coffee
Project Member

Comment 25 by bugdroid1@chromium.org, Dec 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f9eaa531b5dfe3f6f3dba721ceca8b138d78410f

commit f9eaa531b5dfe3f6f3dba721ceca8b138d78410f
Author: Réda Housni Alaoui <alaoui.rda@gmail.com>
Date: Mon Dec 04 20:38:11 2017

Win video capture: use IMFCaptureEngine for Media Foundation

- Full rewrite of the MediaFoundation implementation video part to use
IMFCaptureEngine
- Implementation of takePhoto, setPhotoOptions and getPhotoCapabilities
- takePhoto triggers a still image capture with the highest available
resolution without stopping the video stream thanks to IMFCaptureEngine

TEST=adapted video_capture_device_unittest.cc and
webrtc_image_capture_browsertest.cc; launch Chrome with
--force-mediafoundation on Win8+ and capture video using
e.g. https://webrtc.github.io/samples/src/content/getusermedia/gum/

R=mcasas@chromium.org

Bug: 730068
Change-Id: Ib8e7f475d8120a63dd08c7b215c1eaf2c6f3d800
Reviewed-on: https://chromium-review.googlesource.com/734042
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521435}
[modify] https://crrev.com/f9eaa531b5dfe3f6f3dba721ceca8b138d78410f/AUTHORS
[modify] https://crrev.com/f9eaa531b5dfe3f6f3dba721ceca8b138d78410f/content/browser/webrtc/webrtc_image_capture_browsertest.cc
[modify] https://crrev.com/f9eaa531b5dfe3f6f3dba721ceca8b138d78410f/content/test/data/media/image_capture_test.html
[modify] https://crrev.com/f9eaa531b5dfe3f6f3dba721ceca8b138d78410f/media/capture/video/video_capture_device_unittest.cc
[modify] https://crrev.com/f9eaa531b5dfe3f6f3dba721ceca8b138d78410f/media/capture/video/win/video_capture_device_factory_win.cc
[modify] https://crrev.com/f9eaa531b5dfe3f6f3dba721ceca8b138d78410f/media/capture/video/win/video_capture_device_factory_win.h
[modify] https://crrev.com/f9eaa531b5dfe3f6f3dba721ceca8b138d78410f/media/capture/video/win/video_capture_device_mf_win.cc
[modify] https://crrev.com/f9eaa531b5dfe3f6f3dba721ceca8b138d78410f/media/capture/video/win/video_capture_device_mf_win.h

chfremer@, the code is being reverted because of some failing tests on "Win8 tester" [1].

I am planning to debug that and commit the fix in [2].
Is that ok for you?

[1] https://chromium-review.googlesource.com/c/chromium/src/+/810624
[2] https://chromium-review.googlesource.com/c/chromium/src/+/734042
#26: when you do so, please name the CL like this:
 Reland: <original title>
and in the CL description, add first the fixes and
then the original description; then upload first the
originally landed CL and add your fixes as subsequent
patches: this will make the review process easier, 
specially given the large size of the original CL.
Blockedon: 791610
Cc: alaoui....@gmail.com
Components: Internals>Media>Capture Blink>GetUserMedia>Webcam
Labels: -Needs-Triage-M59 -pre-stable-59.0.3071.86 M-65
Owner: chfremer@chromium.org
 alaoui.rda@gmail.com is working on this Issue, but 
can't be assigned as Owner, so putting chfremer@ instead.
#27: mcasas@, just to be sure to understand, [1] must not be reopened and you want a new CL including [1] patch + fixes?

[1] https://chromium-review.googlesource.com/c/chromium/src/+/734042
#29: yes, easiest thing you can do to bootstrap is to pull 
master, create a new branch, patch the reverted CL (git cl
patch 734042 && git cl issue 0), the reupload with the 
caveats of #27, and start working from there.
Project Member

Comment 31 by bugdroid1@chromium.org, Dec 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ea772d7237b12df60f7fbb0ba78f535287af0b37

commit ea772d7237b12df60f7fbb0ba78f535287af0b37
Author: Henrik Boström <hbos@chromium.org>
Date: Wed Dec 06 14:11:45 2017

Revert "Win video capture: use IMFCaptureEngine for Media Foundation"

This reverts commit f9eaa531b5dfe3f6f3dba721ceca8b138d78410f.

Reason for revert: Suspected cause of Win8 Tester failures
https://build.chromium.org/deprecated/chromium.webrtc/builders/Win8%20Tester/builds/38865

Original change's description:
> Win video capture: use IMFCaptureEngine for Media Foundation
> 
> - Full rewrite of the MediaFoundation implementation video part to use
> IMFCaptureEngine
> - Implementation of takePhoto, setPhotoOptions and getPhotoCapabilities
> - takePhoto triggers a still image capture with the highest available
> resolution without stopping the video stream thanks to IMFCaptureEngine
> 
> TEST=adapted video_capture_device_unittest.cc and
> webrtc_image_capture_browsertest.cc; launch Chrome with
> --force-mediafoundation on Win8+ and capture video using
> e.g. https://webrtc.github.io/samples/src/content/getusermedia/gum/
> 
> R=​mcasas@chromium.org
> 
> Bug: 730068
> Change-Id: Ib8e7f475d8120a63dd08c7b215c1eaf2c6f3d800
> Reviewed-on: https://chromium-review.googlesource.com/734042
> Commit-Queue: Christian Fremerey <chfremer@chromium.org>
> Reviewed-by: Miguel Casas <mcasas@chromium.org>
> Reviewed-by: Christian Fremerey <chfremer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#521435}

TBR=mcasas@chromium.org,chfremer@chromium.org,alaoui.rda@gmail.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 730068
Change-Id: I439af305a6bb45c26efb1b395e69088344a71536
Reviewed-on: https://chromium-review.googlesource.com/810624
Reviewed-by: Henrik Boström <hbos@chromium.org>
Commit-Queue: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522080}
[modify] https://crrev.com/ea772d7237b12df60f7fbb0ba78f535287af0b37/AUTHORS
[modify] https://crrev.com/ea772d7237b12df60f7fbb0ba78f535287af0b37/content/browser/webrtc/webrtc_image_capture_browsertest.cc
[modify] https://crrev.com/ea772d7237b12df60f7fbb0ba78f535287af0b37/content/test/data/media/image_capture_test.html
[modify] https://crrev.com/ea772d7237b12df60f7fbb0ba78f535287af0b37/media/capture/video/video_capture_device_unittest.cc
[modify] https://crrev.com/ea772d7237b12df60f7fbb0ba78f535287af0b37/media/capture/video/win/video_capture_device_factory_win.cc
[modify] https://crrev.com/ea772d7237b12df60f7fbb0ba78f535287af0b37/media/capture/video/win/video_capture_device_factory_win.h
[modify] https://crrev.com/ea772d7237b12df60f7fbb0ba78f535287af0b37/media/capture/video/win/video_capture_device_mf_win.cc
[modify] https://crrev.com/ea772d7237b12df60f7fbb0ba78f535287af0b37/media/capture/video/win/video_capture_device_mf_win.h

mcasas@, chfremer@, about [1], how can I know which webcam model and exact OS version was used?

[1] https://build.chromium.org/deprecated/chromium.webrtc/builders/Win8%20Tester/builds/38865
Project Member

Comment 33 by bugdroid1@chromium.org, Dec 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7c1a115d3fbe1ba60f8160fc6dab38ed59a1ece2

commit 7c1a115d3fbe1ba60f8160fc6dab38ed59a1ece2
Author: Réda Housni Alaoui <alaoui.rda@gmail.com>
Date: Fri Dec 15 18:42:25 2017

Reland: Win video capture: use IMFCaptureEngine for Media Foundation

Fixes for reland:
- "Win8 Tester" browser_tests_functional failure
- "Win8 Tester" capture_unittests failure

Original description:
- Full rewrite of the MediaFoundation implementation video part to use
IMFCaptureEngine
- Implementation of takePhoto, setPhotoOptions and getPhotoCapabilities
- takePhoto triggers a still image capture with the highest available
resolution without stopping the video stream thanks to IMFCaptureEngine

TEST=adapted video_capture_device_unittest.cc and
webrtc_image_capture_browsertest.cc; launch Chrome with
--force-mediafoundation on Win8+ and capture video using
e.g. https://webrtc.github.io/samples/src/content/getusermedia/gum/

R=mcasas@chromium.org

Bug: 730068
Change-Id: I3835a48ca8516fc66a6a8394b3709d4dea862f89
Reviewed-on: https://chromium-review.googlesource.com/734042
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#521435}
Reviewed-on: https://chromium-review.googlesource.com/810766
Cr-Commit-Position: refs/heads/master@{#524417}
[modify] https://crrev.com/7c1a115d3fbe1ba60f8160fc6dab38ed59a1ece2/AUTHORS
[modify] https://crrev.com/7c1a115d3fbe1ba60f8160fc6dab38ed59a1ece2/content/browser/webrtc/webrtc_image_capture_browsertest.cc
[modify] https://crrev.com/7c1a115d3fbe1ba60f8160fc6dab38ed59a1ece2/content/test/data/media/image_capture_test.html
[modify] https://crrev.com/7c1a115d3fbe1ba60f8160fc6dab38ed59a1ece2/media/capture/video/video_capture_device_unittest.cc
[modify] https://crrev.com/7c1a115d3fbe1ba60f8160fc6dab38ed59a1ece2/media/capture/video/win/video_capture_device_factory_win.cc
[modify] https://crrev.com/7c1a115d3fbe1ba60f8160fc6dab38ed59a1ece2/media/capture/video/win/video_capture_device_factory_win.h
[modify] https://crrev.com/7c1a115d3fbe1ba60f8160fc6dab38ed59a1ece2/media/capture/video/win/video_capture_device_mf_win.cc
[modify] https://crrev.com/7c1a115d3fbe1ba60f8160fc6dab38ed59a1ece2/media/capture/video/win/video_capture_device_mf_win.h

Project Member

Comment 34 by bugdroid1@chromium.org, Dec 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c7e93d23f627f90d99a779ded63c86a5b8d6b602

commit c7e93d23f627f90d99a779ded63c86a5b8d6b602
Author: Mirko Bonadei <mbonadei@chromium.org>
Date: Mon Dec 18 12:36:40 2017

Revert "Reland: Win video capture: use IMFCaptureEngine for Media Foundation"

This reverts commit 7c1a115d3fbe1ba60f8160fc6dab38ed59a1ece2.

Reason for revert: I think this CL is still breaking Win8 and Win10:
https://ci.chromium.org/buildbot/chromium.webrtc.fyi/Win10%20Tester/10071
https://ci.chromium.org/buildbot/chromium.webrtc.fyi/Win8%20Tester/3085

Original change's description:
> Reland: Win video capture: use IMFCaptureEngine for Media Foundation
> 
> Fixes for reland:
> - "Win8 Tester" browser_tests_functional failure
> - "Win8 Tester" capture_unittests failure
> 
> Original description:
> - Full rewrite of the MediaFoundation implementation video part to use
> IMFCaptureEngine
> - Implementation of takePhoto, setPhotoOptions and getPhotoCapabilities
> - takePhoto triggers a still image capture with the highest available
> resolution without stopping the video stream thanks to IMFCaptureEngine
> 
> TEST=adapted video_capture_device_unittest.cc and
> webrtc_image_capture_browsertest.cc; launch Chrome with
> --force-mediafoundation on Win8+ and capture video using
> e.g. https://webrtc.github.io/samples/src/content/getusermedia/gum/
> 
> R=​mcasas@chromium.org
> 
> Bug: 730068
> Change-Id: I3835a48ca8516fc66a6a8394b3709d4dea862f89
> Reviewed-on: https://chromium-review.googlesource.com/734042
> Commit-Queue: Christian Fremerey <chfremer@chromium.org>
> Reviewed-by: Miguel Casas <mcasas@chromium.org>
> Reviewed-by: Christian Fremerey <chfremer@chromium.org>
> Cr-Original-Commit-Position: refs/heads/master@{#521435}
> Reviewed-on: https://chromium-review.googlesource.com/810766
> Cr-Commit-Position: refs/heads/master@{#524417}

TBR=mcasas@chromium.org,chfremer@chromium.org,alaoui.rda@gmail.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 730068
Change-Id: I04e276e3b9f42e7f8ec3a60fd8b9439788ad994f
Reviewed-on: https://chromium-review.googlesource.com/831588
Commit-Queue: Olga Sharonova <olka@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524686}
[modify] https://crrev.com/c7e93d23f627f90d99a779ded63c86a5b8d6b602/AUTHORS
[modify] https://crrev.com/c7e93d23f627f90d99a779ded63c86a5b8d6b602/content/browser/webrtc/webrtc_image_capture_browsertest.cc
[modify] https://crrev.com/c7e93d23f627f90d99a779ded63c86a5b8d6b602/content/test/data/media/image_capture_test.html
[modify] https://crrev.com/c7e93d23f627f90d99a779ded63c86a5b8d6b602/media/capture/video/video_capture_device_unittest.cc
[modify] https://crrev.com/c7e93d23f627f90d99a779ded63c86a5b8d6b602/media/capture/video/win/video_capture_device_factory_win.cc
[modify] https://crrev.com/c7e93d23f627f90d99a779ded63c86a5b8d6b602/media/capture/video/win/video_capture_device_factory_win.h
[modify] https://crrev.com/c7e93d23f627f90d99a779ded63c86a5b8d6b602/media/capture/video/win/video_capture_device_mf_win.cc
[modify] https://crrev.com/c7e93d23f627f90d99a779ded63c86a5b8d6b602/media/capture/video/win/video_capture_device_mf_win.h

This one is tricky.
I will roll a new CL when ready.
mcasas@, chfremer@, could you please tell me how to find out what camera was used in [1] ?

[1] https://ci.chromium.org/buildbot/chromium.webrtc.fyi/Win10%20Tester/10071
In theory, someone from the infrastructure team could be tracked down to answer that, but I'd have to find out who that is.

When I last faced this question I decided to add logging here [1] and here [2] that prints out the camera model on each test run, hoping this would give a more immediate and reliable way of finding this information. Unfortunately, the logging at [1] was changed from LOG to DLOG, so it does not currently tell us anything. The log at [2] does not seem to be run either.

Let me see if I can find an old enough build from a date before the LOG was changed to a DLOG. We should then probably also change the DLOG back to a LOG or VLOG or anything else that gets printed on the bot runs.

[1] https://cs.chromium.org/chromium/src/media/capture/video/video_capture_device_unittest.cc?dr=C&l=339
[2] https://cs.chromium.org/chromium/src/content/browser/webrtc/webrtc_image_capture_browsertest.cc?l=116
Got it, see https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.webrtc.fyi%2FWin10_Tester%2F8002%2F%2B%2Frecipes%2Fsteps%2Fcapture_unittests%2F0%2Fstdout

[3372:4148:0710/024035.073:6491031:INFO:video_capture_device_unittest.cc(314)] Using camera HD Pro Webcam C920 (046d:082d)
Thank you chfremer@ !
Project Member

Comment 40 by bugdroid1@chromium.org, Jan 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/096993adf13cd499753b9c68c2db1727f3635b66

commit 096993adf13cd499753b9c68c2db1727f3635b66
Author: Réda Housni Alaoui <alaoui.rda@gmail.com>
Date: Thu Jan 04 23:19:42 2018

Reland: Win video capture: use IMFCaptureEngine for Media Foundation

Fixes for reland number 2:
- "Win10 Tester" browser_tests_functional failure
- "Win10 Tester" capture_unittests failure
- "Win8 Tester" browser_tests_functional failure
- "Win8 Tester" capture_unittests failure

Fixes for reland number 1:
- "Win8 Tester" browser_tests_functional failure
- "Win8 Tester" capture_unittests failure

Original description:
- Full rewrite of the MediaFoundation implementation video part to use
IMFCaptureEngine
- Implementation of takePhoto, setPhotoOptions and getPhotoCapabilities
- takePhoto triggers a still image capture with the highest available
resolution without stopping the video stream thanks to IMFCaptureEngine

TEST=adapted video_capture_device_unittest.cc and
webrtc_image_capture_browsertest.cc; launch Chrome with
--force-mediafoundation on Win8+ and capture video using
e.g. https://webrtc.github.io/samples/src/content/getusermedia/gum/

R=mcasas@chromium.org

Bug: 730068
Change-Id: I454f6a87c43e0148d50c671911bc214c969f6610
Reviewed-on: https://chromium-review.googlesource.com/734042
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Cr-Original-Original-Commit-Position: refs/heads/master@{#521435}
Reviewed-on: https://chromium-review.googlesource.com/810766
Cr-Original-Commit-Position: refs/heads/master@{#524417}
Reviewed-on: https://chromium-review.googlesource.com/843974
Cr-Commit-Position: refs/heads/master@{#527139}
[modify] https://crrev.com/096993adf13cd499753b9c68c2db1727f3635b66/AUTHORS
[modify] https://crrev.com/096993adf13cd499753b9c68c2db1727f3635b66/content/browser/webrtc/webrtc_image_capture_browsertest.cc
[modify] https://crrev.com/096993adf13cd499753b9c68c2db1727f3635b66/content/test/data/media/image_capture_test.html
[modify] https://crrev.com/096993adf13cd499753b9c68c2db1727f3635b66/media/capture/video/video_capture_device_unittest.cc
[modify] https://crrev.com/096993adf13cd499753b9c68c2db1727f3635b66/media/capture/video/win/video_capture_device_factory_win.cc
[modify] https://crrev.com/096993adf13cd499753b9c68c2db1727f3635b66/media/capture/video/win/video_capture_device_factory_win.h
[modify] https://crrev.com/096993adf13cd499753b9c68c2db1727f3635b66/media/capture/video/win/video_capture_device_mf_win.cc
[modify] https://crrev.com/096993adf13cd499753b9c68c2db1727f3635b66/media/capture/video/win/video_capture_device_mf_win.h

mcasas@, chfremer@,

A new rollback is coming [1].
There is still the same issue but with more detail:

  ./../media/capture/video/video_capture_device_unittest.cc(483): error: Mock function called more times than expected - taking default 
  action specified at:
  ../../media/capture/video/video_capture_device_unittest.cc(170):
    Function call: OnError(@0873F778 16-byte object <2C-54 EE-00 A8-53 EE-00 29-03 00-00 E0-BC A7-00>, @0873F740 
  "VideoCaptureDeviceMFWin: No suitable transform was found to encode or decode the content. (0xC00D5212)")
         Expected: to be never called
           Actual: called once - over-saturated and active

I was reproducing the exact same issue with my C920 in 1080p mode before my last CL.
MediaFoundation is not able to perform a transform with MJPEG. I do not understand why yet.

By adding NV12 processing in my last CL, NV12 was selected instead of MJPEG, and there was no more issue.


Therefore, I think the current test camera is not a pro 920.

Do you want me to add a log to a new CL to capture the camera model or can you find it by asking the right person?

[1] https://ci.chromium.org/buildbot/chromium.webrtc.fyi/Win10%20Tester/10308
Cc: -chfremer@chromium.org phoglund@chromium.org ehmaldonado@chromium.org
re #41: Thanks for keeping an eye on the build. I issued the revert to make the bots green again.

Regarding the question about the webcam:
+phoglund +ehmaldonado, could you confirm for us which webcam is connected to the machine running the tests?

In addition, I also think it is a good idea to do a CL to add logging that can tell us the camera model.
Project Member

Comment 43 by bugdroid1@chromium.org, Jan 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0a92eb4f698e2800eca601e27f44105f8379d99b

commit 0a92eb4f698e2800eca601e27f44105f8379d99b
Author: Christian Fremerey <chfremer@chromium.org>
Date: Fri Jan 05 16:51:48 2018

Revert "Reland: Win video capture: use IMFCaptureEngine for Media Foundation"

This reverts commit 096993adf13cd499753b9c68c2db1727f3635b66.

Reason for revert: Still causing test failures on bot with real camera:
https://ci.chromium.org/buildbot/chromium.webrtc.fyi/Win10%20Tester/10308

Original change's description:
> Reland: Win video capture: use IMFCaptureEngine for Media Foundation
> 
> Fixes for reland number 2:
> - "Win10 Tester" browser_tests_functional failure
> - "Win10 Tester" capture_unittests failure
> - "Win8 Tester" browser_tests_functional failure
> - "Win8 Tester" capture_unittests failure
> 
> Fixes for reland number 1:
> - "Win8 Tester" browser_tests_functional failure
> - "Win8 Tester" capture_unittests failure
> 
> Original description:
> - Full rewrite of the MediaFoundation implementation video part to use
> IMFCaptureEngine
> - Implementation of takePhoto, setPhotoOptions and getPhotoCapabilities
> - takePhoto triggers a still image capture with the highest available
> resolution without stopping the video stream thanks to IMFCaptureEngine
> 
> TEST=adapted video_capture_device_unittest.cc and
> webrtc_image_capture_browsertest.cc; launch Chrome with
> --force-mediafoundation on Win8+ and capture video using
> e.g. https://webrtc.github.io/samples/src/content/getusermedia/gum/
> 
> R=​mcasas@chromium.org
> 
> Bug: 730068
> Change-Id: I454f6a87c43e0148d50c671911bc214c969f6610
> Reviewed-on: https://chromium-review.googlesource.com/734042
> Commit-Queue: Christian Fremerey <chfremer@chromium.org>
> Reviewed-by: Miguel Casas <mcasas@chromium.org>
> Reviewed-by: Christian Fremerey <chfremer@chromium.org>
> Cr-Original-Original-Commit-Position: refs/heads/master@{#521435}
> Reviewed-on: https://chromium-review.googlesource.com/810766
> Cr-Original-Commit-Position: refs/heads/master@{#524417}
> Reviewed-on: https://chromium-review.googlesource.com/843974
> Cr-Commit-Position: refs/heads/master@{#527139}

TBR=mcasas@chromium.org,chfremer@chromium.org,alaoui.rda@gmail.com

Change-Id: Ia78a7eed1958b6d48eda6a432ba86814474bdd8e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 730068
Reviewed-on: https://chromium-review.googlesource.com/852552
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527297}
[modify] https://crrev.com/0a92eb4f698e2800eca601e27f44105f8379d99b/AUTHORS
[modify] https://crrev.com/0a92eb4f698e2800eca601e27f44105f8379d99b/content/browser/webrtc/webrtc_image_capture_browsertest.cc
[modify] https://crrev.com/0a92eb4f698e2800eca601e27f44105f8379d99b/content/test/data/media/image_capture_test.html
[modify] https://crrev.com/0a92eb4f698e2800eca601e27f44105f8379d99b/media/capture/video/video_capture_device_unittest.cc
[modify] https://crrev.com/0a92eb4f698e2800eca601e27f44105f8379d99b/media/capture/video/win/video_capture_device_factory_win.cc
[modify] https://crrev.com/0a92eb4f698e2800eca601e27f44105f8379d99b/media/capture/video/win/video_capture_device_factory_win.h
[modify] https://crrev.com/0a92eb4f698e2800eca601e27f44105f8379d99b/media/capture/video/win/video_capture_device_mf_win.cc
[modify] https://crrev.com/0a92eb4f698e2800eca601e27f44105f8379d99b/media/capture/video/win/video_capture_device_mf_win.h

Project Member

Comment 44 by bugdroid1@chromium.org, Jan 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a37ef001a79010e7104ee5050822c7110237a1eb

commit a37ef001a79010e7104ee5050822c7110237a1eb
Author: Réda Housni Alaoui <alaoui.rda@gmail.com>
Date: Sun Jan 07 18:44:29 2018

Use LOG to print the camera model in video capture device unittest

We are investigating a strange failure in bot "Win10 Tester".
Allows to print the camera model during bot test.

Bug: 730068
Change-Id: I617687538a52ae145b548191fb2703a5979e17ca
Reviewed-on: https://chromium-review.googlesource.com/853492
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527542}
[modify] https://crrev.com/a37ef001a79010e7104ee5050822c7110237a1eb/AUTHORS
[modify] https://crrev.com/a37ef001a79010e7104ee5050822c7110237a1eb/media/capture/video/video_capture_device_unittest.cc

Re #42: All the tester bots in these two waterfalls use Logitech C920 webcams: 
https://uberchromegw.corp.google.com/i/chromium.webrtc/console
https://uberchromegw.corp.google.com/i/chromium.webrtc.fyi/console

They are the only bots in Chromium that are equipped with webcams.
Project Member

Comment 46 by bugdroid1@chromium.org, Jan 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/32c0a2e0d691e20e966974b1cb510b05ebb22b40

commit 32c0a2e0d691e20e966974b1cb510b05ebb22b40
Author: Réda Housni Alaoui <alaoui.rda@gmail.com>
Date: Tue Jan 09 16:00:59 2018

Reland: Win video capture: use IMFCaptureEngine for Media Foundation

Fixes for reland number 3:
- "Win10 Tester" browser_tests_functional failure
- "Win10 Tester" capture_unittests failure
- "Win8 Tester" browser_tests_functional failure
- "Win8 Tester" capture_unittests failure

Fixes for reland number 2:
- "Win10 Tester" browser_tests_functional failure
- "Win10 Tester" capture_unittests failure
- "Win8 Tester" browser_tests_functional failure
- "Win8 Tester" capture_unittests failure

Fixes for reland number 1:
- "Win8 Tester" browser_tests_functional failure
- "Win8 Tester" capture_unittests failure

Original description:
- Full rewrite of the MediaFoundation implementation video part to use
IMFCaptureEngine
- Implementation of takePhoto, setPhotoOptions and getPhotoCapabilities
- takePhoto triggers a still image capture with the highest available
resolution without stopping the video stream thanks to IMFCaptureEngine

TEST=adapted video_capture_device_unittest.cc and
webrtc_image_capture_browsertest.cc; launch Chrome with
--force-mediafoundation on Win8+ and capture video using
e.g. https://webrtc.github.io/samples/src/content/getusermedia/gum/

R=mcasas@chromium.org

Bug: 730068
Change-Id: I13fa632b25a345a20465e95a48162064eb2c4cae
Reviewed-on: https://chromium-review.googlesource.com/734042
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Cr-Original-Original-Original-Commit-Position: refs/heads/master@{#521435}
Reviewed-on: https://chromium-review.googlesource.com/810766
Cr-Original-Original-Commit-Position: refs/heads/master@{#524417}
Reviewed-on: https://chromium-review.googlesource.com/843974
Cr-Original-Commit-Position: refs/heads/master@{#527139}
Reviewed-on: https://chromium-review.googlesource.com/852455
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528005}
[modify] https://crrev.com/32c0a2e0d691e20e966974b1cb510b05ebb22b40/content/browser/webrtc/webrtc_image_capture_browsertest.cc
[modify] https://crrev.com/32c0a2e0d691e20e966974b1cb510b05ebb22b40/content/test/data/media/image_capture_test.html
[modify] https://crrev.com/32c0a2e0d691e20e966974b1cb510b05ebb22b40/media/capture/video/video_capture_device_unittest.cc
[modify] https://crrev.com/32c0a2e0d691e20e966974b1cb510b05ebb22b40/media/capture/video/win/video_capture_device_factory_win.cc
[modify] https://crrev.com/32c0a2e0d691e20e966974b1cb510b05ebb22b40/media/capture/video/win/video_capture_device_factory_win.h
[modify] https://crrev.com/32c0a2e0d691e20e966974b1cb510b05ebb22b40/media/capture/video/win/video_capture_device_mf_win.cc
[modify] https://crrev.com/32c0a2e0d691e20e966974b1cb510b05ebb22b40/media/capture/video/win/video_capture_device_mf_win.h

chfremer@, mcasas@ captureunittest passes now but a functional test still fails.
Please revert the commit.

https://ci.chromium.org/buildbot/chromium.webrtc.fyi/Win10%20Tester/10364

I am working on a fix.
Project Member

Comment 48 by bugdroid1@chromium.org, Jan 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/40055d8366e465a44a99a89e9edad570d22baf32

commit 40055d8366e465a44a99a89e9edad570d22baf32
Author: Christian Fremerey <chfremer@chromium.org>
Date: Tue Jan 09 20:34:08 2018

Revert "Reland: Win video capture: use IMFCaptureEngine for Media Foundation"

This reverts commit 32c0a2e0d691e20e966974b1cb510b05ebb22b40.

Reason for revert: Still causing issues on Win10 Tester with physical camera on webrtc waterfall, see https://ci.chromium.org/buildbot/chromium.webrtc/Win10%20Tester/24081

Original change's description:
> Reland: Win video capture: use IMFCaptureEngine for Media Foundation
> 
> Fixes for reland number 3:
> - "Win10 Tester" browser_tests_functional failure
> - "Win10 Tester" capture_unittests failure
> - "Win8 Tester" browser_tests_functional failure
> - "Win8 Tester" capture_unittests failure
> 
> Fixes for reland number 2:
> - "Win10 Tester" browser_tests_functional failure
> - "Win10 Tester" capture_unittests failure
> - "Win8 Tester" browser_tests_functional failure
> - "Win8 Tester" capture_unittests failure
> 
> Fixes for reland number 1:
> - "Win8 Tester" browser_tests_functional failure
> - "Win8 Tester" capture_unittests failure
> 
> Original description:
> - Full rewrite of the MediaFoundation implementation video part to use
> IMFCaptureEngine
> - Implementation of takePhoto, setPhotoOptions and getPhotoCapabilities
> - takePhoto triggers a still image capture with the highest available
> resolution without stopping the video stream thanks to IMFCaptureEngine
> 
> TEST=adapted video_capture_device_unittest.cc and
> webrtc_image_capture_browsertest.cc; launch Chrome with
> --force-mediafoundation on Win8+ and capture video using
> e.g. https://webrtc.github.io/samples/src/content/getusermedia/gum/
> 
> R=​mcasas@chromium.org
> 
> Bug: 730068
> Change-Id: I13fa632b25a345a20465e95a48162064eb2c4cae
> Reviewed-on: https://chromium-review.googlesource.com/734042
> Commit-Queue: Christian Fremerey <chfremer@chromium.org>
> Reviewed-by: Miguel Casas <mcasas@chromium.org>
> Reviewed-by: Christian Fremerey <chfremer@chromium.org>
> Cr-Original-Original-Original-Commit-Position: refs/heads/master@{#521435}
> Reviewed-on: https://chromium-review.googlesource.com/810766
> Cr-Original-Original-Commit-Position: refs/heads/master@{#524417}
> Reviewed-on: https://chromium-review.googlesource.com/843974
> Cr-Original-Commit-Position: refs/heads/master@{#527139}
> Reviewed-on: https://chromium-review.googlesource.com/852455
> Commit-Queue: Miguel Casas <mcasas@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#528005}

TBR=mcasas@chromium.org,chfremer@chromium.org,alaoui.rda@gmail.com

Change-Id: I474f07a244ade53895054543c23e988ab3e00800
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 730068
Reviewed-on: https://chromium-review.googlesource.com/857801
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528091}
[modify] https://crrev.com/40055d8366e465a44a99a89e9edad570d22baf32/content/browser/webrtc/webrtc_image_capture_browsertest.cc
[modify] https://crrev.com/40055d8366e465a44a99a89e9edad570d22baf32/content/test/data/media/image_capture_test.html
[modify] https://crrev.com/40055d8366e465a44a99a89e9edad570d22baf32/media/capture/video/video_capture_device_unittest.cc
[modify] https://crrev.com/40055d8366e465a44a99a89e9edad570d22baf32/media/capture/video/win/video_capture_device_factory_win.cc
[modify] https://crrev.com/40055d8366e465a44a99a89e9edad570d22baf32/media/capture/video/win/video_capture_device_factory_win.h
[modify] https://crrev.com/40055d8366e465a44a99a89e9edad570d22baf32/media/capture/video/win/video_capture_device_mf_win.cc
[modify] https://crrev.com/40055d8366e465a44a99a89e9edad570d22baf32/media/capture/video/win/video_capture_device_mf_win.h

mcasas@, chfremer@,

Among failing tests, I have 2 new ones on Win 8 Tester [1] since the last CL:
- VideoCaptureDeviceTests/VideoCaptureDeviceTest.CaptureMjpeg/1
- VideoCaptureDeviceTests/VideoCaptureDeviceTest.CaptureMjpeg/3

  ../../media/capture/video/video_capture_device_unittest.cc(627): error: Expected equality of these values:
    last_format().pixel_format
      Which is: 1
    PIXEL_FORMAT_MJPEG
      Which is: 14

Those tests ask for MJPEG and expect MJPEG as output.
But since IMFCaptureEngine decode itself the frames to an uncompressed format (that I fixed to I420), those tests will never pass as they are.

How can we proceed to make them pass?

Should we add a flag to VideoCaptureDevice abstraction telling if the implementation emits only uncompressed frames? 
In the mentionned test we would check the new flag.
Or should the flag be instead a fixed output PIXEL_FORMAT (I420 in our case)?

What is your opinion on this?

[1] https://ci.chromium.org/buildbot/chromium.webrtc.fyi/Win8%20Tester/3377
My current thinking on this is that the abstraction represented by interfaces media::VideoCaptureDeviceFactory and media::VideoCaptureDevice should be made consistent in what formats it reports as supported and which formats it delivers.

It seems right now, the implementation reports PIXEL_FORMAT_MJPEG as supported, but when requested it actually delivers (from the perspective of the client of media::VideoCaptureDevice) a different format (I420).

The question is, if the factory only reports I420 as supported, how can a client still differentiate requests to capture I420 directly vs. requests to capture MJPEG and have it decoded. I am thinking that maybe the client should not need to know what happens internally, and just request I420. The implementation can then decide on its own whether to internally capture in MJPEG and decode or capture directly as I420.
Re #50:
Thank you for the answer.

So MF implementation would only enumerate I420 pixel formats from the point of view of the client? 
I am fine with that.
Re #51: Yes, exactly. And it looks like will in that case exit early and pass.

chfremer@, mcasas@,

The last failing test is WebRtcWebcamBrowserTests_WebRtcWebcamBrowserTest.MANUAL_TestAcquiringAndReacquiringWebcam_1 on Win 10 Tester [1] and Win 8 Tester [2].

Here is an excerpt of the tester log:

  [4656:312:0109/090009.957:INFO:CONSOLE(13)] "Requesting doGetUserMedia: 
  constraints: {"video":{"mandatory":
  {"minWidth":640,"maxWidth":640,"minHeight":480,"maxHeight":480}}}", source: 
  http://127.0.0.1:55306/webrtc/test_functions.js (13)
  [4656:312:0109/090010.256:INFO:CONSOLE(13)] "GetUserMedia FAILED: Maybe the 
  camera is in use by another process?", source: 
  http://127.0.0.1:55306/webrtc/test_functions.js (13)
  [4656:312:0109/090010.256:INFO:CONSOLE(13)] "failed-with-error-
  NotReadableError", source: 
  http://127.0.0.1:55306/webrtc/test_functions.js(13)

I am unable to reproduce the issue with my following local configurations:
- C920 + Win 10
- C920 + Win 8.1
- C920 + Win 2012 R2

The trybot logs only the JS side, not the C++ side.
What would be really helpful to me is to retrieve the error that must have been logged by media::VideoCaptureDevice in the trybot.

Could I have an access (restricted?) to the buildbot?
Or can you help me in another way?

[1] https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.webrtc.fyi%2FWin10_Tester%2F10364%2F%2B%2Frecipes%2Fsteps%2Fbrowser_tests_functional%2F0%2Flogs%2FWebRtcWebcamBrowserTests_WebRtcWebcamBrowserTest.MANUAL_TestAcquiringAndReacquiringWebcam_1%2F0

[2] https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.webrtc.fyi%2FWin8_Tester%2F3377%2F%2B%2Frecipes%2Fsteps%2Fbrowser_tests_functional%2F0%2Flogs%2FWebRtcWebcamBrowserTests_WebRtcWebcamBrowserTest.MANUAL_TestAcquiringAndReacquiringWebcam_1%2F0
Hi!

We can't give you access to the bot itself. When you reproduce, have you tried running the actual test or more general gUM tests? If you could repro the failure locally you could add whatever logging you want. You build and run the test like so:

gn gen out/Default
ninja -C out/Default browser_tests
out/Default/browser_tests.exe --gtest_filter="WebRtcWebcamBrowserTests_WebRtcWebcamBrowserTest.MANUAL_TestAcquiringAndReacquiringWebcam_1" --run-manual

There isn't anything magical about the win10 box. It's a regular PC with win10 and a C920 plugged into it, so I think it should repro for you.

If you already tried the above, your only option is to land your patch with more logging turned on, let one build fail on the FYI bots, and then revert. From you log you can see that WARNING:chrome_browser_main_win.cc(616)] gets printed, so if you log on level WARNING from the C++ code and it makes it to stdout on the bot. You should be able to do the same for the videocapturedevice error.
re # 54:

Hello phoglund@,

Thank your for your help.

I always used explicit test commands to reproduce trybot failures (like the one you showed). Nevertheless, I had hard time to reproduce some behaviours. Most of the time it was ok because I could guess what happened thanks to the log (capture unit test).

Maybe Win 10 and Win 8 trybot have not the latest OS updates? I use Windows VM through linux qemu, maybe it affects the way Windows use the camera? 
From my experience, with time, many things between 2 identical OS installations can go south.

I will add logs to the new CL as proposed.

Thank you
I agree that adding the extra logging might be needed, so I think it is a valid approach.

One more thing that can sometimes help with reproing is to double-check that the gn args used for the local build match the ones used on the bot. To find these from the two builds listed in #53, I follow the builder and version number as follows:

Build for Win10 Tester:
https://ci.chromium.org/buildbot/chromium.webrtc.fyi/Win10%20Tester/10364
  parent_buildername  "Win Builder" ParentBuild
  parent_buildnumber  13529 ParentBuild
=> https://ci.chromium.org/buildbot/chromium.webrtc.fyi/Win%20Builder/13529
  stdout for step generate_build_files
=> https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.webrtc.fyi%2FWin_Builder%2F13529%2F%2B%2Frecipes%2Fsteps%2Fgenerate_build_files%2F0%2Fstdout
  com_init_check_hook_disabled = true
  goma_dir = "E:\\b\\c\\goma_client"
  is_component_build = false
  is_debug = false
  strip_absolute_paths_from_debug_symbols = true
  symbol_level = 1
  target_cpu = "x86"
  use_goma = true

Build for Win8 Tester:
https://ci.chromium.org/buildbot/chromium.webrtc.fyi/Win8%20Tester/3377
  parent_buildername  "Win Builder" ParentBuild
  parent_buildnumber  13531 ParentBuild
=> https://ci.chromium.org/buildbot/chromium.webrtc.fyi/Win%20Builder/13531
  stdout for step generate_build_files
=> https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.webrtc.fyi%2FWin_Builder%2F13531%2F%2B%2Frecipes%2Fsteps%2Fgenerate_build_files%2F0%2Fstdout
  com_init_check_hook_disabled = true
  goma_dir = "E:\\b\\c\\goma_client"
  is_component_build = false
  is_debug = false
  strip_absolute_paths_from_debug_symbols = true
  symbol_level = 1
  target_cpu = "x86"
  use_goma = true
I found one more interesting thing (on Linux, but that may apply to Windows as well). The command-line that runs the WebRtcImage* tests on the bots uses a flag --test-launcher-bot-mode. When I use that on my local machine, the tests are executed in parallel (in separate processes). Without the flag, the tests run one by one on my local machine.

It is not surprising that this causes problems, when many of these tests try to access the same single physical camera. The platform API we use on Linux to access the camera does not seem to support using the camera simultaneously from multiple processes. On MacOS it seems that simultaneous camera usage from multiple processes works. I am not sure about Windows.

In either case, this is very likely to be the cause for some flakiness and also Issue 733582.

phoglund@: I think we need to make changes to how we run tests involving a physical camera on the bots. These tests cannot run in parallel.
Oh, I wasn't aware there were more such tests! Yeah, that's easy to fix. Let's talk more in https://bugs.chromium.org/p/chromium/issues/detail?id=733582...
re #57:

"I am not sure about Windows"

It seems that Windows Media Foundation allow concurrent camera access since August 2016 [1].

But I think it is not expected to work when one process uses DirectShow while the other uses MediaFoundation. Maybe this is what is happening.

Therefore, maybe I should disable DirectShow test in [2] and let only MediaFoundation run with real webcam on Windows?

[1] http://alax.info/blog/1686
[2] https://chromium-review.googlesource.com/c/chromium/src/+/858138
I am working on a CL to move all tests that involve a real camera to test runs where test cases are run strictly in sequence, see [1]. With that we should be able to keep the DirectShow and MediaFoundation cases both active.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/872260
Project Member

Comment 61 by bugdroid1@chromium.org, Jan 19 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0f2a785127eeda2c9bdc2d52458d7f0481c2eb13

commit 0f2a785127eeda2c9bdc2d52458d7f0481c2eb13
Author: Christian Fremerey <chfremer@chromium.org>
Date: Fri Jan 19 16:23:01 2018

Prefix content_browsertests that use real webcam with UsingRealWebcam

Before this change, certain image capture tests using a real webcam
would run on webrtc bots as part of a build step called
content_browsertests, which executes multiple test cases in parallel.

By giving all content_browsertests that involve a real webcam a
common prefix "UsingRealWebcam" we make it simpler for the bot
configuration to run these tests in a different build step where we
can ensure they get run sequentially.

Bug: 733582, 730068
Change-Id: I2691f13a454af899a0249932ed1a90e76df8e6b4
Reviewed-on: https://chromium-review.googlesource.com/872260
Reviewed-by: Patrik Höglund <phoglund@chromium.org>
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530529}
[modify] https://crrev.com/0f2a785127eeda2c9bdc2d52458d7f0481c2eb13/content/browser/webrtc/webrtc_image_capture_browsertest.cc
[modify] https://crrev.com/0f2a785127eeda2c9bdc2d52458d7f0481c2eb13/content/browser/webrtc/webrtc_webcam_browsertest.cc
[modify] https://crrev.com/0f2a785127eeda2c9bdc2d52458d7f0481c2eb13/content/browser/webrtc/webrtc_webcam_browsertest.h

Project Member

Comment 62 by bugdroid1@chromium.org, Jan 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3488d01d78be25bb090193038d08d39a3bf561b3

commit 3488d01d78be25bb090193038d08d39a3bf561b3
Author: Réda Housni Alaoui <alaoui.rda@gmail.com>
Date: Tue Jan 23 01:26:31 2018

Reland: Win video capture: use IMFCaptureEngine for Media Foundation

Fixes for reland number 4:
- "Win10 Tester" browser_tests_functional failure
- "Win8 Tester" browser_tests_functional failure
- "Win8 Tester" capture_unittests failure

Fixes for reland number 3:
- "Win10 Tester" browser_tests_functional failure
- "Win10 Tester" capture_unittests failure
- "Win8 Tester" browser_tests_functional failure
- "Win8 Tester" capture_unittests failure

Fixes for reland number 2:
- "Win10 Tester" browser_tests_functional failure
- "Win10 Tester" capture_unittests failure
- "Win8 Tester" browser_tests_functional failure
- "Win8 Tester" capture_unittests failure

Fixes for reland number 1:
- "Win8 Tester" browser_tests_functional failure
- "Win8 Tester" capture_unittests failure

Original description:
- Full rewrite of the MediaFoundation implementation video part to use
IMFCaptureEngine
- Implementation of takePhoto, setPhotoOptions and getPhotoCapabilities
- takePhoto triggers a still image capture with the highest available
resolution without stopping the video stream thanks to IMFCaptureEngine

TEST=adapted video_capture_device_unittest.cc and
webrtc_image_capture_browsertest.cc; launch Chrome with
--force-mediafoundation on Win8+ and capture video using
e.g. https://webrtc.github.io/samples/src/content/getusermedia/gum/

R=mcasas@chromium.org

Bug: 730068
Change-Id: I8da39896ae7b914537f78ce809c9376248409741
Reviewed-on: https://chromium-review.googlesource.com/734042
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Cr-Original-Original-Original-Original-Commit-Position: refs/heads/master@{#521435}
Reviewed-on: https://chromium-review.googlesource.com/810766
Cr-Original-Original-Original-Commit-Position: refs/heads/master@{#524417}
Reviewed-on: https://chromium-review.googlesource.com/843974
Cr-Original-Original-Commit-Position: refs/heads/master@{#527139}
Reviewed-on: https://chromium-review.googlesource.com/852455
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#528005}
Reviewed-on: https://chromium-review.googlesource.com/858138
Cr-Commit-Position: refs/heads/master@{#531109}
[modify] https://crrev.com/3488d01d78be25bb090193038d08d39a3bf561b3/content/browser/webrtc/webrtc_image_capture_browsertest.cc
[modify] https://crrev.com/3488d01d78be25bb090193038d08d39a3bf561b3/content/test/data/media/image_capture_test.html
[modify] https://crrev.com/3488d01d78be25bb090193038d08d39a3bf561b3/media/capture/video/video_capture_device_unittest.cc
[modify] https://crrev.com/3488d01d78be25bb090193038d08d39a3bf561b3/media/capture/video/win/video_capture_device_factory_win.cc
[modify] https://crrev.com/3488d01d78be25bb090193038d08d39a3bf561b3/media/capture/video/win/video_capture_device_factory_win.h
[modify] https://crrev.com/3488d01d78be25bb090193038d08d39a3bf561b3/media/capture/video/win/video_capture_device_mf_win.cc
[modify] https://crrev.com/3488d01d78be25bb090193038d08d39a3bf561b3/media/capture/video/win/video_capture_device_mf_win.h

Project Member

Comment 63 by bugdroid1@chromium.org, Jan 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e69ceeaf10f8130f313efa1a2e137b01e8601773

commit e69ceeaf10f8130f313efa1a2e137b01e8601773
Author: Christian Fremerey <chfremer@chromium.org>
Date: Tue Jan 23 03:10:27 2018

Revert "Reland: Win video capture: use IMFCaptureEngine for Media Foundation"

This reverts commit 3488d01d78be25bb090193038d08d39a3bf561b3.

Reason for revert: Looks like there is still an issue causing test failures on webrtc bots with real cameras, see https://ci.chromium.org/buildbot/chromium.webrtc/Win10%20Tester/24601.

Original change's description:
> Reland: Win video capture: use IMFCaptureEngine for Media Foundation
> 
> Fixes for reland number 4:
> - "Win10 Tester" browser_tests_functional failure
> - "Win8 Tester" browser_tests_functional failure
> - "Win8 Tester" capture_unittests failure
> 
> Fixes for reland number 3:
> - "Win10 Tester" browser_tests_functional failure
> - "Win10 Tester" capture_unittests failure
> - "Win8 Tester" browser_tests_functional failure
> - "Win8 Tester" capture_unittests failure
> 
> Fixes for reland number 2:
> - "Win10 Tester" browser_tests_functional failure
> - "Win10 Tester" capture_unittests failure
> - "Win8 Tester" browser_tests_functional failure
> - "Win8 Tester" capture_unittests failure
> 
> Fixes for reland number 1:
> - "Win8 Tester" browser_tests_functional failure
> - "Win8 Tester" capture_unittests failure
> 
> Original description:
> - Full rewrite of the MediaFoundation implementation video part to use
> IMFCaptureEngine
> - Implementation of takePhoto, setPhotoOptions and getPhotoCapabilities
> - takePhoto triggers a still image capture with the highest available
> resolution without stopping the video stream thanks to IMFCaptureEngine
> 
> TEST=adapted video_capture_device_unittest.cc and
> webrtc_image_capture_browsertest.cc; launch Chrome with
> --force-mediafoundation on Win8+ and capture video using
> e.g. https://webrtc.github.io/samples/src/content/getusermedia/gum/
> 
> R=​mcasas@chromium.org
> 
> Bug: 730068
> Change-Id: I8da39896ae7b914537f78ce809c9376248409741
> Reviewed-on: https://chromium-review.googlesource.com/734042
> Commit-Queue: Christian Fremerey <chfremer@chromium.org>
> Reviewed-by: Miguel Casas <mcasas@chromium.org>
> Reviewed-by: Christian Fremerey <chfremer@chromium.org>
> Cr-Original-Original-Original-Original-Commit-Position: refs/heads/master@{#521435}
> Reviewed-on: https://chromium-review.googlesource.com/810766
> Cr-Original-Original-Original-Commit-Position: refs/heads/master@{#524417}
> Reviewed-on: https://chromium-review.googlesource.com/843974
> Cr-Original-Original-Commit-Position: refs/heads/master@{#527139}
> Reviewed-on: https://chromium-review.googlesource.com/852455
> Commit-Queue: Miguel Casas <mcasas@chromium.org>
> Cr-Original-Commit-Position: refs/heads/master@{#528005}
> Reviewed-on: https://chromium-review.googlesource.com/858138
> Cr-Commit-Position: refs/heads/master@{#531109}

TBR=mcasas@chromium.org,chfremer@chromium.org,alaoui.rda@gmail.com,danielparada232016@gmail.com

Change-Id: I1aa99ade74de9ba78a485f1c0a2ad96f0a275708
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 730068
Reviewed-on: https://chromium-review.googlesource.com/880041
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531138}
[modify] https://crrev.com/e69ceeaf10f8130f313efa1a2e137b01e8601773/content/browser/webrtc/webrtc_image_capture_browsertest.cc
[modify] https://crrev.com/e69ceeaf10f8130f313efa1a2e137b01e8601773/content/test/data/media/image_capture_test.html
[modify] https://crrev.com/e69ceeaf10f8130f313efa1a2e137b01e8601773/media/capture/video/video_capture_device_unittest.cc
[modify] https://crrev.com/e69ceeaf10f8130f313efa1a2e137b01e8601773/media/capture/video/win/video_capture_device_factory_win.cc
[modify] https://crrev.com/e69ceeaf10f8130f313efa1a2e137b01e8601773/media/capture/video/win/video_capture_device_factory_win.h
[modify] https://crrev.com/e69ceeaf10f8130f313efa1a2e137b01e8601773/media/capture/video/win/video_capture_device_mf_win.cc
[modify] https://crrev.com/e69ceeaf10f8130f313efa1a2e137b01e8601773/media/capture/video/win/video_capture_device_mf_win.h

Project Member

Comment 64 by bugdroid1@chromium.org, Jan 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e69ceeaf10f8130f313efa1a2e137b01e8601773

commit e69ceeaf10f8130f313efa1a2e137b01e8601773
Author: Christian Fremerey <chfremer@chromium.org>
Date: Tue Jan 23 03:10:27 2018

Revert "Reland: Win video capture: use IMFCaptureEngine for Media Foundation"

This reverts commit 3488d01d78be25bb090193038d08d39a3bf561b3.

Reason for revert: Looks like there is still an issue causing test failures on webrtc bots with real cameras, see https://ci.chromium.org/buildbot/chromium.webrtc/Win10%20Tester/24601.

Original change's description:
> Reland: Win video capture: use IMFCaptureEngine for Media Foundation
> 
> Fixes for reland number 4:
> - "Win10 Tester" browser_tests_functional failure
> - "Win8 Tester" browser_tests_functional failure
> - "Win8 Tester" capture_unittests failure
> 
> Fixes for reland number 3:
> - "Win10 Tester" browser_tests_functional failure
> - "Win10 Tester" capture_unittests failure
> - "Win8 Tester" browser_tests_functional failure
> - "Win8 Tester" capture_unittests failure
> 
> Fixes for reland number 2:
> - "Win10 Tester" browser_tests_functional failure
> - "Win10 Tester" capture_unittests failure
> - "Win8 Tester" browser_tests_functional failure
> - "Win8 Tester" capture_unittests failure
> 
> Fixes for reland number 1:
> - "Win8 Tester" browser_tests_functional failure
> - "Win8 Tester" capture_unittests failure
> 
> Original description:
> - Full rewrite of the MediaFoundation implementation video part to use
> IMFCaptureEngine
> - Implementation of takePhoto, setPhotoOptions and getPhotoCapabilities
> - takePhoto triggers a still image capture with the highest available
> resolution without stopping the video stream thanks to IMFCaptureEngine
> 
> TEST=adapted video_capture_device_unittest.cc and
> webrtc_image_capture_browsertest.cc; launch Chrome with
> --force-mediafoundation on Win8+ and capture video using
> e.g. https://webrtc.github.io/samples/src/content/getusermedia/gum/
> 
> R=​mcasas@chromium.org
> 
> Bug: 730068
> Change-Id: I8da39896ae7b914537f78ce809c9376248409741
> Reviewed-on: https://chromium-review.googlesource.com/734042
> Commit-Queue: Christian Fremerey <chfremer@chromium.org>
> Reviewed-by: Miguel Casas <mcasas@chromium.org>
> Reviewed-by: Christian Fremerey <chfremer@chromium.org>
> Cr-Original-Original-Original-Original-Commit-Position: refs/heads/master@{#521435}
> Reviewed-on: https://chromium-review.googlesource.com/810766
> Cr-Original-Original-Original-Commit-Position: refs/heads/master@{#524417}
> Reviewed-on: https://chromium-review.googlesource.com/843974
> Cr-Original-Original-Commit-Position: refs/heads/master@{#527139}
> Reviewed-on: https://chromium-review.googlesource.com/852455
> Commit-Queue: Miguel Casas <mcasas@chromium.org>
> Cr-Original-Commit-Position: refs/heads/master@{#528005}
> Reviewed-on: https://chromium-review.googlesource.com/858138
> Cr-Commit-Position: refs/heads/master@{#531109}

TBR=mcasas@chromium.org,chfremer@chromium.org,alaoui.rda@gmail.com,danielparada232016@gmail.com

Change-Id: I1aa99ade74de9ba78a485f1c0a2ad96f0a275708
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 730068
Reviewed-on: https://chromium-review.googlesource.com/880041
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531138}
[modify] https://crrev.com/e69ceeaf10f8130f313efa1a2e137b01e8601773/content/browser/webrtc/webrtc_image_capture_browsertest.cc
[modify] https://crrev.com/e69ceeaf10f8130f313efa1a2e137b01e8601773/content/test/data/media/image_capture_test.html
[modify] https://crrev.com/e69ceeaf10f8130f313efa1a2e137b01e8601773/media/capture/video/video_capture_device_unittest.cc
[modify] https://crrev.com/e69ceeaf10f8130f313efa1a2e137b01e8601773/media/capture/video/win/video_capture_device_factory_win.cc
[modify] https://crrev.com/e69ceeaf10f8130f313efa1a2e137b01e8601773/media/capture/video/win/video_capture_device_factory_win.h
[modify] https://crrev.com/e69ceeaf10f8130f313efa1a2e137b01e8601773/media/capture/video/win/video_capture_device_mf_win.cc
[modify] https://crrev.com/e69ceeaf10f8130f313efa1a2e137b01e8601773/media/capture/video/win/video_capture_device_mf_win.h

Project Member

Comment 65 by bugdroid1@chromium.org, Jan 25 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2e8b15012048302db69790bef8818a6ed758eed7

commit 2e8b15012048302db69790bef8818a6ed758eed7
Author: Réda Housni Alaoui <alaoui.rda@gmail.com>
Date: Thu Jan 25 02:17:51 2018

Reland: Win video capture: use IMFCaptureEngine for Media Foundation

Fixes for reland number 5:
- "Win10 Tester" browser_tests_functional failure
- "Win8 Tester" browser_tests_functional failure

Fixes for reland number 4:
- "Win10 Tester" browser_tests_functional failure
- "Win8 Tester" browser_tests_functional failure
- "Win8 Tester" capture_unittests failure

Fixes for reland number 3:
- "Win10 Tester" browser_tests_functional failure
- "Win10 Tester" capture_unittests failure
- "Win8 Tester" browser_tests_functional failure
- "Win8 Tester" capture_unittests failure

Fixes for reland number 2:
- "Win10 Tester" browser_tests_functional failure
- "Win10 Tester" capture_unittests failure
- "Win8 Tester" browser_tests_functional failure
- "Win8 Tester" capture_unittests failure

Fixes for reland number 1:
- "Win8 Tester" browser_tests_functional failure
- "Win8 Tester" capture_unittests failure

Original description:
- Full rewrite of the MediaFoundation implementation video part to use
IMFCaptureEngine
- Implementation of takePhoto, setPhotoOptions and getPhotoCapabilities
- takePhoto triggers a still image capture with the highest available
resolution without stopping the video stream thanks to IMFCaptureEngine

TEST=adapted video_capture_device_unittest.cc and
webrtc_image_capture_browsertest.cc; launch Chrome with
--force-mediafoundation on Win8+ and capture video using
e.g. https://webrtc.github.io/samples/src/content/getusermedia/gum/

TBR=mcasas@chromium.org

Bug: 730068
Change-Id: If081d29402e9f595a3fd1906e45a3bec1d712b3a
Reviewed-on: https://chromium-review.googlesource.com/734042
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Cr-Original-Original-Original-Original-Original-Commit-Position: refs/heads/master@{#521435}
Reviewed-on: https://chromium-review.googlesource.com/810766
Cr-Original-Original-Original-Original-Commit-Position: refs/heads/master@{#524417}
Reviewed-on: https://chromium-review.googlesource.com/843974
Cr-Original-Original-Original-Commit-Position: refs/heads/master@{#527139}
Reviewed-on: https://chromium-review.googlesource.com/852455
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Original-Original-Commit-Position: refs/heads/master@{#528005}
Reviewed-on: https://chromium-review.googlesource.com/858138
Cr-Original-Commit-Position: refs/heads/master@{#531109}
Reviewed-on: https://chromium-review.googlesource.com/880944
Cr-Commit-Position: refs/heads/master@{#531791}
[modify] https://crrev.com/2e8b15012048302db69790bef8818a6ed758eed7/content/browser/webrtc/webrtc_image_capture_browsertest.cc
[modify] https://crrev.com/2e8b15012048302db69790bef8818a6ed758eed7/content/test/data/media/image_capture_test.html
[modify] https://crrev.com/2e8b15012048302db69790bef8818a6ed758eed7/media/capture/video/video_capture_device_unittest.cc
[modify] https://crrev.com/2e8b15012048302db69790bef8818a6ed758eed7/media/capture/video/win/video_capture_device_factory_win.cc
[modify] https://crrev.com/2e8b15012048302db69790bef8818a6ed758eed7/media/capture/video/win/video_capture_device_factory_win.h
[modify] https://crrev.com/2e8b15012048302db69790bef8818a6ed758eed7/media/capture/video/win/video_capture_device_mf_win.cc
[modify] https://crrev.com/2e8b15012048302db69790bef8818a6ed758eed7/media/capture/video/win/video_capture_device_mf_win.h

Project Member

Comment 66 by bugdroid1@chromium.org, Jan 25 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b9e02304907cb3ddf98ec4a1303bda16a4378073

commit b9e02304907cb3ddf98ec4a1303bda16a4378073
Author: Christian Fremerey <chfremer@chromium.org>
Date: Thu Jan 25 06:36:45 2018

Revert "Reland: Win video capture: use IMFCaptureEngine for Media Foundation"

This reverts commit 2e8b15012048302db69790bef8818a6ed758eed7.

Reason for revert: Failing tests on webrtc bots, e.g. https://ci.chromium.org/buildbot/chromium.webrtc/Win10%20Tester/24669

Original change's description:
> Reland: Win video capture: use IMFCaptureEngine for Media Foundation
> 
> Fixes for reland number 5:
> - "Win10 Tester" browser_tests_functional failure
> - "Win8 Tester" browser_tests_functional failure
> 
> Fixes for reland number 4:
> - "Win10 Tester" browser_tests_functional failure
> - "Win8 Tester" browser_tests_functional failure
> - "Win8 Tester" capture_unittests failure
> 
> Fixes for reland number 3:
> - "Win10 Tester" browser_tests_functional failure
> - "Win10 Tester" capture_unittests failure
> - "Win8 Tester" browser_tests_functional failure
> - "Win8 Tester" capture_unittests failure
> 
> Fixes for reland number 2:
> - "Win10 Tester" browser_tests_functional failure
> - "Win10 Tester" capture_unittests failure
> - "Win8 Tester" browser_tests_functional failure
> - "Win8 Tester" capture_unittests failure
> 
> Fixes for reland number 1:
> - "Win8 Tester" browser_tests_functional failure
> - "Win8 Tester" capture_unittests failure
> 
> Original description:
> - Full rewrite of the MediaFoundation implementation video part to use
> IMFCaptureEngine
> - Implementation of takePhoto, setPhotoOptions and getPhotoCapabilities
> - takePhoto triggers a still image capture with the highest available
> resolution without stopping the video stream thanks to IMFCaptureEngine
> 
> TEST=adapted video_capture_device_unittest.cc and
> webrtc_image_capture_browsertest.cc; launch Chrome with
> --force-mediafoundation on Win8+ and capture video using
> e.g. https://webrtc.github.io/samples/src/content/getusermedia/gum/
> 
> TBR=mcasas@chromium.org
> 
> Bug: 730068
> Change-Id: If081d29402e9f595a3fd1906e45a3bec1d712b3a
> Reviewed-on: https://chromium-review.googlesource.com/734042
> Commit-Queue: Christian Fremerey <chfremer@chromium.org>
> Reviewed-by: Miguel Casas <mcasas@chromium.org>
> Reviewed-by: Christian Fremerey <chfremer@chromium.org>
> Cr-Original-Original-Original-Original-Original-Commit-Position: refs/heads/master@{#521435}
> Reviewed-on: https://chromium-review.googlesource.com/810766
> Cr-Original-Original-Original-Original-Commit-Position: refs/heads/master@{#524417}
> Reviewed-on: https://chromium-review.googlesource.com/843974
> Cr-Original-Original-Original-Commit-Position: refs/heads/master@{#527139}
> Reviewed-on: https://chromium-review.googlesource.com/852455
> Commit-Queue: Miguel Casas <mcasas@chromium.org>
> Cr-Original-Original-Commit-Position: refs/heads/master@{#528005}
> Reviewed-on: https://chromium-review.googlesource.com/858138
> Cr-Original-Commit-Position: refs/heads/master@{#531109}
> Reviewed-on: https://chromium-review.googlesource.com/880944
> Cr-Commit-Position: refs/heads/master@{#531791}

TBR=mcasas@chromium.org,chfremer@chromium.org,alaoui.rda@gmail.com

Change-Id: Idae6023d22c51e2df66687729ea0df5232c67fc6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 730068
Reviewed-on: https://chromium-review.googlesource.com/886081
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531831}
[modify] https://crrev.com/b9e02304907cb3ddf98ec4a1303bda16a4378073/content/browser/webrtc/webrtc_image_capture_browsertest.cc
[modify] https://crrev.com/b9e02304907cb3ddf98ec4a1303bda16a4378073/content/test/data/media/image_capture_test.html
[modify] https://crrev.com/b9e02304907cb3ddf98ec4a1303bda16a4378073/media/capture/video/video_capture_device_unittest.cc
[modify] https://crrev.com/b9e02304907cb3ddf98ec4a1303bda16a4378073/media/capture/video/win/video_capture_device_factory_win.cc
[modify] https://crrev.com/b9e02304907cb3ddf98ec4a1303bda16a4378073/media/capture/video/win/video_capture_device_factory_win.h
[modify] https://crrev.com/b9e02304907cb3ddf98ec4a1303bda16a4378073/media/capture/video/win/video_capture_device_mf_win.cc
[modify] https://crrev.com/b9e02304907cb3ddf98ec4a1303bda16a4378073/media/capture/video/win/video_capture_device_mf_win.h

Project Member

Comment 67 by bugdroid1@chromium.org, Jan 26 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cb078a8adb14a1d3420231253a98819002087028

commit cb078a8adb14a1d3420231253a98819002087028
Author: Réda Housni Alaoui <alaoui.rda@gmail.com>
Date: Fri Jan 26 19:40:16 2018

Reland: Win video capture: use IMFCaptureEngine for Media Foundation

Fixes for reland number 6:
- "Win10 Tester" capture_unittests failure
- "Win8 Tester" capture_unittests failure

Fixes for reland number 5:
- "Win10 Tester" browser_tests_functional failure
- "Win8 Tester" browser_tests_functional failure

Fixes for reland number 4:
- "Win10 Tester" browser_tests_functional failure
- "Win8 Tester" browser_tests_functional failure
- "Win8 Tester" capture_unittests failure

Fixes for reland number 3:
- "Win10 Tester" browser_tests_functional failure
- "Win10 Tester" capture_unittests failure
- "Win8 Tester" browser_tests_functional failure
- "Win8 Tester" capture_unittests failure

Fixes for reland number 2:
- "Win10 Tester" browser_tests_functional failure
- "Win10 Tester" capture_unittests failure
- "Win8 Tester" browser_tests_functional failure
- "Win8 Tester" capture_unittests failure

Fixes for reland number 1:
- "Win8 Tester" browser_tests_functional failure
- "Win8 Tester" capture_unittests failure

Original description:
- Full rewrite of the MediaFoundation implementation video part to use
IMFCaptureEngine
- Implementation of takePhoto, setPhotoOptions and getPhotoCapabilities
- takePhoto triggers a still image capture with the highest available
resolution without stopping the video stream thanks to IMFCaptureEngine

TEST=adapted video_capture_device_unittest.cc and
webrtc_image_capture_browsertest.cc; launch Chrome with
--force-mediafoundation on Win8+ and capture video using
e.g. https://webrtc.github.io/samples/src/content/getusermedia/gum/

R=mcasas@chromium.org

Bug: 730068
Change-Id: I7b7ff88f2db8d71f46428a2ecbb733e18a25a334
Reviewed-on: https://chromium-review.googlesource.com/734042
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Cr-Original-Original-Original-Original-Original-Commit-Position: refs/heads/master@{#521435}
Reviewed-on: https://chromium-review.googlesource.com/810766
Cr-Original-Original-Original-Original-Commit-Position: refs/heads/master@{#524417}
Reviewed-on: https://chromium-review.googlesource.com/843974
Cr-Original-Original-Original-Commit-Position: refs/heads/master@{#527139}
Reviewed-on: https://chromium-review.googlesource.com/852455
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Original-Original-Commit-Position: refs/heads/master@{#528005}
Reviewed-on: https://chromium-review.googlesource.com/858138
Cr-Original-Commit-Position: refs/heads/master@{#531109}
Reviewed-on: https://chromium-review.googlesource.com/885815
Cr-Commit-Position: refs/heads/master@{#532040}
[modify] https://crrev.com/cb078a8adb14a1d3420231253a98819002087028/content/browser/webrtc/webrtc_image_capture_browsertest.cc
[modify] https://crrev.com/cb078a8adb14a1d3420231253a98819002087028/content/test/data/media/image_capture_test.html
[modify] https://crrev.com/cb078a8adb14a1d3420231253a98819002087028/media/capture/video/video_capture_device_unittest.cc
[modify] https://crrev.com/cb078a8adb14a1d3420231253a98819002087028/media/capture/video/win/capability_list_win.h
[modify] https://crrev.com/cb078a8adb14a1d3420231253a98819002087028/media/capture/video/win/video_capture_device_factory_win.cc
[modify] https://crrev.com/cb078a8adb14a1d3420231253a98819002087028/media/capture/video/win/video_capture_device_factory_win.h
[modify] https://crrev.com/cb078a8adb14a1d3420231253a98819002087028/media/capture/video/win/video_capture_device_mf_win.cc
[modify] https://crrev.com/cb078a8adb14a1d3420231253a98819002087028/media/capture/video/win/video_capture_device_mf_win.h
[modify] https://crrev.com/cb078a8adb14a1d3420231253a98819002087028/media/capture/video/win/video_capture_device_win.cc

Looks like the bots are okay with the latest reland. Woohoo!

https://ci.chromium.org/buildbot/chromium.webrtc.fyi/Win10%20Tester/10604
https://ci.chromium.org/buildbot/chromium.webrtc.fyi/Win8%20Tester/3615
https://ci.chromium.org/buildbot/chromium.webrtc/Win8%20Tester/39996
https://ci.chromium.org/buildbot/chromium.webrtc/Win10%20Tester/24713

In the meantime, I (re-)realized that the image capture test cases currently do not run on the Windows bots, including the newly added cases for the Media Foundation code path.

https://cs.chromium.org/chromium/src/content/browser/webrtc/webrtc_image_capture_browsertest.cc?q=UsingRealWebcam&l=27

As a next step we should try to enable them. Since they are now already configured/named to no longer run in parallel, I wouldn't be surprised if that just works without any additional change.


chfremer@, mcasas@,

I think it was the last reland, because all tests are green!

:D
re #68:
Oh, didn't see your message.
Woohoo yeah !

Is it the next step toward --force-mediafoundation flag deletion?
Do you want me to do it?
re #70: If you have the time, please feel free to set up the CL for that.

After that, the next step on the path toward rolling out the feature would IMO be to mock out the MediaFoundation calls in video_capture_device_mf_win.cc and write tests that confirm that the implementation behaves as expected in the various (sometimes strange) cases of how MediaFoundation behaves with real cameras.
Re #71:
To which ticket should I link the CL enabling windows image capture testing?
alaoui.rda@gmail.com, chfremer@, great work on this.

I just noticed that there is regression with depth camera capture (raw 16 bit):

"No suitable transform was found to encode or decode the content" [1]
So, the same reason as with MJPEG in Comment 41: Source and Sink logic.

You could check the details for kMediaSubTypeZ16. Also, don't know if the logic could be shared with https://cs.chromium.org/chromium/src/media/capture/video/win/sink_input_pin_win.cc but probably you checked already.

Understand you loose some time on revert&reland. Tests are not covering it. With the tests now added, it should be easier to maintain this. Thanks for that too.

If you have no RealSense camera to test the changes, I could try.

[1]
https://social.msdn.microsoft.com/Forums/en-US/090e44f9-c55f-4f8a-bf7e-ac60060b8422/imfcaptureengine-with-mjpeg-camera-output-no-suitable-transform-was-found-to-encode-or-decode-the?forum=mediafoundationdevelopment


Blocking: 807293
Re #74:

aleksand...@intel.com, thanks for your kind words.

The MSDN topic that you gave was created by me and solved by me!
My login on the forum is indeed reda-alaoui ;)

So the solution that I exposed on MSDN is part of the current implementation. This is why in the current implementation, the true camera video format is fixed to any of the enumerated camera video format and the sink format is always set to I420.

kMediaSubTypeZ16 is already mapped in the implementation [1]. 
So with your camera, the source should be set to kMediaSubTypeZ16 and the sink to I420.

Maybe ConvertToVideoSinkMediaType() [2] performs an incorrect conversion for your camera? Could you try to debug that?

[1] https://chromium.googlesource.com/chromium/src.git/+/master/media/capture/video/win/video_capture_device_mf_win.cc#352
[2] https://chromium.googlesource.com/chromium/src.git/+/master/media/capture/video/win/video_capture_device_mf_win.cc#192

aleksand...@intel.com, the tool downloadable at [1] allows to enumerate the device MF capabilities in a markdown file. Could you give us this output with your camera?

[1] http://alax.info/blog/1579
alaoui.rda@gmail.com,

"So with your camera, the source should be set to kMediaSubTypeZ16 and the sink to I420."

Intel RealSense SR300 exposes INVZ format [1] while previous models expose Z16 and Y16.

I have verified that this fixes the problem with all the cameras:
1) 
Code at [1] should be changed to:
      {kMediaSubTypeY16, PIXEL_FORMAT_Y16},
      {kMediaSubTypeZ16, PIXEL_FORMAT_Y16},
      {kMediaSubTypeINVZ, PIXEL_FORMAT_Y16},

2) 
ConvertToVideoSinkMediaType() [2]
It needs to be same as source subtype GUID (kMediaSubTypeZ16, kMediaSubTypeY16 or kMediaSubTypeINVZ).

[1] https://chromium.googlesource.com/chromium/src.git/+/master/media/capture/video/win/video_capture_device_mf_win.cc#352
[2] https://chromium.googlesource.com/chromium/src.git/+/master/media/capture/video/win/video_capture_device_mf_win.cc#192


Ok I will create CL with that fix once my build is over.
Unless you want to create the CL?
Thanks. If you assemble the patch I could run the tests. That would be the most productive at the moment as you would refactor your code to pass the original source subtype (or read it again).

Ok.
I think I will add also a unit test for this.
aleksand...@intel.com, is there no possible MFTransform from Z16 to Y16 and from INVZ to Y16?
#82
Doesn't look there is:
Forcing INVZ to D16 or vice versa results with error.
D435 camera [1] declares MFVideoFormat_D16 (fourcc value is D3DFMT_D16) stream, and although the memory layout is exactly the same as MFVideoFormat_L16, kMediaSubTypeY16, kMediaSubTypeZ16 and kMediaSubTypeINVZ - 16bpp raw data - there shouldn't be any transcoding happening. Original data should be passed...
I believe it should be the same for MFVideoFormat_MJPG, as mentioned in review request [2].


[1] https://click.intel.com/intelr-realsensetm-depth-camera-d435.html
[2] https://chromium-review.googlesource.com/c/chromium/src/+/918001#message-72f8268d719a7efea50bf9b15de99654a2e19c16
Re #83:

I feel the need to explain why we forced I420 for the sink on standard formats.

In preview mode, IMFCaptureEngine performs the frame decompression itself by automatically setting an MFTransform.
Therefore, IMFCapturePreviewSink must always be configured with an uncompressed video format [1]. We chose I420 since it is the format used by chromium in the end.

This is why we had the issue described in #41. 
We were setting the source to MJPEG and the sink to MJPEG. 
This can't work with the preview sink.

So I think that formats like Y16, Z16 and INVZ, that are unknown to MediaFoundation must be specially handled in a "passthrough" mode since IMFCaptureEngine would be unable to set the correct MFTransform.

[1] https://msdn.microsoft.com/en-us/library/windows/desktop/hh447883(v=vs.85).aspx (Take a look at "Preview sink: An uncompressed audio or video format" line)
Edit:

*uncompressed* formats like Y16 ...
re #84: So far, for color video streams, the Chromium video capture stack downstream from here prefers to get I420 (otherwise it does its own conversion to I420 in class VideoCaptureDeviceClient). For depth video streams, the situation is different, because they are "monochrome" and so I420 does not make a lot of sense for them. Therefore, besides I420, we also allow Y16 to get passed through VideoCaptureDeviceClient without conversion, see [1].

[1] https://cs.chromium.org/chromium/src/media/capture/video/video_capture_device_client.cc?q=VideoCaptureDeviceClient&dr=CSs&l=31
Re [1]

chfremer@,

> It could indeed be because MF imposes that a "preview" stream is intended for immediate rendering and therefore ought to be uncompressed, even though it feels somewhat "impolite" that it forces the use of its own decoding.

From my understanding, MF is being "impolite" because it recently introduced application concurrent camera access with FrameServer [2][3]. With this introduction, Microsoft decided that MF would be the one platform decoding compressed frames in order to mutualise the frame decoding effort among multiple video frame clients requesting display of compressed formats (i.e. MJPG).

> If, in the future, we were to actually want to be able to get an encoded MJPEG stream, would it be possible to support this, e.g. by using a stream that is not "preview"?

I think we could do that by using IMFCaptureRecordSink [4][5]

> If we require pass-through of these formats, then we should add them to the PIXEL_FORMAT_**** enum to allow the client code to decide what to do with it.

I agree with that.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/918001/3/media/capture/video/win/video_capture_device_mf_win.cc#199
[2] http://alax.info/blog/1686
[3] https://social.msdn.microsoft.com/Forums/windowsdesktop/en-US/9d6a8704-764f-46df-a41c-8e9d84f7f0f3/mjpg-encoded-media-type-is-not-available-for-usbuvc-webcameras-after-windows-10-version-1607-os?forum=mediafoundationdevelopment
[4] https://msdn.microsoft.com/en-us/library/windows/desktop/hh447875(v=vs.85).aspx
[5] https://msdn.microsoft.com/en-us/library/windows/desktop/hh447882%28v=vs.85%29.aspx?f=255&MSPPError=-2147217396 (Recording sink. Creates compressed audio/video files or compressed audio/video streams.)
alaoui.rda@gmail.com,
Thanks for links. Good read.
About MJPEG.
Given someone has the needed camera...
1)Windows with FrameServer doesn’t provide MJPEG stream.
2)Windows before (or with registry disabled?) FrameServer provides MJPEG stream.
Doesn’t it make sense to mark MJPEG as passthrough. For case 1 it is noop and for case two it makes it behave as it was before and as it is in DirectShow approach?



Project Member

Comment 89 by bugdroid1@chromium.org, Feb 15 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/26c242a77261d92fd10fadddd21ba7223b1f8fa1

commit 26c242a77261d92fd10fadddd21ba7223b1f8fa1
Author: Réda Housni Alaoui <alaoui.rda@gmail.com>
Date: Thu Feb 15 00:30:01 2018

Fix depth camera MediaFoundation video capture failure

Depth camera video capture is working with DirectShow but
not with MediaFoundation. This comes from the fact that
IMFCaptureEngine is unable to perform encoding/decoding on non standard
MediaFoundation formats Y16, Z16 and INVZ.

To fix it, formats Y16, Z16 and INZV are set equally in the source and
the preview sink.

Bug: 730068
Change-Id: I66bd25f9b8796e4ed564df1e3a9867dcbd9f9522
Reviewed-on: https://chromium-review.googlesource.com/918001
Reviewed-by: Aleksandar Stojiljkovic <aleksandar.stojiljkovic@intel.com>
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536908}
[modify] https://crrev.com/26c242a77261d92fd10fadddd21ba7223b1f8fa1/media/capture/video/win/sink_filter_win.cc
[modify] https://crrev.com/26c242a77261d92fd10fadddd21ba7223b1f8fa1/media/capture/video/win/sink_filter_win.h
[modify] https://crrev.com/26c242a77261d92fd10fadddd21ba7223b1f8fa1/media/capture/video/win/video_capture_device_factory_win.cc
[modify] https://crrev.com/26c242a77261d92fd10fadddd21ba7223b1f8fa1/media/capture/video/win/video_capture_device_mf_win.cc
[modify] https://crrev.com/26c242a77261d92fd10fadddd21ba7223b1f8fa1/media/capture/video/win/video_capture_device_mf_win.h
[modify] https://crrev.com/26c242a77261d92fd10fadddd21ba7223b1f8fa1/media/capture/video/win/video_capture_device_mf_win_unittest.cc

Re #88

>for case two it makes it behave as it was before and as it is in DirectShow approach?

I suppose. I didn't check that.

IMO, it is a good think that Chromium can use a camera without exclusive access. 
This should allow Chromium to use a camera already used by a third party application (if the third party application didn't require exclusive access) and vice versa without performance deterioration.

Shouldn't we welcome this behaviour with the associated constraint on IMFCapturePreviewSink media subtype?
Agree, this is a significant improvement. Great work.

#88 is only about change in MJPEG handling in MF code and the need to verify it.

Status: Assigned (was: Available)
Blockedon: 735576
Project Member

Comment 94 by bugdroid1@chromium.org, Apr 10 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8f68d00946dd1c77f0e5936561146a5110ebc8d7

commit 8f68d00946dd1c77f0e5936561146a5110ebc8d7
Author: Réda Housni Alaoui <alaoui.rda@gmail.com>
Date: Tue Apr 10 16:13:18 2018

[Video Capture Win] Append DirectShow virtual cameras to MediaFoundation
enumeration

MediaFoundation can't handle DirectShow virtual cameras.
This is to append MediaFoundation unsupported DirectShow cameras to the
camera enumeration supplied to the consumer.

Bug: 730068
Change-Id: I3240e2158f822c04a193ec20a091c1f17ee2e373
Reviewed-on: https://chromium-review.googlesource.com/1000853
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549550}
[modify] https://crrev.com/8f68d00946dd1c77f0e5936561146a5110ebc8d7/media/capture/video/win/video_capture_device_factory_win.cc
[modify] https://crrev.com/8f68d00946dd1c77f0e5936561146a5110ebc8d7/media/capture/video/win/video_capture_device_factory_win.h
[modify] https://crrev.com/8f68d00946dd1c77f0e5936561146a5110ebc8d7/media/capture/video/win/video_capture_device_factory_win_unittest.cc

chfremer@, could you take a look at https://bugs.chromium.org/p/monorail/issues/detail?id=3701 ?

Do you know if anything related to VideoCaptureDevice::getPhotoState changed recently?

Sign in to add a comment