Add wav-reverberate.cc and modify the scripts using it. #76

tomkocse · 2015-08-17T07:35:55Z

Please comment.

vijayaditya · 2015-08-17T07:50:18Z

egs/aspire/s5/local/multi_condition/reverberate_data_dir.sh

 $src_dir/wav.scp $log_dir/corrupted_${random_seed}.list $impnoise_dir \
-$log_dir/corrupt_wavs.${random_seed}.list > $log_dir/num_corruption_jobs || exit 1;
+$dest_dir/wav.scp || exit 1;



remove the "wc" command. At least print a message to say what the number means.

vijayaditya · 2015-08-17T08:13:19Z

A side note:
We need to test out the speed of using wav-reverberate vs. extract-reverb-segments. A simple test for this would be to compare running
wav-reverberate on the full signal followed by extract-segments multiple times.
vs
running wav-reverberate on random signals of the size of the segments (to emulate running extract-reverb-segments)

You can use sox to generate random signals.

vijayaditya · 2015-08-17T20:05:52Z

src/featbin/wav-reverberate.cc

+  int xdim = x.Dim();
+  int M = h.Dim();
+
+  KALDI_LOG << "M is " << M;


Make the log output more descriptive.
e.g. "Length of the filter is " << M;

vijayaditya · 2015-08-17T22:39:46Z

Break the code in main function into smaller parts. Move everything other than data reading and writing to separate functions.
Move early reverb energy estimation to separate function called
ComputeEarlyReverbEnergy(const Vector & room_impulse_response, const Vector& signal, BaseFloat * early_reverb_energy )

vijayaditya · 2015-08-17T22:41:28Z

src/featbin/wav-reverberate.cc

+      RepeatedlyAdd(src, noise);
+    }
+
+    float src_max2 = src.Min();    // for checking the negative max


move this code to a separate function called

MaxAbsolute(const Vector & vector, BaseFloat * max_value)

kkm000 · 2015-08-18T09:45:50Z

I may be misunderstanding what the code does, but just want to point FWIW that sox supports both FIR convolution (fir effect) and adding noise (although the latter is a bit, ehm, convoluted).

vijayaditya · 2015-08-18T10:35:40Z

The code also computes the scaling coefficient for noise signal, this involves
computing the early reverberation component of the reverberated signal .
This makes a sox based implementation a bit circuitous.

Vijay

On Tue, Aug 18, 2015 at 5:45 AM, Kirill Katsnelson <notifications@github.com

wrote:

I may be misunderstanding what the code does, but just want to point FWIW
that sox supports both FIR convolution (fir effect) and adding noise
(although the latter is a bit, ehm, convoluted
http://linguistics.berkeley.edu/plab/guestwiki/index.php?title=Sox_in_phonetic_research#Add_noise_to_an_audio_file
).

—
Reply to this email directly or view it on GitHub
#76 (comment).

danpovey · 2015-08-20T00:14:29Z

Vijay, let me know when you think this is ready to merge.

tomkocse · 2015-08-20T09:33:55Z

I have made the changes according to all your comments.
The multi channel output mode is added.
The test mode (with option --test-mode) is added by comparing block-based convolution and full signal convolution.

danpovey · 2015-08-20T18:32:57Z

Thanks. Vijay, please confirm when you think it's ready to commit, and
I'll quickly look over the code again before merging (if I find no issues).
Dan

On Thu, Aug 20, 2015 at 2:33 AM, tomkocse notifications@github.com wrote:

I have made the changes according to all your comments.
The multi channel output mode is added.
The test mode (with option --test-mode) is added by comparing block-based
convolution and full signal convolution.

—
Reply to this email directly or view it on GitHub
#76 (comment).

vijayaditya · 2015-08-20T18:36:08Z

I am going through it now, will add comments soon.

Vijay

On Thu, Aug 20, 2015 at 2:32 PM, Daniel Povey notifications@github.com
wrote:

Thanks. Vijay, please confirm when you think it's ready to commit, and
I'll quickly look over the code again before merging (if I find no issues).
Dan

On Thu, Aug 20, 2015 at 2:33 AM, tomkocse notifications@github.com
wrote:

I have made the changes according to all your comments.
The multi channel output mode is added.
The test mode (with option --test-mode) is added by comparing block-based
convolution and full signal convolution.

—
Reply to this email directly or view it on GitHub
#76 (comment).

—
Reply to this email directly or view it on GitHub
#76 (comment).

vijayaditya · 2015-08-20T20:13:35Z

src/featbin/wav-reverberate.cc

+
+    srfft_->Compute(signal_block_padded.Data(), true);
+
+    for (int i = 0; i < fft_length / 2; i++) {


Separate out the for loop into a function like ElementwiseProductOfFft. Same in the ConvolveSignals block above.

vijayaditya · 2015-08-27T06:23:14Z

src/featbin/wav-reverberate.cc

+  // compute the energy
+  return VecVec(early_reverb, early_reverb) / early_reverb.Dim();
+}
+


When defining a function, parameter order is: inputs, then outputs.

Add a comment describing what the function does above it.

tomkocse · 2015-09-01T02:28:30Z

Added comments to the code.

kkm000 · 2015-09-02T01:12:19Z

src/featbin/wav-reverberate.cc

+   the room impulse response). This function returns the energy in 
+   this early reverberation component of the signal. 
+   The input parameters to this function are the room impulse response, the signal
+   and their sampling freqency respectively.


s/freqency/frequency/

danpovey · 2015-09-02T18:44:30Z

Vijay, is this ready for me to review?

vijayaditya · 2015-09-02T18:53:56Z

Yes, its ready except for one suggested change to function signature.

Vijay

On Wed, Sep 2, 2015 at 2:44 PM, Daniel Povey notifications@github.com
wrote:

Vijay, is this ready for me to review?

—
Reply to this email directly or view it on GitHub
#76 (comment).

danpovey · 2015-09-02T18:55:32Z

I'll wait.

On Wed, Sep 2, 2015 at 2:53 PM, Vijayaditya Peddinti <
notifications@github.com> wrote:

Yes, its ready except for one suggested change to function signature.

Vijay

On Wed, Sep 2, 2015 at 2:44 PM, Daniel Povey notifications@github.com
wrote:

Vijay, is this ready for me to review?

—
Reply to this email directly or view it on GitHub
#76 (comment).

—
Reply to this email directly or view it on GitHub
#76 (comment).

tomkocse · 2015-09-03T03:09:23Z

Function signature changed.

vijayaditya · 2015-09-03T03:11:26Z

src/feat/signal.h

+/*
+   This function implements convolution of two signals.
+   However this should be an inefficient version of BlockConvolveSignals()
+   as it process the entire signal with a single FFT.


process -> processes

danpovey · 2015-09-03T03:14:06Z

Can you please clean up the commit history to have a single commit with a suitable message?
Most likely you can do
git rebase -i HEAD~10
and you'll see something like this:

pick 48dc4cc nnet3: fix to how objective function is normalized in nnet3-show-progress
pick f569d0c Fixing the segfault issue in lattice-oracle.cc: used lattice composition instead of GetOracleLattice(); also fixed various style issues that I noticed.
pick 1df55e7 Correcting minor bug in ASpIRE recipe, doesn't affect results
pick d1aceed fixing the install script to fail correctly

Rebase `3e76aa5`..`85aa227` onto `3e76aa5`

Commands:

p, pick = use commit

r, reword = use commit, but edit the commit message

e, edit = use commit, but stop for amending

s, squash = use commit, but meld into previous commit

f, fixup = like "squash", but discard this commit's log message

x, exec = run command (the rest of the line) using shell

These lines can be re-ordered; they are executed from top to bottom.

If you remove a line here THAT COMMIT WILL BE LOST.

However, if you remove everything, the rebase will be aborted.

and you edit the first line's commit message to the overall commit message you want for the whole thing, and the other ones from 'pick' to 'f' (or 'fix'), and their commit messages will be ignored.
e.g.

pick 48dc4cc nnet3: fix to how objective function is normalized in nnet3-show-progress
f f569d0c Fixing the segfault issue in lattice-oracle.cc: used lattice composition instead of GetOracleLattice(); also fixed various style issues that I noticed.
f 1df55e7 Correcting minor bug in ASpIRE recipe, doesn't affect results
f d1aceed fixing the install script to fail correctly

Then push it (you may need an option like --no-ff) to force the push because it's not fast forward).

danpovey · 2015-09-03T03:21:20Z

src/feat/signal.cc

+namespace kaldi {
+
+void ElementwiseProductOfFft(const Vector<BaseFloat> &a, Vector<BaseFloat> *b) {
+  for (int i = 0; i < a.Dim() / 2; i++) {


It's good practice to use a temporary
e.g.
int32 num_fft_bins = a.Dim() / 2;
and do i < num_fft_bins, to avoid the function call being repeated.
Also, inside the loop, please replace your code with:
// do complex multiplication b _= a.
ComplexMul(a(2_i), a(2_i + 1), &(b(2_i)), &(b(2*i + 1)));
which will be clearer.

tomkocse · 2015-09-04T04:55:49Z

The test for non-FFT-based convolution is added.
Other minor issues are handled.
I just rebased the commit history and pushed with option --force.

danpovey · 2015-09-04T04:58:07Z

OK, I'll merge now. Thanks for putting up with the process!

Add wav-reverberate.cc and modify the scripts using it.

update WSJ result

vijayaditya reviewed Aug 17, 2015
View reviewed changes

vijayaditya reviewed Aug 20, 2015
View reviewed changes

vijayaditya reviewed Aug 27, 2015
View reviewed changes

kkm000 reviewed Sep 2, 2015
View reviewed changes

vijayaditya reviewed Sep 3, 2015
View reviewed changes

danpovey reviewed Sep 3, 2015
View reviewed changes

Add wav-reverberate.cc and modify the scripts using it.

759114e

tomkocse force-pushed the wav-reverberate branch from c26da38 to 759114e Compare September 4, 2015 04:50

danpovey added a commit that referenced this pull request Sep 4, 2015

Merge pull request #76 from tomkocse/wav-reverberate

5682f61

Add wav-reverberate.cc and modify the scripts using it.

danpovey merged commit 5682f61 into kaldi-asr:master Sep 4, 2015

hainan-xv pushed a commit to hainan-xv/kaldi that referenced this pull request Jul 24, 2018

Merge pull request kaldi-asr#76 from sw005320/update_wsj

6c1be44

update WSJ result

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add wav-reverberate.cc and modify the scripts using it. #76

Add wav-reverberate.cc and modify the scripts using it. #76

tomkocse commented Aug 17, 2015

vijayaditya Aug 17, 2015

vijayaditya commented Aug 17, 2015

vijayaditya Aug 17, 2015

vijayaditya commented Aug 17, 2015

vijayaditya Aug 17, 2015

kkm000 commented Aug 18, 2015

vijayaditya commented Aug 18, 2015

danpovey commented Aug 20, 2015

tomkocse commented Aug 20, 2015

danpovey commented Aug 20, 2015

vijayaditya commented Aug 20, 2015

vijayaditya Aug 20, 2015

vijayaditya Aug 27, 2015

tomkocse commented Sep 1, 2015

kkm000 Sep 2, 2015

danpovey commented Sep 2, 2015

vijayaditya commented Sep 2, 2015

danpovey commented Sep 2, 2015

tomkocse commented Sep 3, 2015

vijayaditya Sep 3, 2015

danpovey commented Sep 3, 2015

danpovey Sep 3, 2015

tomkocse commented Sep 4, 2015

danpovey commented Sep 4, 2015


		srfft_->Compute(signal_block_padded.Data(), true);

		for (int i = 0; i < fft_length / 2; i++) {

Add wav-reverberate.cc and modify the scripts using it. #76

Add wav-reverberate.cc and modify the scripts using it. #76

Conversation

tomkocse commented Aug 17, 2015

Choose a reason for hiding this comment

vijayaditya commented Aug 17, 2015

Choose a reason for hiding this comment

vijayaditya commented Aug 17, 2015

Choose a reason for hiding this comment

kkm000 commented Aug 18, 2015

vijayaditya commented Aug 18, 2015

danpovey commented Aug 20, 2015

tomkocse commented Aug 20, 2015

danpovey commented Aug 20, 2015

vijayaditya commented Aug 20, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomkocse commented Sep 1, 2015

Choose a reason for hiding this comment

danpovey commented Sep 2, 2015

vijayaditya commented Sep 2, 2015

danpovey commented Sep 2, 2015

tomkocse commented Sep 3, 2015

Choose a reason for hiding this comment

danpovey commented Sep 3, 2015

Rebase 3e76aa5..85aa227 onto 3e76aa5

Commands:

p, pick = use commit

r, reword = use commit, but edit the commit message

e, edit = use commit, but stop for amending

s, squash = use commit, but meld into previous commit

f, fixup = like "squash", but discard this commit's log message

x, exec = run command (the rest of the line) using shell

These lines can be re-ordered; they are executed from top to bottom.

If you remove a line here THAT COMMIT WILL BE LOST.

However, if you remove everything, the rebase will be aborted.

Choose a reason for hiding this comment

tomkocse commented Sep 4, 2015

danpovey commented Sep 4, 2015

Rebase `3e76aa5`..`85aa227` onto `3e76aa5`