1308412 - Webcams based on CoreMedia I/O plugin don't work since FF switch to AVFoundation

Status: VERIFIED FIXED

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

Description

[Dmitriy Kravchenko]

Attachment: Facebook_FF49.png

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:44.0) Gecko/20100101 Firefox/44.0
Build ID: 20160210153822

Steps to reproduce:

1. Under OS X 10.9 or higher install ManyCam Virtual Webcam (http://www.manycam.com) or an Apple sample for CoreMedia I/O plugin from here: http://developer.apple.com/library/mac/samplecode/CoreMediaIO/Introduction/Intro.html. You may use a prebuilt binaries for Apple sample, but you also have to disable signature check for Kexts to have it work.
2. Open Firefox 49 and go to any site that leverages WebRTC, for example, https://www.facebook.com or https://test.webrtc.org.
3. Use ManyCam Virtual Webcam or Apple Sample as webcam and open video in Facebook call or run a test on https://test.webrtc.org.


Actual results:

Entirely black video screen in Facebook call (attached file Facebook_FF49.png).
'Camera' section in test.webrtc.org report:

For 'ManyCam Virtual Webcam' webcam on OS X 10.12 and Firefox 49.0.1 (only appeared in the webcam list for resolutions 640x480 and 1280x720):

Check resolution 640x480
[ INFO ] cameraName: ManyCam Virtual Webcam
[ INFO ] actualVideoWidth: 0
[ INFO ] actualVideoHeight: 0
[ INFO ] mandatoryWidth: 640
[ INFO ] mandatoryHeight: 480
[ INFO ] isMuted: false
[ INFO ] testedFrames: 108
[ INFO ] blackFrames: 108
[ INFO ] frozenFrames: 107
[ INFO ] Not available: encodeSetupTimeMs, avgEncodeTimeMs, minEncodeTimeMs, maxEncodeTimeMs, avgInputFps, minInputFps, maxInputFps, avgSentFps, minSentFps, maxSentFps
[ INFO ] Cannot verify sent FPS.
[ FAILED ] Incorrect captured resolution.
[ FAILED ] Camera delivering lots of black frames.
[ FAILED ] Camera delivering lots of frozen frames.
Check resolution 1280x720
[ INFO ] cameraName: ManyCam Virtual Webcam
[ INFO ] actualVideoWidth: 0
[ INFO ] actualVideoHeight: 0
[ INFO ] mandatoryWidth: 1280
[ INFO ] mandatoryHeight: 720
[ INFO ] isMuted: false
[ INFO ] testedFrames: 58
[ INFO ] blackFrames: 58
[ INFO ] frozenFrames: 57
[ INFO ] Not available: encodeSetupTimeMs, avgEncodeTimeMs, minEncodeTimeMs, maxEncodeTimeMs, avgInputFps, minInputFps, maxInputFps, avgSentFps, minSentFps, maxSentFps
[ INFO ] Cannot verify sent FPS.
[ FAILED ] Incorrect captured resolution.
[ FAILED ] Camera delivering lots of black frames.
[ FAILED ] Camera delivering lots of frozen frames.

------------------------------------------------------------------------------------------------------------
For 'Apple Sample' webcam on OS X 10.9.5 and Firefox 49.0.1 (it only appeared in the webcam list for 1280x720 resolution):

 Check resolution 1280x720
[ INFO ] cameraName: Sample
[ INFO ] actualVideoWidth: 0
[ INFO ] actualVideoHeight: 0
[ INFO ] mandatoryWidth: 1280
[ INFO ] mandatoryHeight: 720
[ INFO ] isMuted: false
[ INFO ] testedFrames: 42
[ INFO ] blackFrames: 42
[ INFO ] frozenFrames: 41
[ INFO ] Not available: encodeSetupTimeMs, avgEncodeTimeMs, minEncodeTimeMs, maxEncodeTimeMs, avgInputFps, minInputFps, maxInputFps, avgSentFps, minSentFps, maxSentFps
[ INFO ] Cannot verify sent FPS.
[ FAILED ] Incorrect captured resolution.
[ FAILED ] Camera delivering lots of black frames.
[ FAILED ] Camera delivering lots of frozen frames.




Expected results:

The video from webcam is shown during call in Facebook. Apple Sample webcam shows video with time counter (see attached Facebook_FF_before_49.png). It was shown under the same conditions in previous FF version.

'Camera' section in test.webrtc.org report shows this (the output is from Firefox):

For 'ManyCam Virtual Webcam' webcam on OS X 10.11 and Firefox 38.0.5:

Check resolution 320x240
[ INFO ] actualVideoWidth: 320
[ INFO ] actualVideoHeight: 240
[ INFO ] mandatoryWidth: 320
[ INFO ] mandatoryHeight: 240
[ INFO ] isMuted: false
[ INFO ] testedFrames: 283
[ INFO ] blackFrames: 6
[ INFO ] frozenFrames: 5
[ INFO ] Not available: cameraName, encodeSetupTimeMs, avgEncodeTimeMs, minEncodeTimeMs, maxEncodeTimeMs, avgInputFps, minInputFps, maxInputFps, avgSentFps, minSentFps, maxSentFps
[ INFO ] Cannot verify sent FPS.
[ OK ] Captured video using expected resolution.
Check resolution 640x480
[ INFO ] actualVideoWidth: 640
[ INFO ] actualVideoHeight: 480
[ INFO ] mandatoryWidth: 640
[ INFO ] mandatoryHeight: 480
[ INFO ] isMuted: false
[ INFO ] testedFrames: 173
[ INFO ] blackFrames: 11
[ INFO ] frozenFrames: 10
[ INFO ] Not available: cameraName, encodeSetupTimeMs, avgEncodeTimeMs, minEncodeTimeMs, maxEncodeTimeMs, avgInputFps, minInputFps, maxInputFps, avgSentFps, minSentFps, maxSentFps
[ INFO ] Cannot verify sent FPS.
[ OK ] Captured video using expected resolution.
Check resolution 1280x720
[ INFO ] actualVideoWidth: 1280
[ INFO ] actualVideoHeight: 720
[ INFO ] mandatoryWidth: 1280
[ INFO ] mandatoryHeight: 720
[ INFO ] isMuted: false
[ INFO ] testedFrames: 74
[ INFO ] blackFrames: 5
[ INFO ] frozenFrames: 4
[ INFO ] Not available: cameraName, encodeSetupTimeMs, avgEncodeTimeMs, minEncodeTimeMs, maxEncodeTimeMs, avgInputFps, minInputFps, maxInputFps, avgSentFps, minSentFps, maxSentFps
[ INFO ] Cannot verify sent FPS.
[ OK ] Captured video using expected resolution.
Check supported resolutions
[ OK ] Supported: 160x120
[ OK ] Supported: 160x120
[ INFO ] 320x180 not supported
[ OK ] Supported: 320x240
[ INFO ] 640x360 not supported
[ OK ] Supported: 640x480
[ INFO ] 768x576 not supported
[ OK ] Supported: 1024x576
[ OK ] Supported: 1280x720
[ INFO ] 1280x768 not supported
[ INFO ] 1280x800 not supported
[ OK ] Supported: 1920x1080
[ INFO ] 1920x1200 not supported
[ INFO ] 3840x2160 not supported
[ INFO ] 4096x2160 not supported


------------------------------------------------------------------------------------------------------------
For 'Apple Sample' webcam on OS X 10.9.5 and Firefox 37.0.2 (it seems to only really support 640x480 resolution):

Camera
Check resolution 320x240
[ INFO ] actualVideoWidth: 640
[ INFO ] actualVideoHeight: 480
[ INFO ] mandatoryWidth: 320
[ INFO ] mandatoryHeight: 240
[ INFO ] isMuted: false
[ INFO ] testedFrames: 274
[ INFO ] blackFrames: 43
[ INFO ] frozenFrames: 90
[ INFO ] Not available: cameraName, encodeSetupTimeMs, avgEncodeTimeMs, minEncodeTimeMs, maxEncodeTimeMs, avgInputFps, minInputFps, maxInputFps, avgSentFps, minSentFps, maxSentFps
[ INFO ] Cannot verify sent FPS.
[ FAILED ] Incorrect captured resolution.
Check resolution 640x480
[ INFO ] actualVideoWidth: 640
[ INFO ] actualVideoHeight: 480
[ INFO ] mandatoryWidth: 640
[ INFO ] mandatoryHeight: 480
[ INFO ] isMuted: false
[ INFO ] testedFrames: 163
[ INFO ] blackFrames: 36
[ INFO ] frozenFrames: 36
[ INFO ] Not available: cameraName, encodeSetupTimeMs, avgEncodeTimeMs, minEncodeTimeMs, maxEncodeTimeMs, avgInputFps, minInputFps, maxInputFps, avgSentFps, minSentFps, maxSentFps
[ INFO ] Cannot verify sent FPS.
[ OK ] Captured video using expected resolution.
Check resolution 1280x720
[ INFO ] actualVideoWidth: 640
[ INFO ] actualVideoHeight: 480
[ INFO ] mandatoryWidth: 1280
[ INFO ] mandatoryHeight: 720
[ INFO ] isMuted: false
[ INFO ] testedFrames: 72
[ INFO ] blackFrames: 18
[ INFO ] frozenFrames: 17
[ INFO ] Not available: cameraName, encodeSetupTimeMs, avgEncodeTimeMs, minEncodeTimeMs, maxEncodeTimeMs, avgInputFps, minInputFps, maxInputFps, avgSentFps, minSentFps, maxSentFps
[ INFO ] Cannot verify sent FPS.
[ FAILED ] Incorrect captured resolution.
Check supported resolutions
[ OK ] Supported: 160x120
[ OK ] Supported: 160x120
[ OK ] Supported: 320x180
[ OK ] Supported: 320x240
[ OK ] Supported: 640x360
[ OK ] Supported: 640x480
[ OK ] Supported: 768x576
[ OK ] Supported: 1024x576
[ OK ] Supported: 1280x720
[ OK ] Supported: 1280x768
[ OK ] Supported: 1280x800
[ OK ] Supported: 1920x1080
[ OK ] Supported: 1920x1200
[ OK ] Supported: 3840x2160
[ OK ] Supported: 4096x2160

Comment 1

[Dmitriy Kravchenko]

Attachment: Facebook_FF_before_49.png

Comment 2

[Dmitriy Kravchenko]

Note that both mentioned webcams worked perfectly under same conditions with FF versions before 49. The support seems to be broken from FF 49.

I tried to debug from the side of CoreMedia I/O plugin (/Library/CoreMediaIO/Plug-Ins/DAL/ManyCamVideoDeviceCMIO.plugin or Sample.plugin) - and nothing wrong can be seen from inside of plugin - all frames are passed to the Firefox application correctly. It just doesn't accept them for some reason. The workflow doesn't seem to differ from any other application where the webcam works - Chrome, QuickTime or previous version of Firefox.

There was a similar issue under Chrome in the past: https://bugs.chromium.org/p/webrtc/issues/detail?id=6077 but they fixed it in new versions. Only test.webrtc.org reports were slightly different.

Comment 3

[Loic]

Dmitriy, can you download the tool mozregression to narrown down a regression range in FF49.
See http://mozilla.github.io/mozregression/ for details (you need to install python 2.7 to run mozreg)
Run the command mozregression --good=48 then copy here the final pushlog from the repository mozilla-inbound.

Comment 4

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

I've seen noted in another bug that ManyCam uses APIs that don't work with the capture framework we recently switched to (AVFoundation).  See bug 1180725 comment 26 and 27

Chrome fixed this in Chrome 53 which should be the stable version soon if it isn't already.  However, it's unclear what they did to fix it.

Comment 5

[Nils Ohlmeier [:drno]]

No regression window needed as it is clear that this is caused by bug 1180725.

Comment 6

[Nils Ohlmeier [:drno]]

webrtc.org 55 contains a new Mac video capturer which uses AVFoundation instead of the old QTKit based one. Dmitry can you confirm that ManyCam is working with Chrome >= 55, e.g. Chrome Canary which is at 56 now?

If it works the simplest way forward might be to compare our AVFoundation capturer with the one found in the webrtc.org code and see if we can figure out what it does differently to accept the frames.

Comment 7

[Dmitriy Kravchenko]

Attachment: Chrome_Canary56.png

Comment 8

[Dmitriy Kravchenko]

(In reply to Nils Ohlmeier [:drno] from comment #6)
> webrtc.org 55 contains a new Mac video capturer which uses AVFoundation
> instead of the old QTKit based one. Dmitry can you confirm that ManyCam is
> working with Chrome >= 55, e.g. Chrome Canary which is at 56 now?
> 
> If it works the simplest way forward might be to compare our AVFoundation
> capturer with the one found in the webrtc.org code and see if we can figure
> out what it does differently to accept the frames.

Confirming that ManyCam webcam works with Chrome >= 55, checked on Chrome Canary 56.0.2886.0 (64-bit) with Facebook call (see attached Chrome_Canary56.png).

Comment 9

[Dmitriy Kravchenko]

In case you need a source code of a CoreMedia I/O plugin - ManyCam plugin is not very much different from Apple Sample plugin which it is based on. And since the problem is reproducible on Apple Sample too, you may take its sources as a base - http://developer.apple.com/library/mac/samplecode/CoreMediaIO/Introduction/Intro.html. It has some minor compilation issues on latest OS X and Xcode, but is still usable.

Comment 10

[Dmitriy Kravchenko]

Attachment: Chrome_Canary56-on-mac.png

Comment 11

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

I found a weird ManyCam behavior.

2016-10-12 09:44:19.150994 UTC - [Unnamed thread 0x120437b40]: D/MediaManager ChooseCapability: prefs: 640x480 @30-10fps
2016-10-12 09:44:19.151003 UTC - [Unnamed thread 0x120437b40]: D/MediaManager Constraints: width: { min: -2147483647, max: 2147483647 }
2016-10-12 09:44:19.151008 UTC - [Unnamed thread 0x120437b40]: D/MediaManager              height: { min: -2147483647, max: 2147483647 }
2016-10-12 09:44:19.151026 UTC - [Unnamed thread 0x120437b40]: D/MediaManager              frameRate: { min: -inf, max: inf, ideal: 30.000000 }
2016-10-12 09:44:19.151486 UTC - [Unnamed thread 0x120437b40]: D/MediaManager Capability:  176 x  144 x 30 maxFps, UYVY, Unknown codec. Distance = 0
2016-10-12 09:44:19.151685 UTC - [Unnamed thread 0x120437b40]: D/MediaManager Capability:  320 x  180 x 30 maxFps, UYVY, Unknown codec. Distance = 0
2016-10-12 09:44:19.151866 UTC - [Unnamed thread 0x120437b40]: D/MediaManager Capability:  352 x  288 x 30 maxFps, UYVY, Unknown codec. Distance = 0
2016-10-12 09:44:19.152087 UTC - [Unnamed thread 0x120437b40]: D/MediaManager Capability:  512 x  384 x 30 maxFps, UYVY, Unknown codec. Distance = 0
2016-10-12 09:44:19.152267 UTC - [Unnamed thread 0x120437b40]: D/MediaManager Capability:  640 x  360 x 30 maxFps, UYVY, Unknown codec. Distance = 0
2016-10-12 09:44:19.152451 UTC - [Unnamed thread 0x120437b40]: D/MediaManager Capability:  640 x  480 x 30 maxFps, UYVY, Unknown codec. Distance = 0
2016-10-12 09:44:19.152657 UTC - [Unnamed thread 0x120437b40]: D/MediaManager Capability:  800 x  600 x 30 maxFps, UYVY, Unknown codec. Distance = 0
2016-10-12 09:44:19.152854 UTC - [Unnamed thread 0x120437b40]: D/MediaManager Capability: 1024 x  768 x 30 maxFps, UYVY, Unknown codec. Distance = 0
2016-10-12 09:44:19.153020 UTC - [Unnamed thread 0x120437b40]: D/MediaManager Capability: 1280 x  720 x 30 maxFps, UYVY, Unknown codec. Distance = 0
2016-10-12 09:44:19.153203 UTC - [Unnamed thread 0x120437b40]: D/MediaManager Capability: 1280 x  960 x 30 maxFps, UYVY, Unknown codec. Distance = 0
2016-10-12 09:44:19.153388 UTC - [Unnamed thread 0x120437b40]: D/MediaManager Capability: 1600 x 1200 x 30 maxFps, UYVY, Unknown codec. Distance = 0
2016-10-12 09:44:19.153577 UTC - [Unnamed thread 0x120437b40]: D/MediaManager Capability: 1920 x 1080 x 30 maxFps, UYVY, Unknown codec. Distance = 0
2016-10-12 09:44:19.153760 UTC - [Unnamed thread 0x120437b40]: D/MediaManager Capability: 1920 x 1440 x 30 maxFps, UYVY, Unknown codec. Distance = 0
2016-10-12 09:44:19.156178 UTC - [Unnamed thread 0x120437b40]: D/MediaManager Chosen capability:  640 x  480 x 30 maxFps, UYVY, Unknown codec. Distance = 0
2016-10-12 09:44:19.167196 UTC - [Unnamed thread 0x120437b40]: D/MediaManager Video device 4097 allocated for https://webrtc.github.io

When we query the capability, every capability supports the format UYUV.
However, what we actually got is MJPEG. since we use different logic to get the buffer for MJPEG and other formats, we failed to retrieve the sample buffer.

https://dxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/webrtc/modules/video_capture/mac/avfoundation/video_capture_avfoundation_objc.mm#247

Chromium always query the format of sampleBuffer. We probably need to do the same thing to fix this bug.

https://github.com/crosswalk-project/chromium-crosswalk/blob/master/media/capture/video/mac/video_capture_device_avfoundation_mac.mm#L372

Comment 12

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

> When we query the capability, every capability supports the format UYUV.
correct the typo. should be UYVY

Comment 13

[Nils Ohlmeier [:drno]]

So if I understand this right ManyCam claims to deliver everything in UYVY, but then ends up delivering the frames as MJPEG?
That sounds like a bug in ManyCam to me which they should fix.

Awesome if we can make Firefox code more resilient for such broken behavior, but I don't think that should high priority.

Comment 14

[Dmitriy Kravchenko]

(In reply to Nils Ohlmeier [:drno] from comment #13)
> So if I understand this right ManyCam claims to deliver everything in UYVY,
> but then ends up delivering the frames as MJPEG?
> That sounds like a bug in ManyCam to me which they should fix.
> 

Unfortunately, things are not that simple. I tried to log media type/subtype of frames which ManyCam delivers to the application:


Buffer buffer(Buffer::Create(NULL, blockBuffer, mFormatPairs[message->mFrameType].first, 1, 1, &mTimingInfo, 1, &frameSize, mBufferSequenceNumber, GetDiscontinuityFlags()));

...

CMSampleBufferRef bufCM = buffer.Get();
CMFormatDescriptionRef format = CMSampleBufferGetFormatDescription(bufCM);
            
CMMediaType type = CMFormatDescriptionGetMediaType(format);
FourCharCode subtype = CMFormatDescriptionGetMediaSubType(format);
            
LogMessage(kLogLevel_Debug, this, " putting to queue: ('%c%c%c%c', '%c%c%c%c')", ts4CC2Chars(type), ts4CC2Chars(subtype));
            
// Put the buffer at the back of the queue
mBufferQueue.Enqueue(buffer);

That last line is last time ManyCam has access to this frame buffer, after that it will be dispatched to application via CMSimpleQueueRef (wrapped in mBufferQueue).

I got this output in Console when trying to stream in FF49:

16:47:34.770436 +0700	firefox	476:870:454 [  388]     |---CMIO::DP::Sample::Stream::FrameArrived CMIO::DP::Sample::Stream::FrameArrived: Frametype = 11 discontinuity = 0 frameSize = 4147200
16:47:34.770543 +0700	firefox	476:870:454 [  388]     |---CMIO::DP::Sample::Stream::FrameArrived  putting to queue: ('vide', '2vuy')
16:47:34.778011 +0700	firefox	476:870:454 [  388]     |---CMIO::DP::Sample::Stream::FrameArrived CMIO::DP::Sample::Stream::FrameArrived: Frametype = 11 discontinuity = 0 frameSize = 4147200
16:47:34.778107 +0700	firefox	476:870:454 [  388]     |---CMIO::DP::Sample::Stream::FrameArrived  putting to queue: ('vide', '2vuy')
16:47:34.787765 +0700	firefox	476:870:454 [  388]     |---CMIO::DP::Sample::Stream::FrameArrived CMIO::DP::Sample::Stream::FrameArrived: Frametype = 11 discontinuity = 0 frameSize = 4147200
16:47:34.788503 +0700	firefox	476:870:454 [  388]     |---CMIO::DP::Sample::Stream::FrameArrived  putting to queue: ('vide', '2vuy')
16:47:34.810505 +0700	firefox	476:870:454 [  388]     |---CMIO::DP::Sample::Stream::FrameArrived CMIO::DP::Sample::Stream::FrameArrived: Frametype = 11 discontinuity = 0 frameSize = 4147200
16:47:34.810638 +0700	firefox	476:870:454 [  388]     |---CMIO::DP::Sample::Stream::FrameArrived  putting to queue: ('vide', '2vuy')
16:47:34.820190 +0700	firefox	476:870:454 [  388]     |---CMIO::DP::Sample::Stream::FrameArrived CMIO::DP::Sample::Stream::FrameArrived: Frametype = 11 discontinuity = 0 frameSize = 4147200
16:47:34.820263 +0700	firefox	476:870:454 [  388]     |---CMIO::DP::Sample::Stream::FrameArrived  putting to queue: ('vide', '2vuy')
16:47:34.829157 +0700	firefox	476:870:454 [  388]     |---CMIO::DP::Sample::Stream::FrameArrived CMIO::DP::Sample::Stream::FrameArrived: Frametype = 11 discontinuity = 0 frameSize = 4147200
16:47:34.829779 +0700	firefox	476:870:454 [  388]     |---CMIO::DP::Sample::Stream::FrameArrived  putting to queue: ('vide', '2vuy')
16:47:34.859778 +0700	firefox	476:870:454 [  388]     |---CMIO::DP::Sample::Stream::FrameArrived CMIO::DP::Sample::Stream::FrameArrived: Frametype = 11 discontinuity = 0 frameSize = 4147200
16:47:34.859899 +0700	firefox	476:870:454 [  388]     |---CMIO::DP::Sample::Stream::FrameArrived  putting to queue: ('vide', '2vuy')
16:47:34.867956 +0700	firefox	476:870:454 [  388]     |---CMIO::DP::Sample::Stream::FrameArrived CMIO::DP::Sample::Stream::FrameArrived: Frametype = 11 discontinuity = 0 frameSize = 4147200

Thus, ManyCam delivers frames in UYVY ('2vuy'), not MJPEG. Maybe they are somehow converted to MJPEG by AVFoundation?

Comment 15

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Attachment: Bug 1308412 - retrieve CVImageBuffer from CMSampleBuffer if it doesn't contain any CMBlockBuffer;

Review commit: https://reviewboard.mozilla.org/r/88518/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/88518/

Comment 16

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Comment on attachment 8804605 [details]
Bug 1308412 - retrieve CVImageBuffer from CMSampleBuffer if it doesn't contain any CMBlockBuffer;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/88518/diff/1-2/

Comment 17

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Attachment: Bug 1308412 - fix setCaptureHeight() failure;

Review commit: https://reviewboard.mozilla.org/r/88520/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/88520/

Comment 18

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Comment on attachment 8804605 [details]
Bug 1308412 - retrieve CVImageBuffer from CMSampleBuffer if it doesn't contain any CMBlockBuffer;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/88518/diff/2-3/

Comment 19

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

Comment on attachment 8804605 [details]
Bug 1308412 - retrieve CVImageBuffer from CMSampleBuffer if it doesn't contain any CMBlockBuffer;

https://reviewboard.mozilla.org/r/88518/#review87690

Reasonable, though I don't know Mac avfoundation stuff

Comment 20

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

Comment on attachment 8804610 [details]
Bug 1308412 - fix setCaptureHeight() failure;

https://reviewboard.mozilla.org/r/88520/#review87692

r+ with just some comment nits

::: media/webrtc/trunk/webrtc/modules/video_capture/mac/avfoundation/video_capture_avfoundation_info_objc.mm:166
(Diff revision 1)
>      }
>  
>      *width = videoDimensions.width;
>      *height = videoDimensions.height;
> -    *maxFPS = maxFrameRateRange.maxFrameRate;
> +
> +    // This is to fix setCaptureHeight() failed for some webcams supporting non-integer framerates.

"failed" -> "which fails"

::: media/webrtc/trunk/webrtc/modules/video_capture/mac/avfoundation/video_capture_avfoundation_info_objc.mm:167
(Diff revision 1)
>  
>      *width = videoDimensions.width;
>      *height = videoDimensions.height;
> -    *maxFPS = maxFrameRateRange.maxFrameRate;
> +
> +    // This is to fix setCaptureHeight() failed for some webcams supporting non-integer framerates.
> +    // In setCaptureHeight(), we match a a best framerate range by searching a range whose max framerate

"a a" -> "a"

Comment 21

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8804610 [details]
Bug 1308412 - fix setCaptureHeight() failure;

https://reviewboard.mozilla.org/r/88520/#review87730

Comment 22

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8804605 [details]
Bug 1308412 - retrieve CVImageBuffer from CMSampleBuffer if it doesn't contain any CMBlockBuffer;

https://reviewboard.mozilla.org/r/88518/#review87732

Comment 23

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Comment on attachment 8804610 [details]
Bug 1308412 - fix setCaptureHeight() failure;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/88520/diff/1-2/

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0680e3176fa5
retrieve CVImageBuffer from CMSampleBuffer if it doesn't contain any CMBlockBuffer; r=jesup,jib
https://hg.mozilla.org/integration/autoland/rev/0b1ae9f4f07b
fix setCaptureHeight() failure; r=jesup,jib

Comment 25

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/0680e3176fa5
https://hg.mozilla.org/mozilla-central/rev/0b1ae9f4f07b

Comment 26

[Ryan VanderMeulen [:RyanVM]]

Should we be considering this for uplift to 51?

Comment 27

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Comment on attachment 8804605 [details]
Bug 1308412 - retrieve CVImageBuffer from CMSampleBuffer if it doesn't contain any CMBlockBuffer;

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1180725
[User impact if declined]: Entirely black video screen while using ManyCam as a webrtc mediasource
[Is this code covered by automated tests?]: no, we use fake camera stream in auto test 
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: install ManyCam, setup a webrtc call and choose ManyCam as the source.
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: low risk. It only impact the scenario in which ManyCam is installed and used.
[Why is the change risky/not risky?]: Same as above
[String changes made/needed]: no

Comment 28

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Comment on attachment 8804610 [details]
Bug 1308412 - fix setCaptureHeight() failure;

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1180725
[User impact if declined]: Entirely black video screen while using ManyCam as a webrtc mediasource
[Is this code covered by automated tests?]: no, we use fake camera stream in auto test 
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: install ManyCam, setup a webrtc call and choose ManyCam as the source.
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: low risk. It only impact the scenario in which ManyCam is installed and used.
[Why is the change risky/not risky?]: Same as above
[String changes made/needed]: no

Comment 29

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8804605 [details]
Bug 1308412 - retrieve CVImageBuffer from CMSampleBuffer if it doesn't contain any CMBlockBuffer;

WebRTC fix for a particular camera, regression from 49, let's bring it to beta 51.

Comment 30

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/62cd1c0a5770
https://hg.mozilla.org/releases/mozilla-beta/rev/ed5b9c0678b0

Comment 31

[Camelia Badau [:cbadau], Desktop QA]

Verified fixed on Mac OS X 10.11.6 using Firefox 51 RC (buildID: 20170116133120) and latest Aurora 52.0a2 (buildID: 20170118004007): 
- the video from webcam is correctly shown during call in Facebook
- "Camera" section in test.webrtc.org report shows this:

Check resolution 640x480
[ INFO ] cameraName: ManyCam Virtual Webcam
[ INFO ] actualVideoWidth: 640
[ INFO ] actualVideoHeight: 480
[ INFO ] mandatoryWidth: 640
[ INFO ] mandatoryHeight: 480
[ INFO ] isMuted: false
[ INFO ] testedFrames: 194
[ INFO ] blackFrames: 1
[ INFO ] frozenFrames: 0
[ INFO ] Not available: encodeSetupTimeMs, avgEncodeTimeMs, minEncodeTimeMs, maxEncodeTimeMs, avgInputFps, minInputFps, maxInputFps, avgSentFps, minSentFps, maxSentFps
[ INFO ] Cannot verify sent FPS.
[ OK ] Captured video using expected resolution.

Check resolution 1280x720
[ INFO ] cameraName: ManyCam Virtual Webcam
[ INFO ] actualVideoWidth: 1280
[ INFO ] actualVideoHeight: 720
[ INFO ] mandatoryWidth: 1280
[ INFO ] mandatoryHeight: 720
[ INFO ] isMuted: false
[ INFO ] testedFrames: 114
[ INFO ] blackFrames: 1
[ INFO ] frozenFrames: 0
[ INFO ] Not available: encodeSetupTimeMs, avgEncodeTimeMs, minEncodeTimeMs, maxEncodeTimeMs, avgInputFps, minInputFps, maxInputFps, avgSentFps, minSentFps, maxSentFps
[ INFO ] Cannot verify sent FPS.
[ OK ] Captured video using expected resolution.