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(macos): prevent indefinite hanging if screen capture is not granted #3360

Merged

Conversation

cathyjf
Copy link
Contributor

@cathyjf cathyjf commented Nov 2, 2024

Description

Currently, if Sunshine has not yet been granted the screen capture
permission, the program simply hangs indefinitely on startup when it
attempts to probe for usable encoders. This is not desirable because
it prevents testing and using all of the parts of Sunshine that do not
require the screen capture permission.

With this patch, the encoder probing will simply fail instead of
hanging indefinitely if Sunshine does not yet have the screen capture
permission.

Note that Sunshine already prints out an error message telling the user
that the screen capture permission is needed. The bug is that Sunshine
currently indefinitely hangs shortly after printing that message.

Screenshot

N/A

Issues Fixed or Closed

N/A

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Dependency update (updates to dependencies)
  • Documentation update (changes to documentation)
  • Repository update (changes to repository files, e.g. .github/...)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated the in code docstring/documentation-blocks for new or existing methods/components

@cathyjf cathyjf changed the title fix(macos): Prevent indefinite hanging if screen capture is not granted fix(macos): prevent indefinite hanging if screen capture is not granted Nov 2, 2024
@cathyjf cathyjf force-pushed the macos-screen-capture-prevent-hang branch from d0148d0 to cca03f1 Compare November 2, 2024 12:50
Copy link
Member

@ReenigneArcher ReenigneArcher left a comment

Choose a reason for hiding this comment

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

Thank you for the PR. The suggestions are mostly to address linting failures.

src/platform/macos/misc.mm Outdated Show resolved Hide resolved
src/platform/macos/misc.h Outdated Show resolved Hide resolved
Copy link

codecov bot commented Nov 2, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 11.06%. Comparing base (c54664b) to head (2b4610e).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/platform/macos/display.mm 33.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3360      +/-   ##
==========================================
+ Coverage   11.05%   11.06%   +0.01%     
==========================================
  Files          99       99              
  Lines       17231    17237       +6     
  Branches     8034     8035       +1     
==========================================
+ Hits         1905     1908       +3     
- Misses      12641    12784     +143     
+ Partials     2685     2545     -140     
Flag Coverage Δ
Linux 8.35% <ø> (ø)
Windows 5.23% <ø> (ø)
macOS-13 13.62% <66.66%> (+0.02%) ⬆️
macOS-14 12.62% <60.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/platform/macos/misc.mm 8.77% <100.00%> (+1.02%) ⬆️
src/platform/macos/display.mm 40.35% <33.33%> (-0.73%) ⬇️

... and 22 files with indirect coverage changes

@cathyjf cathyjf force-pushed the macos-screen-capture-prevent-hang branch from ae90cf1 to 4e22479 Compare November 2, 2024 15:46
Currently, if Sunshine has not yet been granted the screen capture
permission, the program simply hangs indefinitely on startup when it
attempts to probe for usable encoders. This is not desirable because
it prevents testing and using all of the parts of Sunshine that do not
require the screen capture permission.

With this patch, the encoder probing will simply fail instead of
hanging indefinitely if Sunshine does not yet have the screen capture
permission.

Note that Sunshine already prints out an error message telling the user
that the screen capture permission is needed. The bug is that Sunshine
currently indefinitely hangs shortly after printing that message.
@ReenigneArcher ReenigneArcher force-pushed the macos-screen-capture-prevent-hang branch from 4e22479 to 2b4610e Compare November 2, 2024 16:09
@ReenigneArcher ReenigneArcher enabled auto-merge (squash) November 2, 2024 16:09
Copy link

sonarqubecloud bot commented Nov 2, 2024

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.

2 participants