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

Update request: rnnoise 2021-01-22 → 0.2 #304433

Closed
1 task done
toastal opened this issue Apr 16, 2024 · 5 comments · Fixed by #305348
Closed
1 task done

Update request: rnnoise 2021-01-22 → 0.2 #304433

toastal opened this issue Apr 16, 2024 · 5 comments · Fixed by #305348

Comments

@toastal
Copy link
Contributor

toastal commented Apr 16, 2024

  • Package name: rnnoise
  • Latest released version: 0.2
  • Current version on the unstable channel: 2021-01-22
  • Current version on the stable/release channel: 2021-01-22

Notify maintainers

@nh2


Note for maintainers: Please tag this issue in your PR.

https://gitlab.xiph.org/xiph/rnnoise/-/tree/v0.2
https://github.com/xiph/rnnoise/releases/tag/v0.2

Tried building with

-stdenv.mkDerivation (rec {
+stdenv.mkDerivation (finalAttrs: {
   pname = "rnnoise";
-  version = "2021-01-22";
+  version = "0.2";
 
   src = fetchFromGitHub {
     owner = "xiph";
     repo = "rnnoise";
-    rev = "1cbdbcf1283499bbb2230a6b0f126eb9b236defd";
-    sha256 = "1y0rzgmvy8bf9a431garpm2w177s6ajgf79y5ymw4yb0pik57rwb";
+    rev = "refs/tags/v${finalAttrs.version}";
+    sha256 = "sha256-Qaf+0iOprq7ILRWNRkBjsniByctRa/lFVqiU5ZInF/Q=";
   };

but it got a missing file.

       > In file included from src/denoise.c:43:
       > src/rnn.h:31:10: fatal error: rnnoise_data.h: No such file or directory
       >    31 | #include "rnnoise_data.h"
       >       |          ^~~~~~~~~~~~~~~~
       > compilation terminated.
       > make[1]: *** [Makefile:749: src/dump_features-denoise.o] Error 1
       > make[1]: Leaving directory '/build/source'
       > make: *** [Makefile:491: all] Error 2

There’s also warning about not using SSE3/AVX/AVX2, but Nix only supports the bottom of the barrel, common x86 instructions, but I usually compile this one out with some flags.


Add a 👍 reaction to issues you find important.

1

Footnotes

  1. Please consider giving up MS GitHub or offering a non-proprietary, non-US-corporate-controlled mirror for this free software project. I wish to delete this Microsoft account in the future, but I need more projects like this to support alternative methods to send patches & contribute.

@DarthPJB
Copy link
Contributor

DarthPJB commented Apr 16, 2024

The missing file is downloaded as part of the 'autogen.sh'
It looks like they added a step to the build process.

xiph/rnnoise#219 (comment)

@JohnRTitor
Copy link
Contributor

Perhaps it can be fetched using fetchurl/fetchpatch?

@kilianar
Copy link
Contributor

You're correct. The relevant file is retrieved in the script download_model.sh. It fetches a single file from the URL https://media.xiph.org/rnnoise/models/rnnoise_data-${version}.tar.gz, where the version is extracted from the file model_version, currently set to 0b50c45. Hence, we can emulate the download script and execute something similar to:

model_version = "0b50c45";
model = fetchurl {
  url = "https://media.xiph.org/rnnoise/models/rnnoise_data-${model_version}.tar.gz";
  hash = "sha256-SsgcXAiE7EvVkHAmqq4WIJt7ds2df3GvWCCUovmPS0M=";
};

preBuild = ''
  tar xvomf $model
'';

@toastal
Copy link
Contributor Author

toastal commented Apr 16, 2024

Alternatively you could use srcs instead of a separate model + src.

While we are at it, we should be using the main repository instead just Microsoft GitHub which is explicitly listed as a mirror, not the primary code forge. Something like

{
  srcs = [
    (fetchzip {
      name = "…";
      urls = [
        "https://gitlab.xiph.org/xiph/rnnoise/-/archive/v${finalAttrs.version}/rnnoise-v${finalAttrs.version}.tar.gz"
        "https://github.com/xiph/rnnoise/archive/v${finalAttrs.version}.tar.gz"
      ];
      hash = "…";
    })
    (fetchurl {
      # model
    })
  ];
}

Referencing both mirrors would be more resilient for when one of these platforms goes down.

@JohnRTitor
Copy link
Contributor

JohnRTitor commented Apr 19, 2024

Made a PR #305348, but still having trouble designing a proper update script.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants