Add feature to detect device's remove/insert events. - #30
Conversation
|
After merging this PR, we can improve win-dshow to restore camera automatically:
|
|
@jp9000 Request your review |
|
This will not be reviewed until some time after v27 is released. |
Got it! |
|
Hmm~ |
|
sorry about that, I'll try to get around to it sooner than later, it's just easy for things to slide (especially when they're on repositories other than the main repository) |
|
@PatTheMav @RytoEX |
PatTheMav
left a comment
PatTheMav
left a comment
There was a problem hiding this comment.
Conceptually this looks correct as far as the Win32 APIs are concerned, so I got mostly code style nits.
Might need someone else to debug this on an actual Windows machine with a DirectShow device.
|
|
||
| HDevice(); | ||
| // handle insert/remove events | ||
| HANDLE msgEvt; |
There was a problem hiding this comment.
| HANDLE msgEvt; | |
| HANDLE mediaEvent; |
| HDevice(); | ||
| // handle insert/remove events | ||
| HANDLE msgEvt; | ||
| HANDLE exitEvt; |
There was a problem hiding this comment.
| HANDLE exitEvt; | |
| HANDLE exitEvent; |
| // handle insert/remove events | ||
| HANDLE msgEvt; | ||
| HANDLE exitEvt; | ||
| HANDLE msgThread; |
There was a problem hiding this comment.
| HANDLE msgThread; | |
| HANDLE mediaEventThread; |
| ~HDevice(); | ||
|
|
||
| // handle insert/remove events | ||
| static unsigned __stdcall EventThread(void *pParam); |
There was a problem hiding this comment.
Why use a static class function rather than a free-standing function in an anonymous namespace for this? Also why use _beginthreadex over the CreateThread API?
| HANDLE events[] = { | ||
| exitEvt, // must be first one | ||
| msgEvt, | ||
| }; | ||
|
|
||
| DWORD count = sizeof(events) / sizeof(HANDLE); |
There was a problem hiding this comment.
The array is static and the count never changes, so an std::array could be used, which has a size() method to get the count for WaitForMultipleObjects and the data() method to get a raw pointer.
| #define FLAG_REMOVE_DEVICE 0 | ||
| #define FLAG_INSERT_DEVICE 1 |
| if (WAIT_OBJECT_0 == WaitForSingleObject(exitEvt, 0)) | ||
| return false; |
There was a problem hiding this comment.
The idea here is to check whether the exit event was signalled after the current media event and thus make the media event thread exit?
|
|
||
| void HDevice::StopEventThread() | ||
| { | ||
| ::SetEvent(exitEvt); |
There was a problem hiding this comment.
Why is it necessary to use the global scope identifier explicitly here and for other functions like ResetEvent above?
| } | ||
|
|
||
| if (pevent) { | ||
| hr = graph->QueryInterface(IID_IMediaEventEx, (void **)&event); |
There was a problem hiding this comment.
| hr = graph->QueryInterface(IID_IMediaEventEx, (void **)&event); | |
| hr = graph->QueryInterface(IID_IMediaEventEx, static_cast<void **>(&event)); |
Use static_cast instead of C-style casts in C++ code.
|
|
||
| unsigned __stdcall HDevice::EventThread(void *pParam) | ||
| { | ||
| HDevice *self = reinterpret_cast<HDevice *>(pParam); |
There was a problem hiding this comment.
Is it necessary to use reinterpret_cast here? Would static_cast suffice?
github-project-automation
Bot
moved this from Ready For Review
to Requires Changes
in 33.0 Release Tracker
7 participants
Description
Add feature to detect device's remove/insert events. When device is inserted, win-dshow can restore camera when received event.
However, to be enable to detect remove/insert, we must success to initialize dshow firstly.
Besides, it seems insert/remove events won't be sent for audio filter.
Motivation and Context
Help win-dshow to restore camera when device is inserted.
How Has This Been Tested?
Tested on Window10 with Legitech C920
Types of changes
New feature (non-breaking change which adds functionality)
Checklist: