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

mac-virtualcam: Fix IOSurface memory leak #6639

Merged
merged 1 commit into from
Jun 25, 2022

Conversation

fabianishere
Copy link
Contributor

@fabianishere fabianishere commented Jun 22, 2022

This change fixes a memory leak in the mac-virtualcam plugin that causes OBS to not release the CVPixelBuffers (and underlying IOSurfaces) it emits to the virtual camera consumers.

Description

I have found that the plugin is leaking Mach ports associated with IOSurfaces, preventing them from being re-used. The previous approach using NSMachPort does not seem to properly release the Mach port allocated via CVPixelBufferGetIOSurface and IOSurfaceLookupFromMachPort. Instead, we must explicitly deallocate the port using mach_port_deallocate.

Motivation and Context

Pull request #6573 (Avoid transcoding where possible) updated the mac-virtualcam plugin to share the virtual camera feed with other processes via IOSurfaces.

Although the changes work correctly, users have observed that OBS memory usage keeps increasing when the virtual camera is active until OBS runs out of memory or the consuming application is closed.
See the report by @SciTechNick for more information: #6573 (comment)

How Has This Been Tested?

I have tested the changes on a Macbook Pro (M1) running macOS Monterey with Google Chrome, Zoom, and Cameo. OBS shows no signs of memory leakage after multiple minutes.

Types of changes

Bug fix

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.

@PatTheMav PatTheMav requested a review from gxalpha June 22, 2022 09:01
@WizardCM WizardCM added Bug Fix Non-breaking change which fixes an issue macOS Affects macOS labels Jun 22, 2022
@WizardCM WizardCM added this to the OBS Studio 28.0 milestone Jun 22, 2022
Copy link
Member

@gxalpha gxalpha left a comment

Choose a reason for hiding this comment

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

Can confirm that this fixes the issue.

plugins/mac-virtualcam/src/dal-plugin/OBSDALMachClient.mm Outdated Show resolved Hide resolved
plugins/mac-virtualcam/src/dal-plugin/OBSDALMachClient.mm Outdated Show resolved Hide resolved
This change fixes a memory leak in the mac-virtualcam plugin that causes
OBS to not release the CVPixelBuffers (and underlying IOSurfaces)
it emits to the virtual camera consumers.

Pull request obsproject#6573 (Avoid
transcoding where possible) updated the mac-virtualcam to share the
virtual camera feed with other processes via IOSurfaces.

Although the changes work correctly, users have observed that OBS memory
usage keeps increasing when the virtual camera is active until OBS runs
out of memory or the consuming application is closed.
See the report by @SciTechNick for more information:
obsproject#6573 (comment)

After some debugging, I have found that the plugin is leaking Mach ports
associated with IOSurfaces, preventing them from being re-used. The
previous approach using `NSMachPort` does not seem to properly release
the Mach port allocated via `CVPixelBufferGetIOSurface` and
`IOSurfaceLookupFromMachPort`. Instead, we must explicitly deallocate
the port using `mach_port_deallocate`.

I have tested the changes on a Macbook Pro (M1) running macOS Monterey with
Google Chrome, Zoom, and Cameo. OBS shows no signs of memory leakage
after multiple minutes.
@fabianishere fabianishere force-pushed the bug/mac-vcam-mem-leak branch from 1083df5 to 8fda51d Compare June 23, 2022 10:04
@jp9000 jp9000 merged commit b02e4b1 into obsproject:master Jun 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Fix Non-breaking change which fixes an issue macOS Affects macOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants