-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
discord: krisp module doesn't load #195512
Comments
i checked the flatpak repo (commits,issues) but couldn't find a mention of krisp no mention of krisp in strace either |
Still an issue on latest canary build (a491bc2, host 0.0.143). UPDATE: It seems the issue originates from binary stripping. Arch Linux had the same issue, and their fix was to skip binary stripping with |
This is due to discord adding a signature check against the discord binaries inside the Krisp module, which fail because we patch the binaries to make discord load properly in nixpkgs. Unfortunately we can't patch this from nixpkgs, since discord downloads modules at runtime. But it is possible to manually do it yourself by editing the file Inside the function util::GetMyProcessFilename(&discord_exe_filename);
is_signed = util::IsSignedByDiscord(&discord_exe_filename);
if (is_signed) {
// Load Krisp...
}
else {
puts("Application not signed by Discord, Krisp is not enabled");
return -3;
} Looking at the assembly code for the linux version, we can patch the JZ instruction with a 2 byte NOP to go directly to the Krisp loading code, instead of failing: LEA RBX=>discord_exe_filename,[RSP + 0xe8]
MOV RDI,RBX
CALL discord::util::GetMyProcessFilename
MOV RDI,RBX
CALL discord::util::IsSignedByDiscord
TEST is_signed,is_signed
JZ return_application_not_signed_error ;; instruction is two bytes long
;; Code to load Krisp
return_application_not_signed_error:
;; Failure I've attached an already patched file, which you can replace the already existing file mentioned above with after unpacking it with gunzip. It's based on the krisp module from discord version 0.0.21: |
doing it in the wrapper is possible like i did with disabling breaking updates in #197248 |
How exactly could this be done? I'm assuming that putting the patched file in the nixpkgs repo would be impossible due to license issues. There's also the issue of version differences, which will complicate things further |
#!/usr/bin/env nix-shell
#!nix-shell -i bash -p rizin
addr=$(rz-find -x '4889dfe8........4889dfe8' discord_krisp.node | head -n1)
rizin -q -w -c "s $addr + 0x12 ; wao nop" discord_krisp.node Here is a patcher script, it should work for future versions given the AoB stays valid. It requires no interaction and does not require distributing a patched file so someone can incorporate this into nixpkgs. |
What about packaging discord inside an fhs env without patching discord executable at all? |
I've packaged @S-NA 's solution on One can just run: nix run "github:steinerkelvin/dotfiles#discord-krisp-patch" and Discord will be patched. (it's not a package for Discord, just to run the patching script) |
Looks like patches above no longer work on discord-canary
|
You can try this, make sure to start from a clean
|
Thanks! Checked on canary and it indeed works :) |
It seems Discord updated to |
@steinerkelvin I just tried running the latest patcher on |
@blkgoose oh, nice. I'll update my flake. |
Hey how do I run this? I try to run it inside and outside of nix-shell and my terminal just tells me |
It wasn't quoted, I fixed it. |
Thank you! I gave it try again but now it's saying the flake doesn't provide the attributes |
btw I have a patcher for discord-canary in my flake, it should also update itself depending on discord-canary version in nixpkgs
https://github.com/surfaceflinger/flake/blob/master/packages/krisp-patch/default.nix |
Has anyone made a patcher for v0.0.29? The patchers listed here don't seem to work at all.
|
Instead of hard coding the Discord version why not just wildcard it? |
pin the nixpkgs in the registry to your system revision and do |
Thank you! But how the issue is that it's still hardcoded to 0.0.28 and not to the latest version of Discord (which is 0.0.29). Like what @S-NA, the versions shouldn't be hardcoded because then it requires the repo to be updated every time Discord updates |
I get this error when i run nix run "github:steinerkelvin/dotfiles#discord-krisp-patch" . Using sudo does not help Cannot open file '/root/.config/discord/0.0.31/modules/discord_krisp/discord_krisp.node' |
@peat734 I "fixed" by applying the patch manually. But perhaps you could also try to change the permissions of the file. |
@Dokkae6949 i also tried applying the patch manually and i got this new error |
fwiw, I have a script that doesn't produce those warnings: https://github.com/sersorrel/sys/blob/f7ff2fa325f786123a9b9e9a61b07be409dfb0b1/hm/discord/krisp-patcher.py (see the adjacent use like so: |
After using the script the krisp module still doesn't load |
Thanks for sharing the script! Really appreciate it! I made a basic home-manager module around it, which just wraps the Discord binary, and runs the patcher each time. Makes Discord slower to start, but.. doesn't really make any difference. { config, pkgs, lib, ... }:
let
cfg = config.programs.discord;
discordPatcherBin = pkgs.writers.writePython3Bin "discord-krisp-patcher" {
libraries = with pkgs.python3Packages; [ pyelftools capstone ];
flakeIgnore = [
"E265" # from nix-shell shebang
"E501" # line too long (82 > 79 characters)
"F403" # ‘from module import *’ used; unable to detect undefined names
"F405" # name may be undefined, or defined from star imports: module
];
} (builtins.readFile ./discord-patcher.py);
wrapDiscordBinary = pkgs.writeShellScriptBin "discord" ''
${pkgs.findutils}/bin/find -L $HOME/.config/discord -name 'discord_krisp.node' -exec ${discordPatcherBin}/bin/discord-krisp-patcher {} +
${pkgs.discord}/bin/discord "$@"
'';
in {
options.programs.discord = {
enable = lib.mkEnableOption "Discord";
wrapDiscord = lib.mkEnableOption "wrap the Discord binary with a patching each time";
};
config = lib.mkIf cfg.enable {
home.packages = [ discordPatcherBin ]
++ (if cfg.wrapDiscord
then [ wrapDiscordBinary ]
else [ pkgs.discord ]
);
# consideered adding a service here, that would patch discord using
# a systemd service, but instead just opted to patch each time Discord starts
};
} Requires putting the script into the same folder, as this nix file :) |
@peat734 If you want to debug it you can DM me on Discord ( I've updated my flake so now it detects the Discord version.
The Python script from @sersorrel seems to be better, tho. Maybe I'll incorporate it later. |
Reporting in that the patch works for me as well, and @eyJhb 's code activates it cleanly. Thanks all for the patches. |
Also using this successfully - thanks for the patches folks! I modified @eyJhb's wrapper mostly to add a .desktop file for easy desktop launching. It's in my nix-configs repo here. |
I was also faced with this Krisp issue, as well as the issue where it's not possible to stream a window with audio on my NixOS system. For the audio problem, I saw there was a different build for discord, discord-screenaudio. And for the Krisp problem, I found this thread with the python script solution. However, I found no nice way of combining the two, until I read through the discord-screenaudio alternatives and found So my solution to both problems was:
Now I can stream with audio, and Krisp is working nicely :) Keep in mind this might be against TOS, but Discord is not likely to ban you (read the disclaimers). Edit 03.07.2024: Krisp doesn't work anymore on vesktop |
Hi @m1cr0man, using your modified wrapper I get this:
Not sure what's wrong. |
Use the latest patcher here https://github.com/sersorrel/sys/blob/main/hm/discord/krisp-patcher.py |
If anyone wants to update it in the future, you might, maybe, could, potentially, somewhat... Not sure.. But grep for the following strings, and replace them in the patcher script, like so...
|
Want to add to this as well, it seems like Vesktop has "krisp" now, but it doesn't work, at all. :) |
Yep, it's not working anymore. |
@hallowatcher did you try to copy the new values from here, into the patcher script? krisp_initialize_address = symtab.get_symbol_by_name("_ZN7discord15KrispInitializeEv")[0].entry.st_value
isSignedByDiscord_address = symtab.get_symbol_by_name("_ZN7discord4util17IsSignedByDiscordERKNSt4__Cr12basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEE")[0].entry.st_value EDIT: I just closed and opened Discord again, to confirm no new updates, and that it was working. |
I meant the Vesktop solution isn't working anymore. Krisp is working nicely now with the patch script: |
|
update for latest changes to the krisp module (the jump changed size): https://github.com/sersorrel/sys/blob/de1ce2ba941318a05d4d029f717ad8be7b4b09ee/hm/discord/krisp-patcher.py |
Looks like it broke again 🙁 |
fixed: sersorrel/sys@d19bd7c |
Will this be updated on the wiki? |
the wiki appears to link to this issue already; however, you can edit the wiki if you feel it can be improved |
broke again with v0.0.70, now fixed: sersorrel/sys@7806b21 |
For anyone still struggling with this issue and disliking the idea of a krisp-patcher script, I have created a generic electron-wrapper that allows for any url. At the time of writing, support for external urls, screen sharing, spell checking, window management, and krisp is possible! You can use my derivation at: electron-wrapper.nix with your discord config like this: discord.nix. This should (as far as I know) be fine with their TOS, the reason I chose to make this wrapper! |
Describe the bug
Discord has supported Krisp audio filtering for about 2 months now on GNU/Linux. The problem is that Krisp module simply doesn't load if we're using Discord from nixpkgs. It works on all my machines when I install Discord from Flatpak.
Steps To Reproduce
Steps to reproduce the behavior:
Expected behavior
Krisp should be available
Screenshots
On the left - discord from nixpkgs
On the right - discord-canary from flathub
Additional context
I have OpenAsar installed but I tried without it and it's still not loading. Friend has also suggested that probably Discord expects some more standard paths but Krisp is being loaded from home directory. eg.
./.config/discord/0.0.20/modules/discord_krisp
so it shouldn't be a problem.nixpkgs discord log:
flathub discord log:
Notify maintainers
@devins2518 @Artturin @Infinidoge
Metadata
Please run
nix-shell -p nix-info --run "nix-info -m"
and paste the result.The text was updated successfully, but these errors were encountered: