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.

Sign up for GitHub

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): Configure display device based on user config #3441

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
+1,685 −96
Open

feat(display): Configure display device based on user config #3441

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2215927
feat: Configure display device based on user config
FrogTheFrog Jul 20, 2024
8f00b29
Review fixes: vue and json
FrogTheFrog Dec 12, 2024
e735955
Review fixes: configuration.md (mostly)
FrogTheFrog Dec 12, 2024
88016a4
Review fixes: fix invalid option name in web_ui
FrogTheFrog Dec 12, 2024
8900c15
Review fixes: checkbox values
FrogTheFrog Dec 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
204 changes: 204 additions & 0 deletions docs/configuration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -942,6 +942,210 @@ editing the `conf` file in a text editor. Use the examples as reference.
</tr>
</table>

### dd_configuration_option

<table>
<tr>
<td>Description</td>
<td colspan="2">
@warning{Windows only!}
Perform additional configuration for the display device:
<ul>
<li>`disabled` - perform no additional configuration (disables all `dd_` configuration options).</li>
<li>`verify_only` - verify that display is active only (required for changing display mode and other options).</li>
<li>`ensure_active` - activate the display if it's currently inactive.</li>
<li>`ensure_primary` - activate the display if it's currently inactive and make it primary.</li>
<li>`ensure_only_display` - activate the display if it's currently inactive and disable all others.</li>
</ul>
FrogTheFrog marked this conversation as resolved.
Show resolved Hide resolved
</td>
FrogTheFrog marked this conversation as resolved.
Show resolved Hide resolved
</tr>
<tr>
<td>Default</td>
<td colspan="2">@code{}verify_only@endcode</td>
</tr>
<tr>
<td>Example</td>
<td colspan="2">@code{}
dd_configuration_option = ensure_only_display
@endcode</td>
</tr>
</table>

### dd_resolution_option

<table>
<tr>
<td>Description</td>
<td colspan="2">
@warning{Windows only!}
@note{"Optimize game settings" must be enabled for this option to work.}
FrogTheFrog marked this conversation as resolved.
Show resolved Hide resolved
Perform additional resolution configuration for the display device:
<ul>
<li>`disabled` - perform no additional configuration.</li>
<li>`automatic` - change resolution to the requested resolution from the client.</li>
<li>`manual` - change resolution to the user specified one (set via `dd_manual_resolution`).</li>
</ul>
</td>
</tr>
<tr>
<td>Default</td>
<td colspan="2">@code{}automatic@endcode</td>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<td colspan="2">@code{}automatic@endcode</td>
<td colspan="2">Automatic</td>

Note

Applies in a few places.

Copy link
Collaborator Author

@FrogTheFrog FrogTheFrog Dec 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

My intention was to provide an actual value in a code block. Do you suggest to remove the code block or to also make it a description (in a sense), because you capitalized the first letter?

</tr>
<tr>
<td>Example</td>
<td colspan="2">@code{}
dd_resolution_option = manual
@endcode</td>
</tr>
</table>

### dd_manual_resolution

<table>
<tr>
<td>Description</td>
<td colspan="2">
@warning{Windows only!}
@note{`dd_resolution_option` must be set to `manual`}
FrogTheFrog marked this conversation as resolved.
Show resolved Hide resolved
Specify manual resolution to be used.
</td>
</tr>
<tr>
<td>Default</td>
<td colspan="2">No value</td>
FrogTheFrog marked this conversation as resolved.
Show resolved Hide resolved
</tr>
<tr>
<td>Example</td>
<td colspan="2">@code{}
dd_manual_resolution = 1920x1080
@endcode</td>
</tr>
</table>

### dd_refresh_rate_option

<table>
<tr>
<td>Description</td>
<td colspan="2">
@warning{Windows only!}
Perform additional refresh rate configuration for the display device:
<ul>
<li>`disabled` - perform no additional configuration.</li>
<li>`automatic` - change refresh rate to the requested FPS value from the client.</li>
<li>`manual` - change refresh rate to the user specified one (set via `dd_manual_refresh_rate`).</li>
</ul>
</td>
</tr>
<tr>
<td>Default</td>
<td colspan="2">@code{}automatic@endcode</td>
</tr>
<tr>
<td>Example</td>
<td colspan="2">@code{}
dd_refresh_rate_option = manual
@endcode</td>
</tr>
</table>

### dd_manual_refresh_rate

<table>
<tr>
<td>Description</td>
<td colspan="2">
@warning{Windows only!}
@note{`dd_refresh_rate_option` must be set to `manual`}
Specify manual refresh rate to be used.
</td>
</tr>
<tr>
<td>Default</td>
<td colspan="2">No value</td>
</tr>
<tr>
<td>Example</td>
<td colspan="2">@code{}
dd_manual_resolution = 120
dd_manual_resolution = 59.95
@endcode</td>
</tr>
</table>

### dd_hdr_option

<table>
<tr>
<td>Description</td>
<td colspan="2">
@warning{Windows only!}
Perform additional HDR configuration for the display device:
<ul>
<li>`disabled` - perform no additional configuration.</li>
<li>`automatic` - change HDR to the requested state from the client if the display supports it.</li>
</ul>
</td>
</tr>
<tr>
<td>Default</td>
<td colspan="2">@code{}automatic@endcode</td>
</tr>
<tr>
<td>Example</td>
<td colspan="2">@code{}
dd_hdr_option = disabled
@endcode</td>
</tr>
</table>

### dd_wa_hdr_toggle

<table>
<tr>
<td>Description</td>
<td colspan="2">
@warning{Windows only!}
@note{This option works independently of `dd_hdr_option`}
When using virtual display device as for streaming, it might display incorrect (high-contrast) color.
With this option enabled, Sunshine will try to mitigate this issue.
</td>
</tr>
<tr>
<td>Default</td>
<td colspan="2">@code{}false@endcode</td>
FrogTheFrog marked this conversation as resolved.
Show resolved Hide resolved
</tr>
<tr>
<td>Example</td>
<td colspan="2">@code{}
dd_wa_hdr_toggle = true
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
dd_wa_hdr_toggle = true
dd_wa_hdr_toggle = enabled

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is not correct:
image

The saved option is boolean.

Copy link
Member

Choose a reason for hiding this comment

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

I really like the idea of check boxes, but currently we aren't using those anywhere else. So I think that should come in a separate PR changing them universally from the select menus.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I really like the idea of check boxes, but currently we aren't using those anywhere else. So I think that should come in a separate PR changing them universally from the select menus.

I have fixed this issue already. The enabled works with them like a charm. I hope that I don't need to change the checkbox to the menu (your remark leaves it open for interpretation)?

Copy link
Member

Choose a reason for hiding this comment

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

It would be quite weird to have one checkbox in the ui, while everywhere else is a select menu.

I know you don't like to open more than one PR at once, but changing them out globally would be better. If a separate PR is created to change that pattern, then it's fine to use it here as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll create a separate PR then

Copy link
Member

Choose a reason for hiding this comment

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

I'll prioritize reviewing and merging it whenever it's ready. Thanks!

@endcode</td>
</tr>
</table>

### dd_config_revert_delay

<table>
<tr>
<td>Description</td>
<td colspan="2">
@warning{Windows only!}
Additional delay in milliseconds to wait before reverting configuration when the app has been closed or the last session terminated.
Main purpose is to provide a smoother transition when quickly switching between apps.
</td>
</tr>
<tr>
<td>Default</td>
<td colspan="2">@code{}3000@endcode</td>
</tr>
<tr>
<td>Example</td>
<td colspan="2">@code{}
dd_config_revert_delay = 1500
@endcode</td>
</tr>
</table>

### min_fps_factor

<table>
Expand Down
34 changes: 21 additions & 13 deletions src/audio.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,6 @@ namespace audio {
using opus_t = util::safe_ptr<OpusMSEncoder, opus_multistream_encoder_destroy>;
using sample_queue_t = std::shared_ptr<safe::queue_t<std::vector<float>>>;

struct audio_ctx_t {
// We want to change the sink for the first stream only
std::unique_ptr<std::atomic_bool> sink_flag;

std::unique_ptr<platf::audio_control_t> control;

bool restore_sink;
platf::sink_t sink;
};

static int
start_audio_control(audio_ctx_t &ctx);
static void
Expand Down Expand Up @@ -95,8 +85,6 @@ namespace audio {
},
};

auto control_shared = safe::make_shared<audio_ctx_t>(start_audio_control, stop_audio_control);

void
encodeThread(sample_queue_t samples, config_t config, void *channel_data) {
auto packets = mail::man->queue<packet_t>(mail::audio_packets);
Expand Down Expand Up @@ -149,7 +137,7 @@ namespace audio {
apply_surround_params(stream, config.customStreamParams);
}

auto ref = control_shared.ref();
auto ref = get_audio_ctx_ref();
if (!ref) {
return;
}
Expand Down Expand Up @@ -255,6 +243,26 @@ namespace audio {
}
}

audio_ctx_ref_t
get_audio_ctx_ref() {
static auto control_shared { safe::make_shared<audio_ctx_t>(start_audio_control, stop_audio_control) };
return control_shared.ref();
}

bool
is_audio_ctx_sink_available(const audio_ctx_t &ctx) {
if (!ctx.control) {
return false;
}

const std::string &sink = ctx.sink.host.empty() ? config::audio.sink : ctx.sink.host;
if (sink.empty()) {
return false;
}

return ctx.control->is_sink_available(sink);
}

int
map_stream(int channels, bool quality) {
int shift = quality ? 1 : 0;
Expand Down
44 changes: 44 additions & 0 deletions src/audio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
*/
#pragma once

// local includes
#include "platform/common.h"
#include "thread_safe.h"
#include "utility.h"

Expand Down Expand Up @@ -55,8 +57,50 @@ namespace audio {
std::bitset<MAX_FLAGS> flags;
};

struct audio_ctx_t {
// We want to change the sink for the first stream only
std::unique_ptr<std::atomic_bool> sink_flag;

std::unique_ptr<platf::audio_control_t> control;

bool restore_sink;
platf::sink_t sink;
};

using buffer_t = util::buffer_t<std::uint8_t>;
using packet_t = std::pair<void *, buffer_t>;
using audio_ctx_ref_t = safe::shared_t<audio_ctx_t>::ptr_t;

void
capture(safe::mail_t mail, config_t config, void *channel_data);

/**
* @brief Get the reference to the audio context.
* @returns A shared pointer reference to audio context.
* @note Aside from the configuration purposes, it can be used to extend the
* audio sink lifetime to capture sink earlier and restore it later.
*
* @examples
* audio_ctx_ref_t audio = get_audio_ctx_ref()
* @examples_end
*/
audio_ctx_ref_t
get_audio_ctx_ref();

/**
* @brief Check if the audio sink held by audio context is available.
* @returns True if available (and can probably be restored), false otherwise.
* @note Useful for delaying the release of audio context shared pointer (which
* tries to restore original sink).
*
* @examples
* audio_ctx_ref_t audio = get_audio_ctx_ref()
* if (audio.get()) {
* return is_audio_ctx_sink_available(*audio.get());
* }
* return false;
* @examples_end
*/
bool
is_audio_ctx_sink_available(const audio_ctx_t &ctx);
} // namespace audio
Loading
Loading