Open
Conversation
Fixes for multiple Windows-only issues in camera capture: - Prevents a hard crash (access violation) during camera enumeration/open. - Fixes capability enumeration/selection so `GetUserMedia` can find a usable camera mode. - Prevents a panic during device shutdown caused by late DirectShow callbacks.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #686 +/- ##
=======================================
Coverage 42.13% 42.13%
=======================================
Files 86 86
Lines 5186 5186
=======================================
Hits 2185 2185
Misses 2839 2839
Partials 162 162 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This patch set fixes multiple Windows-only issues in
github.com/pion/mediadevicescamera capture:GetUserMediacan find a usable camera mode.Root cause (crash in
listResolution/openCamera)selectCamera()uses an inverted/ambiguous return convention:1when a camera is found (success).0when the camera is not found.fail:path (e.g. COM/DirectShow enumeration failure), it also returned1.Callers interpret
selectCamera()as a boolean success predicate:listResolution()doesif (!selectCamera(...)) goto fail;.selectCamera()hit thefail:label, it returned1, solistResolution()proceeded as if successful, even thoughmonikerwas stillnullptr.listResolution()then executedmoniker->BindToObject(...), causing an access violation (0xc0000005).This is particularly easy to trigger when the COM environment is not initialized on the current OS thread (COM init is per-thread on Windows), but the crash itself is due to the incorrect return value and missing defensive checks.
Changes
selectCamera()failure behavior: return0on thefail:path so callers correctly take the failure path.listResolution()andopenCamera():moniker != nullptrbefore use.BindToObject(...)return value; on failure, set an error string and bail out.freeCameraList(): return0for success (the function’s comment says it frees resources; returning1incorrectly indicates failure).Root cause (
GetUserMediafails with “failed to find the best driver that fits the constraints”)mediadevicesselects devices based on the driver’s reportedProperties(). On Windows, the camera driver could report zero usable properties, causing selection to fail even when a camera exists.Contributing issues:
FORMAT_VideoInfo2(not justFORMAT_VideoInfo), so the current capability parsing misses them.IAMStreamConfigeven though DirectShow can still deliver YUY2 to the SampleGrabber via decoders/converters.Changes (capability enumeration / reporting)
FORMAT_VideoInfo2caps inlistResolution()and normalize negative dimensions.VIDEOINFOHEADER2is available across Windows SDK toolchains (amvideo.h+dvdmedia.h).Properties()filtering to advertise width/height modes regardless of the native FOURCC, and report YUY2 as the output format since the SampleGrabber requests YUY2 (DirectShow may insert converters/decoders).Root cause (panic during shutdown)
During shutdown, DirectShow may deliver a final frame concurrently with
Close(). The exported Go callback (imageCallback) attempted to send intoc.chafterClose()had already closed it, causing:panic: send on closed channelChanges (shutdown race)
closedflag to the Windows camera driver.imageCallbackreturn early if closed and use a non-blocking send.recover()to handle the narrow race where close happens between check and send.