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

Expose Connection environment variables to the app start #1430

Conversation

TheElixZammuto
Copy link
Contributor

@TheElixZammuto TheElixZammuto commented Jul 7, 2023

Description

When starting apps, some environment variables releated to the connection are passed to the commands (pre/detatched/do)
Instead of #1096 that relies on text replacement, this uses the environment variables system, which should be more robust and secure (Although I haven't tested cmd injection techinques). I've also documented inside the Web UI which vars are exposed and how to consume them in a standard cmd command on Windows (other platform suggestions are welcome!)

This should make scripts more elegant instead of relying on parsing log files.

Screenshot

env_vars

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

src/nvhttp.cpp Outdated Show resolved Hide resolved
src/nvhttp.cpp Outdated Show resolved Hide resolved
src/nvhttp.cpp Show resolved Hide resolved
src/process.cpp Outdated Show resolved Hide resolved
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.

src_assets/common/assets/web/apps.html Outdated Show resolved Hide resolved
src_assets/common/assets/web/apps.html Outdated Show resolved Hide resolved
@Nonary
Copy link
Collaborator

Nonary commented Jul 13, 2023

Ideally all parameters of the client should be accessible, anyone opposed to something like that? I’m guessing it would be documentation and more code changes?

@Nonary
Copy link
Collaborator

Nonary commented Jul 13, 2023

Specifically there is a bunch of moonlight variables logged when the log level as set to debug, which gives a lot of useful information. I’ve been wanting to make those accessible but it appears those are logged after the stream starts, not beforehand.

@TheElixZammuto
Copy link
Contributor Author

Specifically there is a bunch of moonlight variables logged when the log level as set to debug, which gives a lot of useful information. I’ve been wanting to make those accessible but it appears those are logged after the stream starts, not beforehand.

Yup, the app starts when a /launch command is issued on the client side, so we can only access the variables sent using this endpoint. Do you have any specific variable you want to be passed?

src/nvhttp.cpp Outdated Show resolved Hide resolved
@Nonary
Copy link
Collaborator

Nonary commented Jul 16, 2023

Specifically there is a bunch of moonlight variables logged when the log level as set to debug, which gives a lot of useful information. I’ve been wanting to make those accessible but it appears those are logged after the stream starts, not beforehand.

Yup, the app starts when a /launch command is issued on the client side, so we can only access the variables sent using this endpoint. Do you have any specific variable you want to be passed?

All of the parameters logged that start with a=nv-video would be great

a=x-nv-video[0].clientViewportWd:3840 
a=x-nv-video[0].clientViewportHt:2160 
a=x-nv-video[0].maxFPS:60 
a=x-nv-video[0].packetSize:1392 
a=x-nv-video[0].rateControlMode:4 
a=x-nv-video[0].timeoutLengthMs:7000 
a=x-nv-video[0].framesWithInvalidRefThreshold:0 
a=x-nv-video[0].initialBitrateKbps:80000 
a=x-nv-video[0].initialPeakBitrateKbps:80000 
a=x-nv-vqos[0].bw.minimumBitrateKbps:80000 
a=x-nv-vqos[0].bw.maximumBitrateKbps:80000 
a=x-nv-vqos[0].fec.enable:1 
a=x-nv-vqos[0].videoQualityScoreUpdateTime:5000 
a=x-nv-vqos[0].qosTrafficType:5 
a=x-nv-aqos.qosTrafficType:4 
a=x-nv-general.featureFlags:167 
a=x-nv-general.useReliableUdp:13 
a=x-nv-vqos[0].fec.minRequiredFecPackets:2 
a=x-nv-vqos[0].bllFec.enable:0 
a=x-nv-vqos[0].drc.enable:0 
a=x-nv-general.enableRecoveryMode:0 
a=x-nv-video[0].videoEncoderSlicesPerFrame:1 
a=x-nv-clientSupportHevc:1 
a=x-nv-vqos[0].bitStreamFormat:1 
a=x-nv-video[0].dynamicRangeMode:1 
a=x-nv-video[0].maxNumReferenceFrames:1 
a=x-nv-video[0].clientRefreshRateX100:0 
a=x-nv-audio.surround.numChannels:2 
a=x-nv-audio.surround.channelMask:3 
a=x-nv-audio.surround.enable:0 
a=x-nv-audio.surround.AudioQuality:0 
a=x-nv-aqos.packetDuration:5 
a=x-nv-video[0].encoderCscMode:0 

But I suppose just having HDR, fps and resolution is fine for my use cases though.

What's the option for figuring out if user has enabled/disabled the feature to close games on stream end? I could use that for my playnite watcher script.

@TheElixZammuto
Copy link
Contributor Author

TheElixZammuto commented Jul 17, 2023

Specifically there is a bunch of moonlight variables logged when the log level as set to debug, which gives a lot of useful information. I’ve been wanting to make those accessible but it appears those are logged after the stream starts, not beforehand.

Yup, the app starts when a /launch command is issued on the client side, so we can only access the variables sent using this endpoint. Do you have any specific variable you want to be passed?

All of the parameters logged that start with a=nv-video would be great

a=x-nv-video[0].clientViewportWd:3840 
...

But I suppose just having HDR, fps and resolution is fine for my use cases though.

Unfortunately these are passed on the RTSP stage, so they cannot be passed on the app start

What's the option for figuring out if user has enabled/disabled the feature to close games on stream end? I could use that for my playnite watcher script.

It seems that it's a client-only option

EDIT: I think the best thing is to make in a future PR a named-pipe to handle all of these cases, so the integration can be completed

src/process.h Outdated Show resolved Hide resolved
Co-authored-by: ReenigneArcher <[email protected]>
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.

Can we add a macOS example?

Also, how about adding a section about the env variables to the docs? I could take care of this in a separate PR in order to get this moving along.

src_assets/common/assets/web/apps.html Show resolved Hide resolved
@ReenigneArcher ReenigneArcher merged commit 3b2a098 into LizardByte:nightly Jul 29, 2023
Comment on lines +281 to +289
launch_session.unique_id = (get_arg(args, "uniqueid"));
launch_session.uuid = (get_arg(args, "uuid"));
launch_session.appid = util::from_view(get_arg(args, "appid"));
launch_session.enable_sops = util::from_view(get_arg(args, "sops"));
launch_session.surround_info = util::from_view(get_arg(args, "surroundAudioInfo"));
launch_session.gcmap = util::from_view(get_arg(args, "gcmap"));
launch_session.enable_hdr = 0;
if (args.find("enableHdr"s) != std::end(args)) {
launch_session.enable_hdr = util::from_view(get_arg(args, "enableHdr"));
Copy link
Member

Choose a reason for hiding this comment

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

@TheElixZammuto

Can we set a default value to empty/null if the client doesn't provide it? I'm thinking something similar to Python's os.getenv method. os.getenv(key, "some default value")

Apparently, Moonlight on Apple doesn't provide UUID (and who knows about every possible homebrew moonlight client). https://discord.com/channels/804382334370578482/1074377411069747250/1135639228613664858

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps something like

  std::string
  get_arg(const args_t &args, const char *name, const char *default_value = nullptr) {
    auto it = args.find(name);
    if (it == std::end(args)) {
      if (default_value != NULL) {
        return std::string(default_value);
      }
      
      throw std::out_of_range(name);
    }
    return it->second;
  }

Used as such:

   launch_session.gcm_key = util::from_hex<crypto::aes_t>(get_arg(args, "rikey", "0"), true);
    std::stringstream mode = std::stringstream(get_arg(args, "mode", "0x0x0"));
    // Split mode by the char "x", to populate width/height/fps
    int x = 0;
    std::string segment;
    while (std::getline(mode, segment, 'x')) {
      if (x == 0) launch_session.width = atoi(segment.c_str());
      if (x == 1) launch_session.height = atoi(segment.c_str());
      if (x == 2) launch_session.fps = atoi(segment.c_str());
      x++;
    }
    
    launch_session.unique_id = (get_arg(args, "uniqueid", "0123456789ABCDEF"));
    launch_session.uuid = (get_arg(args, "uuid", "00000000000000000000000000000000"));
    launch_session.appid = util::from_view(get_arg(args, "appid", "0123456789"));
    launch_session.enable_sops = util::from_view(get_arg(args, "sops", "0"));
    launch_session.surround_info = util::from_view(get_arg(args, "surroundAudioInfo", "0"));
    launch_session.gcmap = util::from_view(get_arg(args, "gcmap", "0"));
    launch_session.enable_hdr = 0;
    if (args.find("enableHdr"s) != std::end(args)) {
      launch_session.enable_hdr = util::from_view(get_arg(args, "enableHdr", "0"));
    }
    uint32_t prepend_iv = util::endian::big<uint32_t>(util::from_view(get_arg(args, "rikeyid", "0")));

@TheElixZammuto TheElixZammuto deleted the connection-environment-variables branch August 14, 2023 16:52
KuleRucket pushed a commit to KuleRucket/Sunshine that referenced this pull request Jun 6, 2024
e-dong pushed a commit to e-dong/Sunshine that referenced this pull request Jul 26, 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.

6 participants