Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SA-14771 fix dshow get device list when only audio devices available #9

Open
wants to merge 2 commits into
base: sbg
Choose a base branch
from

Conversation

derekcyruschow-catapult
Copy link

@derekcyruschow-catapult derekcyruschow-catapult commented Mar 3, 2023

https://sbgsportssoftware.atlassian.net/browse/SA-14771

  • Revert "add dshow_get_device_list"
  • avdevice/dshow: implement get_device_list (cherry-pick of upstream change ec579b4)

derekcyruschow-catapult and others added 2 commits March 3, 2023 11:14
Needed to enable programmatic discovery of DirectShow devices

Signed-off-by: Diederick Niehorster <[email protected]>
Reviewed-by: Roger Pack <[email protected]>
(cherry picked from commit ec579b4)

Conflicts:
	libavdevice/dshow.c
Copy link

@lm3m lm3m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just some comments on the code, not so much your changes
the changes look good.
:shipit:

int i;

r = CoGetMalloc(1, &co_malloc);
if (r != S_OK)
goto fail1;
goto fail;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you want to, you could convert the goto fail in a variable and a break

}

fail1:
fail:
if (device) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

smart pointers?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C-code 😬

if (!device_list)
return AVERROR(EINVAL);

CoInitialize(0);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expensive if we do this too often (maybe have one thread where we do com/dshow stuff?)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeh we can probably limit the amount of calls to this code when we create FFmpegDevice instances on our end - instead of calling QueryForDevices() we could refactor it out into a separate thing like we have on this branch https://github.com/SBGSports/SBGSource/blob/2b0efd1cf3c52149f279f017c23300418486e94a/RaceMedia/ffmpeg_device_list.cpp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants