[vlc-devel] [PATCH] contrib: rnnoise: prefix common symbols

Alexandre Janniaux ajanni at videolabs.io
Tue Oct 20 10:08:44 CEST 2020


Hi,

On Tue, Oct 20, 2020 at 09:07:08AM +0300, Rémi Denis-Courmont wrote:
> Hi,
>
> It's perfectly fine to have same named symbols in different libraries. If you get an error, there must be something wrong with link flags somewhere.

As long as we ship static framework on iOS, it's unavoidable, even
if using partial linking. The reasons the behaviour is different has
been explained in #25156. Note that though partial linking would fix
#25156, the case here is different because the duplicated symbols are
in different libraries and thus unaffected by partial linking.

We could also note that while linkers don't complain when symbols are
duplicated in different libraries, it won't mean that the resulting
object will behave correctly since one symbol might be called from the
first library while it expected it from the second, and behaviour could
change or just be a completely different ABI.

Why is there conflict in the first place? Did rnnoise copy function from
opus for the fft without renaming them? The symbol clash can probably be
fixed upstream then, with the correct prefixing?

Regards,
--
Alexandre Janniaux
Videolabs

> Le 20 octobre 2020 00:53:09 GMT+03:00, Tristan Matthews <tmatth at videolan.org> a écrit :
> >This avoids clashing with symbols of the same name in libopus
> >---
> > contrib/src/rnnoise/dupe-symbols.patch | 134 +++++++++++++++++++++++++
> > contrib/src/rnnoise/rules.mak          |   5 +-
> > 2 files changed, 137 insertions(+), 2 deletions(-)
> > create mode 100644 contrib/src/rnnoise/dupe-symbols.patch
> >
> >diff --git a/contrib/src/rnnoise/dupe-symbols.patch
> >b/contrib/src/rnnoise/dupe-symbols.patch
> >new file mode 100644
> >index 0000000000..f564511990
> >--- /dev/null
> >+++ b/contrib/src/rnnoise/dupe-symbols.patch
> >@@ -0,0 +1,134 @@
> >+diff -ruw rnnoise-orig/src/kiss_fft.c rnnoise/src/kiss_fft.c
> >+--- rnnoise-orig/src/kiss_fft.c	2020-10-18 22:52:44.956998651 -0400
> >++++ rnnoise/src/kiss_fft.c	2020-10-18 22:57:33.355740996 -0400
> >+@@ -515,7 +515,7 @@
> >+
> >+ #endif /* CUSTOM_MODES */
> >+
> >+-void opus_fft_impl(const kiss_fft_state *st,kiss_fft_cpx *fout)
> >++void rnn_opus_fft_impl(const kiss_fft_state *st,kiss_fft_cpx *fout)
> >+ {
> >+     int m2, m;
> >+     int p;
> >+@@ -563,7 +563,7 @@
> >+     }
> >+ }
> >+
> >+-void opus_fft_c(const kiss_fft_state *st,const kiss_fft_cpx
> >*fin,kiss_fft_cpx *fout)
> >++void rnn_opus_fft_c(const kiss_fft_state *st,const kiss_fft_cpx
> >*fin,kiss_fft_cpx *fout)
> >+ {
> >+    int i;
> >+    opus_val16 scale;
> >+@@ -582,11 +582,11 @@
> >+       fout[st->bitrev[i]].r = SHR32(MULT16_32_Q16(scale, x.r),
> >scale_shift);
> >+       fout[st->bitrev[i]].i = SHR32(MULT16_32_Q16(scale, x.i),
> >scale_shift);
> >+    }
> >+-   opus_fft_impl(st, fout);
> >++   rnn_opus_fft_impl(st, fout);
> >+ }
> >+
> >+
> >+-void opus_ifft_c(const kiss_fft_state *st,const kiss_fft_cpx
> >*fin,kiss_fft_cpx *fout)
> >++void rnn_opus_ifft_c(const kiss_fft_state *st,const kiss_fft_cpx
> >*fin,kiss_fft_cpx *fout)
> >+ {
> >+    int i;
> >+    celt_assert2 (fin != fout, "In-place FFT not supported");
> >+@@ -595,7 +595,7 @@
> >+       fout[st->bitrev[i]] = fin[i];
> >+    for (i=0;i<st->nfft;i++)
> >+       fout[i].i = -fout[i].i;
> >+-   opus_fft_impl(st, fout);
> >++   rnn_opus_fft_impl(st, fout);
> >+    for (i=0;i<st->nfft;i++)
> >+       fout[i].i = -fout[i].i;
> >+ }
> >+diff -ruw rnnoise-orig/src/kiss_fft.h rnnoise/src/kiss_fft.h
> >+--- rnnoise-orig/src/kiss_fft.h	2020-10-18 22:52:44.956998651 -0400
> >++++ rnnoise/src/kiss_fft.h	2020-10-18 22:57:40.283710784 -0400
> >+@@ -142,10 +142,10 @@
> >+  * Note that each element is complex and can be accessed like
> >+     f[k].r and f[k].i
> >+  * */
> >+-void opus_fft_c(const kiss_fft_state *cfg,const kiss_fft_cpx
> >*fin,kiss_fft_cpx *fout);
> >+-void opus_ifft_c(const kiss_fft_state *cfg,const kiss_fft_cpx
> >*fin,kiss_fft_cpx *fout);
> >++void rnn_opus_fft_c(const kiss_fft_state *cfg,const kiss_fft_cpx
> >*fin,kiss_fft_cpx *fout);
> >++void rnn_opus_ifft_c(const kiss_fft_state *cfg,const kiss_fft_cpx
> >*fin,kiss_fft_cpx *fout);
> >+
> >+-void opus_fft_impl(const kiss_fft_state *st,kiss_fft_cpx *fout);
> >++void rnn_opus_fft_impl(const kiss_fft_state *st,kiss_fft_cpx *fout);
> >+ void opus_ifft_impl(const kiss_fft_state *st,kiss_fft_cpx *fout);
> >+
> >+ void opus_fft_free(const kiss_fft_state *cfg, int arch);
> >+@@ -188,10 +188,10 @@
> >+          ((void)(arch), opus_fft_free_arch_c(_st))
> >+
> >+ #define opus_fft(_cfg, _fin, _fout, arch) \
> >+-         ((void)(arch), opus_fft_c(_cfg, _fin, _fout))
> >++         ((void)(arch), rnn_opus_fft_c(_cfg, _fin, _fout))
> >+
> >+ #define opus_ifft(_cfg, _fin, _fout, arch) \
> >+-         ((void)(arch), opus_ifft_c(_cfg, _fin, _fout))
> >++         ((void)(arch), rnn_opus_ifft_c(_cfg, _fin, _fout))
> >+
> >+ #endif /* end if defined(OPUS_HAVE_RTCD) && (defined(HAVE_ARM_NE10))
> >*/
> >+ #endif /* end if !defined(OVERRIDE_OPUS_FFT) */
> >+diff -ruw rnnoise-orig/src/rnn.c rnnoise/src/rnn.c
> >+--- rnnoise-orig/src/rnn.c	2020-10-18 22:52:44.956998651 -0400
> >++++ rnnoise/src/rnn.c	2020-10-18 22:55:56.492163401 -0400
> >+@@ -76,7 +76,7 @@
> >+    return x < 0 ? 0 : x;
> >+ }
> >+
> >+-void compute_dense(const DenseLayer *layer, float *output, const
> >float *input)
> >++void rnn_compute_dense(const DenseLayer *layer, float *output, const
> >float *input)
> >+ {
> >+    int i, j;
> >+    int N, M;
> >+@@ -106,7 +106,7 @@
> >+    }
> >+ }
> >+
> >+-void compute_gru(const GRULayer *gru, float *state, const float
> >*input)
> >++void rnn_compute_gru(const GRULayer *gru, float *state, const float
> >*input)
> >+ {
> >+    int i, j;
> >+    int N, M;
> >+@@ -162,17 +162,17 @@
> >+   float dense_out[MAX_NEURONS];
> >+   float noise_input[MAX_NEURONS*3];
> >+   float denoise_input[MAX_NEURONS*3];
> >+-  compute_dense(rnn->model->input_dense, dense_out, input);
> >+-  compute_gru(rnn->model->vad_gru, rnn->vad_gru_state, dense_out);
> >+-  compute_dense(rnn->model->vad_output, vad, rnn->vad_gru_state);
> >++  rnn_compute_dense(rnn->model->input_dense, dense_out, input);
> >++  rnn_compute_gru(rnn->model->vad_gru, rnn->vad_gru_state,
> >dense_out);
> >++  rnn_compute_dense(rnn->model->vad_output, vad, rnn->vad_gru_state);
> >+   for (i=0;i<rnn->model->input_dense_size;i++) noise_input[i] =
> >dense_out[i];
> >+   for (i=0;i<rnn->model->vad_gru_size;i++)
> >noise_input[i+rnn->model->input_dense_size] = rnn->vad_gru_state[i];
> >+   for (i=0;i<INPUT_SIZE;i++)
> >noise_input[i+rnn->model->input_dense_size+rnn->model->vad_gru_size] =
> >input[i];
> >+-  compute_gru(rnn->model->noise_gru, rnn->noise_gru_state,
> >noise_input);
> >++  rnn_compute_gru(rnn->model->noise_gru, rnn->noise_gru_state,
> >noise_input);
> >+
> >+   for (i=0;i<rnn->model->vad_gru_size;i++) denoise_input[i] =
> >rnn->vad_gru_state[i];
> >+   for (i=0;i<rnn->model->noise_gru_size;i++)
> >denoise_input[i+rnn->model->vad_gru_size] = rnn->noise_gru_state[i];
> >+   for (i=0;i<INPUT_SIZE;i++)
> >denoise_input[i+rnn->model->vad_gru_size+rnn->model->noise_gru_size] =
> >input[i];
> >+-  compute_gru(rnn->model->denoise_gru, rnn->denoise_gru_state,
> >denoise_input);
> >+-  compute_dense(rnn->model->denoise_output, gains,
> >rnn->denoise_gru_state);
> >++  rnn_compute_gru(rnn->model->denoise_gru, rnn->denoise_gru_state,
> >denoise_input);
> >++  rnn_compute_dense(rnn->model->denoise_output, gains,
> >rnn->denoise_gru_state);
> >+ }
> >+diff -ruw rnnoise-orig/src/rnn.h rnnoise/src/rnn.h
> >+--- rnnoise-orig/src/rnn.h	2020-10-18 22:52:44.956998651 -0400
> >++++ rnnoise/src/rnn.h	2020-10-18 22:55:59.892148575 -0400
> >+@@ -60,9 +60,9 @@
> >+
> >+ typedef struct RNNState RNNState;
> >+
> >+-void compute_dense(const DenseLayer *layer, float *output, const
> >float *input);
> >++void rnn_compute_dense(const DenseLayer *layer, float *output, const
> >float *input);
> >+
> >+-void compute_gru(const GRULayer *gru, float *state, const float
> >*input);
> >++void rnn_compute_gru(const GRULayer *gru, float *state, const float
> >*input);
> >+
> >+ void compute_rnn(RNNState *rnn, float *gains, float *vad, const float
> >*input);
> >+
> >diff --git a/contrib/src/rnnoise/rules.mak
> >b/contrib/src/rnnoise/rules.mak
> >index f6ded7ecda..3547322f90 100644
> >--- a/contrib/src/rnnoise/rules.mak
> >+++ b/contrib/src/rnnoise/rules.mak
> >@@ -3,9 +3,7 @@
> > RNNOISE_GITURL := http://github.com/xiph/rnnoise.git
> > RNNOISE_GITHASH := 90ec41ef659fd82cfec2103e9bb7fc235e9ea66c
> >
> >-ifndef HAVE_ANDROID
> > PKGS += rnnoise
> >-endif
> >
> > ifeq ($(call need_pkg,"rnnoise"),)
> > PKGS_FOUND += rnnoise
> >@@ -20,6 +18,9 @@ $(TARBALLS)/rnnoise-$(RNNOISE_GITHASH).tar.xz:
> >
> > rnnoise: rnnoise-$(RNNOISE_GITHASH).tar.xz .sum-rnnoise
> > 	$(UNPACK)
> >+ifdef HAVE_ANDROID
> >+	$(APPLY) $(SRC)/rnnoise/dupe-symbols.patch
> >+endif
> > 	$(MOVE)
> >
> > .rnnoise: rnnoise
> >--
> >2.25.1
> >
> >_______________________________________________
> >vlc-devel mailing list
> >To unsubscribe or modify your subscription options:
> >https://mailman.videolan.org/listinfo/vlc-devel
>
> --
> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.

> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel


More information about the vlc-devel mailing list