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

feat(display)!: Add libdisplaydevice dependency and output name mapping #2894

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

FrogTheFrog
Copy link
Collaborator

Description

This is one of the first PRs related to #2582.
The code has been moved to the https://github.com/LizardByte/libdisplaydevice repo and will be coming to Sunshine little by little.

In this PR libdisplaydevice has been added as a submodule and a new singleton class with a new map_output_name method has been added.
For Windows the map_output_name now maps a device id to its corresponding display name, while on other platforms it simply performs a pass-trough.
On WIndows, in case the corresponding device has no display name at the moment (either inactive or the id is just wrong), it will return an empty string and the default display would be used as a result (you would still be able to start the stream).

Unfortunately, the method cannot be called just once when the config is loaded to remap the output_name immediately, since on Windows there is no guarantee that the display name will remain the same for the display (even without restarting the PC).

The display name is tied to a topology of active displays, whose settings are cached by the OS. When Windows encounters a completely new topology of active displays, it will generate the "best" settings to be used as a base.
The new base is not guaranteed to have the same display names as the other cached topology. Now, by simply turning off/on any of the displays, the active topology will change and so can the display name.
This is at least one of the factors why the display name seems random and unpredictable.

Screenshot

Some changes have been made in the UI to show how to configure the output name on Windows.
image

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

Branch Updates

LizardByte requires that branches be up-to-date before merging. This means that after any PR is merged, this branch
must be updated before it can be merged. You must also
Allow edits from maintainers.

  • I want maintainers to keep my branch updated

@FrogTheFrog FrogTheFrog marked this pull request as draft July 19, 2024 08:23
@FrogTheFrog FrogTheFrog marked this pull request as ready for review July 19, 2024 08:23
Copy link

codecov bot commented Jul 19, 2024

Codecov Report

Attention: Patch coverage is 22.58065% with 48 lines in your changes missing coverage. Please review.

Project coverage is 11.16%. Comparing base (0cc98f1) to head (7293c10).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/display_device.cpp 13.04% 16 Missing and 4 partials ⚠️
src/logging.cpp 28.57% 14 Missing and 6 partials ⚠️
src/video.cpp 37.50% 3 Missing and 2 partials ⚠️
src/platform/windows/display_base.cpp 0.00% 2 Missing ⚠️
src/platform/macos/input.cpp 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2894      +/-   ##
==========================================
+ Coverage   11.12%   11.16%   +0.03%     
==========================================
  Files          99      100       +1     
  Lines       17253    17310      +57     
  Branches     8045     8069      +24     
==========================================
+ Hits         1920     1933      +13     
- Misses      12779    12812      +33     
- Partials     2554     2565      +11     
Flag Coverage Δ
Linux 8.43% <24.48%> (+0.05%) ⬆️
Windows 5.38% <20.33%> (+0.06%) ⬆️
macOS-13 13.72% <22.91%> (+0.08%) ⬆️
macOS-14 12.72% <22.91%> (+0.08%) ⬆️

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

Files with missing lines Coverage Δ
src/logging.h 32.35% <ø> (ø)
src/platform/macos/input.cpp 40.86% <0.00%> (-0.23%) ⬇️
src/platform/windows/display_base.cpp 13.46% <0.00%> (-0.03%) ⬇️
src/video.cpp 30.00% <37.50%> (+0.04%) ⬆️
src/display_device.cpp 13.04% <13.04%> (ø)
src/logging.cpp 60.71% <28.57%> (-7.43%) ⬇️

@FrogTheFrog FrogTheFrog marked this pull request as draft July 19, 2024 08:43
@FrogTheFrog FrogTheFrog force-pushed the libdd-part-1 branch 2 times, most recently from 9a8d7bb to c622bec Compare July 19, 2024 17:01
@FrogTheFrog FrogTheFrog marked this pull request as ready for review July 19, 2024 17:55
src/display_device.h Outdated Show resolved Hide resolved
src_assets/common/assets/web/public/assets/locale/en.json Outdated Show resolved Hide resolved
docs/source/about/advanced_usage.rst Outdated Show resolved Hide resolved
cmake/compile_definitions/common.cmake Outdated Show resolved Hide resolved
@ReenigneArcher ReenigneArcher changed the title feat: Add libdisplaydevice dependency and output name mapping feat(display)!: Add libdisplaydevice dependency and output name mapping Jul 26, 2024
@ReenigneArcher

This comment was marked as resolved.

@FrogTheFrog FrogTheFrog force-pushed the libdd-part-1 branch 2 times, most recently from 58c729d to 7a80ccb Compare July 28, 2024 13:41
ReenigneArcher
ReenigneArcher previously approved these changes Jul 28, 2024
docs/source/about/advanced_usage.rst Outdated Show resolved Hide resolved
third-party/libdisplaydevice Outdated Show resolved Hide resolved
Copy link
Member

@Hazer Hazer left a comment

Choose a reason for hiding this comment

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

Looks good to me, I'll be basing my display changes on it "soon".

docs/configuration.md Outdated Show resolved Hide resolved
third-party/libdisplaydevice Outdated Show resolved Hide resolved
@FrogTheFrog FrogTheFrog force-pushed the libdd-part-1 branch 2 times, most recently from af1e369 to 4d7ae6e Compare August 9, 2024 06:46
@FrogTheFrog FrogTheFrog force-pushed the libdd-part-1 branch 2 times, most recently from 6cb95de to 9fe0eea Compare August 24, 2024 09:36
@FrogTheFrog FrogTheFrog force-pushed the libdd-part-1 branch 2 times, most recently from 5c17944 to d15863e Compare September 10, 2024 06:55
@FrogTheFrog FrogTheFrog force-pushed the libdd-part-1 branch 3 times, most recently from 086edc9 to 57feed8 Compare October 7, 2024 12:02
@ReenigneArcher ReenigneArcher added this to the adjust lint rules milestone Oct 20, 2024
@FrogTheFrog FrogTheFrog force-pushed the libdd-part-1 branch 2 times, most recently from 993160b to 2fbc40c Compare October 26, 2024 07:12
@FrogTheFrog FrogTheFrog force-pushed the libdd-part-1 branch 2 times, most recently from 6487802 to 9d0fa95 Compare December 3, 2024 07:25
@ReenigneArcher ReenigneArcher requested review from cgutman and removed request for cgutman December 3, 2024 13:29
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
7 New issues
7 New Code Smells (required ≤ 0)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@ReenigneArcher ReenigneArcher merged commit 1543f58 into LizardByte:master Dec 11, 2024
34 of 37 checks passed
TheElixZammuto pushed a commit to TheElixZammuto/Sunshine-1 that referenced this pull request Dec 23, 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.

3 participants