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(linux): add frame processing latency and logging improvements #2502

Merged
merged 5 commits into from
May 12, 2024

Conversation

gschintgen
Copy link
Contributor

@gschintgen gschintgen commented May 5, 2024

Description

This PR collects a few minor improvements to the Linux logging code. It allows for easier troubleshooting. Here are the commit messages (with minor edits):
1.

    Logging (Linux): emit basic GPU info at loglevel info
    
    On multi-GPU systems it's useful to have essential
    GPU info in Sunshine's log at the default 'info'
    loglevel.
    
    In particular, if x11grab was used in combination with
    VA-API encoding, there was no indication at all about
    the currently used GPU.
    Logging(Linux): log IDR frames at loglevel debug
    
    IDR frames seem to be related to the occasional DATA_SHARDS_MAX
    warnings observed by some AMD users.
Linux/x11grab: add frame processing latency
    
    This commit populates the img->frame_timestamp field, so that
    Moonlight is now capable of showing the "Host processing latency"
    in its statistics overlay.
    
    It's a complement to PR #2273 which missed the codepath for
    shm_attr_t::snapshot().
    Logging(Linux/KMS): emit short note about AppImage/Flatpak
    
    The setcap command won't work for AppImage and Flatpak packages.
    Refer the user to the official documentation for further
    instructions.
Logging/Opus: add basic stream info at loglevel 'info'
    
    Previously no information at all was logged about the used codec.

Example:

[2024:05:11:13:56:49]: Info: Setting default sink to: [sink-sunshine-stereo]
[2024:05:11:13:56:49]: Info: Found default monitor by name: sink-sunshine-stereo.monitor
[2024:05:11:13:56:49]: Info: Opus initialized: 48 kHz, 2 channels, 512 kbps (total), LOWDELAY

Screenshot

Issues Fixed or Closed

improves upon #2273

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

@gschintgen
Copy link
Contributor Author

I think I'll have to revert the loglevel change for the GL: render string. It's misleading:

[2024:05:05:23:18:30]: Info: Sunshine version: 0.23.1.b643a6c
[2024:05:05:23:18:30]: Info: Detecting displays
[2024:05:05:23:18:30]: Info: Detected display: DisplayPort-1 (id: 0)DisplayPort-1 connected: true
[2024:05:05:23:18:30]: Info: Detected display: DisplayPort-2 (id: 1)DisplayPort-2 connected: false
[2024:05:05:23:18:30]: Info: Detected display: HDMI-A-2 (id: 2)HDMI-A-2 connected: false
[2024:05:05:23:18:30]: Info: Detected display: HDMI-A-3 (id: 3)HDMI-A-3 connected: false
[2024:05:05:23:18:30]: Info: Detected display: DP-1-1 (id: 4)DP-1-1 connected: false
[2024:05:05:23:18:30]: Info: Detected display: HDMI-1-1 (id: 5)HDMI-1-1 connected: false
[2024:05:05:23:18:30]: Info: Detected display: HDMI-1-2 (id: 6)HDMI-1-2 connected: false
[2024:05:05:23:18:30]: Info: Trying encoder [vaapi]
[2024:05:05:23:18:30]: Info: Screencasting with X11
[2024:05:05:23:18:30]: Info: GL: renderer: Mesa Intel(R) UHD Graphics 770 (ADL-S GT1)
[2024:05:05:23:18:30]: Info: SDR color coding [Rec. 601]
[2024:05:05:23:18:30]: Info: Color depth: 8-bit
[2024:05:05:23:18:30]: Info: Color range: [JPEG]
[2024:05:05:23:18:30]: Info: vaapi vendor: Intel iHD driver for Intel(R) Gen Graphics - 22.3.1 ()
[2024:05:05:23:18:30]: Info: System tray created

The Intel strings are then repeated quite a lot (due to all the encoder checks that are done), but there's not a single mention of AMD when in fact the rendering and capturing is done on the AMD dGPU. The output above is for forced x11 capture with adapter_name pointing to the Intel iGPU. My display is physically connected to the AMD RX6650XT. The "vaapi vendor" output is more helpful, even if it does not include the exact GPU type. (For the AMD card both strings contain the exact chipset.)

ReenigneArcher
ReenigneArcher previously approved these changes May 11, 2024
@gschintgen
Copy link
Contributor Author

Ah sorry, it seems that I messed up the formatting. At least clang-format finds things to change. I ran it from the CLI and since it ran cleanly I thought everything was fine, only to notice afterwards that the -i flag stands for edit in-place...

Anyway I will work my dev setup to avoid further formatting issues and force push fixed commits. There won't be any actual code changes.

@ReenigneArcher
Copy link
Member

Copy link

codecov bot commented May 11, 2024

Codecov Report

Attention: Patch coverage is 0% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 6.99%. Comparing base (659a426) to head (3200732).

Additional details and impacted files
@@            Coverage Diff             @@
##           nightly   #2502      +/-   ##
==========================================
- Coverage     7.00%   6.99%   -0.01%     
==========================================
  Files           87      87              
  Lines        17670   17679       +9     
  Branches      8391    8399       +8     
==========================================
  Hits          1237    1237              
- Misses       13709   13713       +4     
- Partials      2724    2729       +5     
Flag Coverage Δ
Linux 5.34% <0.00%> (-0.01%) ⬇️
Windows 2.56% <0.00%> (-0.01%) ⬇️
macOS-12 7.99% <0.00%> (-0.01%) ⬇️
macOS-13 ?
macOS-14 8.24% <0.00%> (-0.01%) ⬇️

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

Files Coverage Δ
src/platform/linux/vaapi.cpp 2.79% <0.00%> (ø)
src/platform/linux/x11grab.cpp 34.75% <0.00%> (-0.18%) ⬇️
src/video.cpp 25.04% <0.00%> (-0.05%) ⬇️
src/audio.cpp 1.55% <0.00%> (-0.04%) ⬇️
src/platform/linux/kmsgrab.cpp 2.80% <0.00%> (-0.01%) ⬇️

@gschintgen
Copy link
Contributor Author

Make sure you are using spaces, not tabs: https://github.com/LizardByte/Sunshine/actions/runs/9043429894/job/24853606135?pr=2502#step:5:14

Yes, I've now made my dev container available over the network so that I can properly edit remotely with vscode, including the clang-format extension...

On multi-GPU systems it's useful to have essential
GPU info in Sunshine's log at the default 'info'
loglevel.

In particular, if x11grab was used in combination with
VA-API encoding, there was no indication at all about
the currently used GPU.
IDR frames seem to be related to the occasional DATA_SHARDS_MAX
warnings observed by some AMD users.
This commit populates the img->frame_timestamp field, so that
Moonlight is now capable of showing the "Host processing latency"
in its statistics overlay.

It's a complement to PR LizardByte#2273 which did miss the codepath for
shm_attr_t::snapshot().
The setcap won't work for AppImage and Flatpak packages.
Refer the user to the official documentation for further
instructions.
Previously no information at all was logged about the used codec.
@ReenigneArcher ReenigneArcher changed the title Minor improvements to logging (Linux) fix(linux): add frame processing latency and logging improvements May 12, 2024
@ReenigneArcher ReenigneArcher merged commit 0a595dc into LizardByte:nightly May 12, 2024
50 of 53 checks passed
@gschintgen gschintgen deleted the better-logging branch May 13, 2024 13:06
@Hazer Hazer mentioned this pull request May 30, 2024
11 tasks
KuleRucket pushed a commit to KuleRucket/Sunshine that referenced this pull request Jun 6, 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