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

fix: Force syncpoint on everysample #52

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sandercox
Copy link
Contributor

@sandercox sandercox commented Mar 7, 2023

Description

This changes every sample to clearly state that they are sync points.

Motivation and Context

As the virtual camera always sends frames that are uncompressed and/or keyframes the best is to always mark these samples as such. When not doing this I was having issues in Resolume Arena that sometimes the OBS stream would show up and on other times it would not and the virtualcam filter would not transition into running state properly.

IMediaControl->Run() would only call into the Pause call on the filter but not the Run call, not always sometimes it would work, so I guess this memory was 'uninitialized' when not set explicitly.

How Has This Been Tested?

Running aforementioned software with and without this change. Tested on Windows 11, core i9.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Tweak (non-breaking change to improve existing 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.

@WizardCM
Copy link
Member

Is this considered standard practice, or a hack due to some other behaviour (either elsewhere in the library or in the app interacting with the virtual camera)?

@sandercox
Copy link
Contributor Author

From MSDN documentation: https://learn.microsoft.com/en-us/windows/win32/api/strmif/nf-strmif-imediasample-setsyncpoint

The filter that first generates the data in the sample should set this flag to TRUE or FALSE, as appropriate. For uncompressed video and PCM audio, set every sample to TRUE. For compressed video, set key frames to TRUE and delta frames to FALSE.

As only keyframes are recorded with this filter it should be marked as true for every frame.

@sandercox
Copy link
Contributor Author

Though we also clear-out the reference clock on an OBS source explicitly as otherwise we don't get updates on the graphics. So I would not be surprised if there is a bigger issue with the frame timing inside the capture filter.

@sandercox
Copy link
Contributor Author

@WizardCM can we get this in?

even if this is just more explicit than maybe set by default, it still would make sense to add this flag as it is the correct behaviour for these streams.

@sandercox
Copy link
Contributor Author

This still seems to be an issue for our users and our modified Virtual Camera with just this fix seems to solve it. Would be great if we could get this merged.

@Fenrirthviti
Copy link
Member

Users of what? Having a way to test this would be helpful to verify the issue and that this is the correct fix.

@sandercox
Copy link
Contributor Author

I'm sorry that was unclear. I'm one of the authors behind Resolume Arena when we use the OBS virtual camera without this fix we receive black video / no samples.

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