Add Torchscript sox effects #760

mthrok · 2020-07-01T21:01:06Z

This PR add new sox effects functions torchaudio.sox_effects.apply_effects_tensor and torchaudio.sox_effects.apply_effects_file, which applies sox effects to Tensor object and file object respectively.

This new functions are written from scratch to take advantage of Torchscript, and tested on various effects by comparing against the results from sox command. The existing torchaudio.sox_effects.SoxEffectsChain does not have this and it does not work correctly on certain format (see #771 )

Also this PR adds torchaudio.utils.sox_utils module, which allows users to set verbosity, multi-threading option, buffer size and get the list of supported effects/formats.

Off topic: @cpuhrsch I was adding some tests on data loader with num_workers > 1. This test example applies speed perturbation, which alters the length of input Tensor. It would be interesting if we can combine nestedtensor there.
https://github.com/mthrok/audio/blob/sox-effects-chain/test/sox_effect/test_dataset.py#L17-L63

codecov · 2020-07-02T01:37:21Z

Codecov Report

Merging #760 into master will increase coverage by 0.13%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #760      +/-   ##
==========================================
+ Coverage   89.53%   89.66%   +0.13%     
==========================================
  Files          32       34       +2     
  Lines        2617     2652      +35     
==========================================
+ Hits         2343     2378      +35     
  Misses        274      274

Impacted Files	Coverage Δ
torchaudio/__init__.py	`73.33% <ø> (ø)`
torchaudio/sox_effects/__init__.py	`100.00% <ø> (ø)`
torchaudio/sox_effects/sox_effects.py	`95.12% <100.00%> (+0.67%)`	⬆️
torchaudio/utils/__init__.py	`100.00% <100.00%> (ø)`
torchaudio/utils/sox_utils.py	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 131e48b...a17fffb. Read the comment docs.

cpuhrsch · 2020-07-14T16:43:55Z

Is it possible to split this up a bit more into smaller PRs or would that result in unreasonable amount of work?

mthrok · 2020-07-14T19:57:53Z

Is it possible to split this up a bit more into smaller PRs or would that result in unreasonable amount of work?

@cpuhrsch I spliced the PR into logical commits. I can still make them separate PRs if necessary, but this way, we do not need to worry about merge conflict between them. Let me know if you feel separate PRs is better.

The commits are basically about

70763f4 Add sox utilities. Add functions that can control verbosity, multithreading, buffer size of sox effects, list format/effect functions.
7b7b4d2 Make init/shutdown_sox_effects functions thread safe by adding lock/guard mechanism
b3d4f38 Add sox_effects implementation.
4ef41ad Add tests for sox_effects implementation.

vincentqb · 2020-07-14T22:27:19Z

btw, could you add a example code snippet to the description to showcase how to use the new interface? with old-new syntax comparison?

mthrok · 2020-07-15T18:38:16Z

btw, could you add a example code snippet to the description to showcase how to use the new interface? with old-new syntax comparison?

@vincentqb Updated the docstring and added examples ~~c4dafdb~~ a17fffb.

I will add migration message in the next PR in which deprecation messages are added to the existing SoxEffectsChain class. I do not think migration step should belong to the docsrtings of the new functions.

vincentqb · 2020-07-15T20:21:57Z

btw, could you add a example code snippet to the description to showcase how to use the new interface? with old-new syntax comparison?

@vincentqb Updated the docstring and added examples ~~c4dafdb~~ a17fffb.

Cool!

I will add migration message in the next PR in which deprecation messages are added to the existing SoxEffectsChain class. I do not think migration step should belong to the docsrtings of the new functions.

That's even better. I initially meant to simply add instructions in the description of this PR as a first step.

vincentqb · 2020-07-15T21:51:32Z

cc @eugene-kharitonov for interest in applying sox effects to tensors directly without using files :)

vincentqb · 2020-07-15T21:52:41Z

This PR add new sox effects functions torchaudio.sox_effects.apply_effects_tensor and torchaudio.sox_effects.apply_effects_file, which applies sox effects to Tensor object and file object respectively.

Is there value in having an overloaded function that switched between the two depending on input?

vincentqb · 2020-07-15T21:56:23Z

torchaudio/utils/sox_utils.py

+
+
+@_mod_utils.requires_module('torchaudio._torchaudio')
+def set_buffer_size(buffer_size: int):


When I see functions like these, I understand that there are global parameters behind the scene to manage the sox_effects_chain. Is that the case? Is there a way here to have many chains with different such settings?

When I see functions like these, I understand that there are global parameters behind the scene to manage the sox_effects_chain. Is that the case?

Yes

Is there a way here to have many chains with different such settings?

Nope

vincentqb · 2020-07-15T21:57:45Z

torchaudio/csrc/sox_effects.cpp

 namespace torchaudio {
 namespace sox_effects {

 namespace {

 enum SoxEffectsResourceState { NotInitialized, Initialized, ShutDown };
 SoxEffectsResourceState SOX_RESOURCE_STATE = NotInitialized;
+std::mutex SOX_RESOUCE_STATE_MUTEX;


So the code is thread safe through this global lock, right?

So the code is thread safe through this global lock, right?

Yes.

vincentqb · 2020-07-15T22:04:53Z

torchaudio/csrc/sox_io.cpp

@@ -125,14 +125,12 @@ void save_audio_file(
    const c10::intrusive_ptr<TensorSignal>& signal,
    const double compression) {
  const auto tensor = signal->getTensor();
-  const auto sample_rate = signal->getSampleRate();


nit: what do people do to get sample rate then?

vincentqb

LGTM with a few clarifying questions. Thanks for grouping the changes into logical commits. Separate PR would have made it easier to associate descriptions with each changes though, at least for the reader :)

mthrok · 2020-07-16T02:47:23Z

This PR add new sox effects functions torchaudio.sox_effects.apply_effects_tensor and torchaudio.sox_effects.apply_effects_file, which applies sox effects to Tensor object and file object respectively.

Is there value in having an overloaded function that switched between the two depending on input?

No.

mthrok · 2020-07-16T02:48:41Z

Thanks!

Update Dynamic Quant BERT Tutorial 4

mthrok force-pushed the sox-effects-chain branch 6 times, most recently from 96cbfb4 to a53314e Compare July 2, 2020 01:20

mthrok force-pushed the sox-effects-chain branch 15 times, most recently from 90098e6 to 75c8b51 Compare July 8, 2020 17:42

mthrok mentioned this pull request Jul 8, 2020

🐛 Bug: SoxEffectsChain returns wrong result for float32 wav #771

Closed

mthrok force-pushed the sox-effects-chain branch from 75c8b51 to 6ad1e13 Compare July 8, 2020 18:18

mthrok marked this pull request as ready for review July 8, 2020 18:42

mthrok requested a review from vincentqb July 8, 2020 18:42

This was referenced Jul 13, 2020

Issue 764: Convert mp3 to wav #773

Merged

Merge sox effect and sox_io C++ implementation #779

Merged

mthrok force-pushed the sox-effects-chain branch from 6ad1e13 to 78858a4 Compare July 14, 2020 17:04

mthrok force-pushed the sox-effects-chain branch 4 times, most recently from ea0b9e0 to b2ba4dc Compare July 14, 2020 19:52

mthrok added 4 commits July 14, 2020 20:01

Add sox_utils module

70763f4

Make init/shutdown thread safe

7b7b4d2

Add sox effects implementation

b3d4f38

Add test for sox effects

4ef41ad

mthrok force-pushed the sox-effects-chain branch from b2ba4dc to 4ef41ad Compare July 14, 2020 20:02

mthrok force-pushed the sox-effects-chain branch from e5c716d to 2d41e38 Compare July 15, 2020 18:39

mthrok mentioned this pull request Jul 15, 2020

Issue 764: Switch Pitch Detection Test to use On the Fly Generation instead of file. #783

Merged

mthrok added 2 commits July 15, 2020 20:48

Tweak csrc

b3e5b5f

Update docstrings and add examples

a17fffb

mthrok force-pushed the sox-effects-chain branch from f55a67b to a17fffb Compare July 15, 2020 20:48

vincentqb reviewed Jul 15, 2020

View reviewed changes

vincentqb approved these changes Jul 15, 2020

View reviewed changes

mthrok merged commit 60a8e23 into pytorch:master Jul 16, 2020

mthrok deleted the sox-effects-chain branch July 16, 2020 02:48

vincentqb mentioned this pull request Oct 19, 2020

Resolve setup issue of macOS CI Unittest on release/0.7 #946

Merged

mthrok pushed a commit to mthrok/audio that referenced this pull request Feb 26, 2021

Merge pull request pytorch#760 from jianyuh/jlin27-quant-tutorials

a8eab2d

Update Dynamic Quant BERT Tutorial 4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Torchscript sox effects #760

Add Torchscript sox effects #760

mthrok commented Jul 1, 2020 •

edited

codecov bot commented Jul 2, 2020 •

edited

cpuhrsch commented Jul 14, 2020

mthrok commented Jul 14, 2020 •

edited

vincentqb commented Jul 14, 2020 •

edited

mthrok commented Jul 15, 2020 •

edited

vincentqb commented Jul 15, 2020 •

edited by mthrok

vincentqb commented Jul 15, 2020

vincentqb commented Jul 15, 2020

vincentqb Jul 15, 2020

mthrok Jul 16, 2020

vincentqb Jul 15, 2020 •

edited

mthrok Jul 16, 2020

vincentqb Jul 15, 2020

vincentqb left a comment

mthrok commented Jul 16, 2020

mthrok commented Jul 16, 2020



		@_mod_utils.requires_module('torchaudio._torchaudio')
		def set_buffer_size(buffer_size: int):

Add Torchscript sox effects #760

Add Torchscript sox effects #760

Conversation

mthrok commented Jul 1, 2020 • edited

codecov bot commented Jul 2, 2020 • edited

Codecov Report

cpuhrsch commented Jul 14, 2020

mthrok commented Jul 14, 2020 • edited

vincentqb commented Jul 14, 2020 • edited

mthrok commented Jul 15, 2020 • edited

vincentqb commented Jul 15, 2020 • edited by mthrok

vincentqb commented Jul 15, 2020

vincentqb commented Jul 15, 2020

vincentqb Jul 15, 2020

Choose a reason for hiding this comment

mthrok Jul 16, 2020

Choose a reason for hiding this comment

vincentqb Jul 15, 2020 • edited

Choose a reason for hiding this comment

mthrok Jul 16, 2020

Choose a reason for hiding this comment

vincentqb Jul 15, 2020

Choose a reason for hiding this comment

vincentqb left a comment

Choose a reason for hiding this comment

mthrok commented Jul 16, 2020

mthrok commented Jul 16, 2020

mthrok commented Jul 1, 2020 •

edited

codecov bot commented Jul 2, 2020 •

edited

mthrok commented Jul 14, 2020 •

edited

vincentqb commented Jul 14, 2020 •

edited

mthrok commented Jul 15, 2020 •

edited

vincentqb commented Jul 15, 2020 •

edited by mthrok

vincentqb Jul 15, 2020 •

edited