Add feature to detect device's remove/insert events. #30

2021-04-28T01:05:37-05:00

walker-WSH commented

2021-04-28 01:05:37 -05:00

(Migrated from github.com)

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:

My code has been run through clang-format.
I have read the contributing document.
My code is not on the master branch.
The code has been tested.
All commit messages are properly formatted and commits squashed where appropriate.
I have included updates to all appropriate documentation.

### 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:   - [x] My code has been run through [clang-format](https://github.com/obsproject/obs-studio/blob/master/.clang-format). - [x] I have read the [**contributing** document](https://github.com/obsproject/obs-studio/blob/master/CONTRIBUTING.rst). - [x] My code is not on the master branch. - [x] The code has been tested. - [x] All commit messages are properly formatted and commits squashed where appropriate. - [x] I have included updates to all appropriate documentation.

walker-WSH commented

2021-04-28 01:13:10 -05:00

(Migrated from github.com)

After merging this PR, we can improve win-dshow to restore camera automatically:

After merging this PR, we can improve win-dshow to restore camera automatically: ![1619590349](https://user-images.githubusercontent.com/18413354/116355031-daa15b80-a82b-11eb-9059-fec933771828.png)

walker-WSH commented

2021-05-06 21:12:23 -05:00

(Migrated from github.com)

@jp9000 Request your review

dodgepong commented

2021-05-06 23:29:16 -05:00

(Migrated from github.com)

This will not be reviewed until some time after v27 is released.

walker-WSH commented

2021-05-07 03:02:06 -05:00

(Migrated from github.com)

This will not be reviewed until some time after v27 is released.

Got it!

> This will not be reviewed until some time after v27 is released. Got it!

walker-WSH commented

2022-01-13 03:59:13 -06:00

(Migrated from github.com)

Hmm~
will this PR be merged ？

Hmm~ will this PR be merged ？

jp9000 commented

2022-01-18 04:40:29 -06:00

(Migrated from github.com)

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)

👀 1

walker-WSH commented

2024-06-04 03:43:03 -05:00

(Migrated from github.com)

@PatTheMav @RytoEX
can you help review this PR ? thanks~

@PatTheMav @RytoEX can you help review this PR ? thanks~

PatTheMav (Migrated from github.com) requested changes

2026-08-27 10:22:56 -05:00

PatTheMav (Migrated from github.com) left a 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.

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.

source/device.cpp

					
				@ -56,0 +92,4 @@

							break;

					}

				}

Is it necessary to use reinterpret_cast here? Would static_cast suffice?

Is it necessary to use `reinterpret_cast` here? Would `static_cast` suffice?

source/device.cpp

					
				@ -56,0 +106,4 @@

						if (EC_DEVICE_LOST == eventCode) {

							if (FLAG_REMOVE_DEVICE == param2) {

								Warning(L"Device is removed.");

								callback->OnDeviceRemoved();

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.

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.

source/device.cpp

					
				@ -56,0 +111,4 @@

								Info(L"Device is inserted again.");

								callback->OnDeviceInserted();

							}

						}

Per Microsoft's documentation states this should use the logic res - WAIT_OBJECT_0 == <expected array index>.

Per Microsoft's documentation states this should use the logic `res - WAIT_OBJECT_0 == <expected array index>`.

source/device.cpp

					
				@ -56,0 +115,4 @@

						eventEx->FreeEventParams(eventCode, param1, param2);

						if (WAIT_OBJECT_0 == WaitForSingleObject(exitEvt, 0))

		if (!ReadAllEvents()) {
			break;
		}

Needs to use curly braces for all blocks per current code style guidelines. Applies throughout.

```suggestion if (!ReadAllEvents()) { break; } ``` Needs to use curly braces for all blocks per current code style guidelines. Applies throughout.

source/device.cpp

					
				@ -56,0 +120,4 @@

					}

					return true;

				}

Replace with constexpr int.

Replace with `constexpr int`.

source/device.cpp

					
				@ -56,0 +142,4 @@

				{

					::SetEvent(exitEvt);

					if (msgThread && (msgThread != INVALID_HANDLE_VALUE)) {

						WaitForSingleObject(msgThread, INFINITE);

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?

source/device.cpp

Why is it necessary to use the global scope identifier explicitly here and for other functions like ResetEvent above?

Why is it necessary to use the global scope identifier explicitly here and for other functions like `ResetEvent` above?

source/device.hpp

	HANDLE mediaEvent;

```suggestion HANDLE mediaEvent; ```

source/device.hpp

	HANDLE exitEvent;

```suggestion HANDLE exitEvent; ```

source/device.hpp

	HANDLE mediaEventThread;

```suggestion HANDLE mediaEventThread; ```

source/device.hpp

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?

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?

source/dshow-base.cpp

		hr = graph->QueryInterface(IID_IMediaEventEx, static_cast<void **>(&event));

Use static_cast instead of C-style casts in C++ code.

```suggestion hr = graph->QueryInterface(IID_IMediaEventEx, static_cast<void **>(&event)); ``` Use `static_cast` instead of C-style casts in C++ code.

This pull request is broken due to missing fork information.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin 'unknown repository:unknown repository'

git switch 'unknown repository'

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch 'master'

git merge --no-ff 'unknown repository'

git switch 'unknown repository'

git rebase 'master'

git switch 'master'

git merge --ff-only 'unknown repository'

git switch 'unknown repository'

git rebase 'master'

git switch 'master'

git merge --no-ff 'unknown repository'

git switch 'master'

git merge --squash 'unknown repository'