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

Loading noise profile from Carla carxp file does not work #114

Open
bergfried opened this issue Sep 17, 2022 · 7 comments · May be fixed by #119
Open

Loading noise profile from Carla carxp file does not work #114

bergfried opened this issue Sep 17, 2022 · 7 comments · May be fixed by #119
Labels

Comments

@bergfried
Copy link

First of all, thanks for the great work!

I am using Carla on JACK 2 with Noise Repellent on Arch.

  • Tested with Carla 2.5.0, but the specific version does not seem to matter since the issue can be triggered with 2.4.3 from back in around April as well.
  • Noise Repellent 0.2.3 is affected, and 0.2.2 also seems to be affected. Version 0.1.5 is NOT affected. Also tested commit 4dab814 which seems to be affected as well. Could not test with more recent commits due to build issues.
  • Using the non-adaptive, non-stereo variant. In other words, the manual fixed-profile mono variant.
  • The stereo variant might be affected as well, though not tested.
  • Does not apply to the adaptive variant (obviously).

Noise Repellent works fine if Carla learned the noise profile during the very same session. That is, "Residual listen" clearly demonstrates the difference between noise and signal. However, now that it works, try saving the current Carla session in a carxp file and close Carla. The carxp file is an XML file and seems to store the noise profile as well.

Now, open Carla again and load the carxp file. "Residual listen" indicates that the noise profile stored in the carxp file is not respected. Note, however, that other parameters like "Reduction amount" ARE respected. Now, without changing anything, save the very same (seemingly broken) session under a different name using "File" -> "Save as". Both the old and the new carxp file are completely identical, indicating that the noise profile has not been discarded and/or not been overwritten during loading the carxp file.

Since Noise Repellent 0.1.5 is not affected, even when using the same Carla version, Carla is probably handling all the values as expected, indicating that Noise Repellent, for some reason, does not care about the noise profile presented by Carla.

Speculation: Maybe not all identifiers have been properly renamed while progressing from 0.1.5 to 0.2.0 or later.

Please keep up the good work.

@lucianodato
Copy link
Owner

Thanks for reporting! Will look into it soon!

@bergfried
Copy link
Author

Thank you for the quick reply. I just want to point out issue lucianodato/libspecbleach#52 to you in case you come to the conclusion that fixing the bug here involves changes that are closely related to said other issue. (Of course, if it turns out that they are unrelated and can be fixed independently, please keep them separate.)

@bergfried
Copy link
Author

Is there any progress regarding this issue? I consider it quite important because being able to always use the same noise profile is the main reason as to why someone would use this plugin in the first place (as opposed to, let's say, the adaptive variant).

Also, I just found out that Carla produces log lines whenever I open a carxp file with it. Here are the relevant lines:

[carla] lv2_rdf_new("https://github.com/lucianodato/noise-repellent#new") - got unknown port designation 'http://lv2plug.in/ns/lv2core#threshold'
[carla] lv2_rdf_new("https://github.com/lucianodato/noise-repellent#new") - got unknown port designation 'http://lv2plug.in/ns/lv2core#gain'

I hope this helps.

@TekMiikka
Copy link

TekMiikka commented Dec 15, 2022

I have (or had) the same issue. I actually didn't even know this plugin saves the learned noise profile before I saw this post.
It happens on Raspian (Linux raspberrypi 5.15.76-v8+) + Carla 2.5.2 + lv2-dev 1.18.4.

I found the cause for it.
On "LV2_State_Status restore"-function, the "fftsize" and "averagedblocks" get assigned to something else before they reach "specbleach_load_noise_profile"-function.

I didn't find a clear explanation why it happens, but from what I gathered, calling "retrieve" again changes the previous retrieved value to something high (maybe pointers to pointers? or just trash data?). I didn't try dereferencing them twice tho, because I just now thought of that.

Some links I researched:
Here the value just gets reused every time
Here on "State Methods" they use two variables, but they get assigned to somewhere else right away
These last remarks I don't fully understand cause this is my first dwelve into lv2, but are they connected to this?

Anyways, I jerry rigged it with two extra constants and it works (nrepellent.c from line 402):

static LV2_State_Status restore(LV2_Handle instance,
                                LV2_State_Retrieve_Function retrieve,
                                LV2_State_Handle handle, uint32_t flags,
                                const LV2_Feature *const *features) {
  NoiseRepellentPlugin *self = (NoiseRepellentPlugin *)instance;

  size_t size = 0U;
  uint32_t type = 0U;
  uint32_t valflags = 0U;

  const uint32_t *fftsize = (const uint32_t *)retrieve(
      handle, self->state.property_noise_profile_size, &size, &type, &valflags);
  if (fftsize == NULL || type != self->uris.atom_Int) {
    return LV2_STATE_ERR_NO_PROPERTY;
  }
  const uint32_t permanent_fftsize = *fftsize;    // NEW

  const uint32_t *averagedblocks = (const uint32_t *)retrieve(
      handle, self->state.property_averaged_blocks, &size, &type, &valflags);
  if (averagedblocks == NULL || type != self->uris.atom_Int) {
    return LV2_STATE_ERR_NO_PROPERTY;
  }
  const uint32_t permanent_averagedblocks = *averagedblocks;    // NEW

  const void *saved_noise_profile_1 = retrieve(
      handle, self->state.property_noise_profile_1, &size, &type, &valflags);
  if (!saved_noise_profile_1 || size != noise_profile_get_size() ||
      type != self->uris.atom_Vector) {
    return LV2_STATE_ERR_NO_PROPERTY;
  }

  memcpy(self->noise_profile_1, (float *)LV2_ATOM_BODY(saved_noise_profile_1),
         sizeof(float) * self->profile_size);

  specbleach_load_noise_profile(self->lib_instance_1, self->noise_profile_1,
                                permanent_fftsize, permanent_averagedblocks);    // CHANGED

  if (strstr(self->plugin_uri, NOISEREPELLENT_STEREO_URI)) {
    const void *saved_noise_profile_2 = retrieve(
        handle, self->state.property_noise_profile_2, &size, &type, &valflags);
    if (!saved_noise_profile_2 || size != noise_profile_get_size() ||
        type != self->uris.atom_Vector) {
      return LV2_STATE_ERR_NO_PROPERTY;
    }

    memcpy(self->noise_profile_2, (float *)LV2_ATOM_BODY(saved_noise_profile_2),
           sizeof(float) * self->profile_size);

    specbleach_load_noise_profile(self->lib_instance_2, self->noise_profile_2,
                                  permanent_fftsize, permanent_averagedblocks);    // CHANGED
  }

  return LV2_STATE_SUCCESS;
}

I didn't wanna create a pull request cause I don't know if this is proper way to deal with it.

If you want to know more about the values, on one test run "fftsize" printed out as 1105 when first retrived and 128445920 after the "averagedblocks" retrieve. And "averagedblocks" printed out at first retrieve as 2354 and after the "saved_noise_profile_1" retrieve it printed out as 128444992.

@bergfried
Copy link
Author

Thank you so much for fixing this issue, @TekMiikka. I simply replaced the entire restore function in plugins/nrepellent.c with what you have posted, and it works like a charm!

Let us hope that the issue will be fixed upstream so other users can enjoy it as well.

@OracleToes
Copy link

I tried your solution @TekMiikka and it seems like it almost worked. It definitely loaded something, but it only seemed to be doing something with one side of the audio. At least it's progress

@SnipeXandrej
Copy link

SnipeXandrej commented Aug 28, 2023

Here's @TekMiikka patch with my horrible hackiness applied...

I just made it to load the saved_noise_profile_1 on the noise_profile_2 thingy and now it seems to "work" when using the stereo version

diff --git a/plugins/nrepellent.c b/plugins/nrepellent.c
index d5743bf..ced680a 100644
--- a/plugins/nrepellent.c
+++ b/plugins/nrepellent.c
@@ -414,12 +414,14 @@ static LV2_State_Status restore(LV2_Handle instance,
   if (fftsize == NULL || type != self->uris.atom_Int) {
     return LV2_STATE_ERR_NO_PROPERTY;
   }
+  const uint32_t permanent_fftsize = *fftsize;
 
   const uint32_t *averagedblocks = (const uint32_t *)retrieve(
       handle, self->state.property_averaged_blocks, &size, &type, &valflags);
   if (averagedblocks == NULL || type != self->uris.atom_Int) {
     return LV2_STATE_ERR_NO_PROPERTY;
   }
+  const uint32_t permanent_averagedblocks = *averagedblocks;
 
   const void *saved_noise_profile_1 = retrieve(
       handle, self->state.property_noise_profile_1, &size, &type, &valflags);
@@ -432,21 +434,21 @@ static LV2_State_Status restore(LV2_Handle instance,
          sizeof(float) * self->profile_size);
 
   specbleach_load_noise_profile(self->lib_instance_1, self->noise_profile_1,
-                                *fftsize, *averagedblocks);
+                                permanent_fftsize, permanent_averagedblocks);
 
   if (strstr(self->plugin_uri, NOISEREPELLENT_STEREO_URI)) {
-    const void *saved_noise_profile_2 = retrieve(
-        handle, self->state.property_noise_profile_2, &size, &type, &valflags);
-    if (!saved_noise_profile_2 || size != noise_profile_get_size() ||
+    const void *saved_noise_profile_1 = retrieve(
+        handle, self->state.property_noise_profile_1, &size, &type, &valflags);
+    if (!saved_noise_profile_1 || size != noise_profile_get_size() ||
         type != self->uris.atom_Vector) {
       return LV2_STATE_ERR_NO_PROPERTY;
     }
 
-    memcpy(self->noise_profile_2, (float *)LV2_ATOM_BODY(saved_noise_profile_2),
+    memcpy(self->noise_profile_2, (float *)LV2_ATOM_BODY(saved_noise_profile_1),
            sizeof(float) * self->profile_size);
 
     specbleach_load_noise_profile(self->lib_instance_2, self->noise_profile_2,
-                                  *fftsize, *averagedblocks);
+                                  permanent_fftsize, permanent_averagedblocks);
   }
 
   return LV2_STATE_SUCCESS;

orivej added a commit to orivej/noise-repellent that referenced this issue Oct 12, 2023
@orivej orivej linked a pull request Oct 12, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants