Test building with Sox on Windows by peterjc123 · Pull Request #648 · pytorch/audio

peterjc123 · 2020-05-17T14:34:24Z

Relates to #425

peterjc123 · 2020-05-17T17:39:40Z

Unit tests passed for both CPU and GPU with Sox on Windows. Marking it as ready, cc @vincentqb @mthrok.

mthrok

Thanks for the splendid job. Left some comments.

mthrok · 2020-05-17T19:44:48Z

.circleci/unittest/windows/scripts/install.sh


 printf "* Installing torchaudio\n"
-IS_CONDA=true python setup.py develop
+curl --retry 3 https://s3.amazonaws.com/ossci-windows/torchaudio_deps.7z --output /tmp/torchaudio_deps.7z


Several questions regarding this

What does torchaudio_deps.7z contain?

Where is it from?

Does it include source or binary?

(if binary)

How was it built?

How to update it if dependencies should change in future?

What does torchaudio_deps.7z contain?

The header and the libraries of the third-party dependencies, like FLAC, libmad and libmp3lame.

Where is it from?

Built locally by myself.

Does it include source or binary?

Binary.

How was it built?

Generally speaking, I built them one by one manually according to the steps given by the project itself using VS 2019. However, some of the project support only very old format of the MSVC project, so I have to rewrite it using CMake.

How to update it if dependencies should change in future?

I hope that I could figure out the automatic build script for those projects. It is not easy because like I said in the previous question that I had to modify them a little bit. But I think it would be fine since some of the dependencies are not updated for a long time, like Sox and libmad. So for those old projects, maybe we could just download deps or maintain a fork. As for the active projects, like FLAC, we could just use the steps provided by the project itself.

Yeah I'm not entirely sure if I would want to sign us up for this considering any update would have to be done by hand (however infrequent the updates may come).

Is there a way to contribute what we've done to make this dependency archive to those upstream dependencies?

mthrok · 2020-05-17T19:49:06Z

setup.py

+        extra_objects += [os.path.join(audio_path, 'third_party/lame/lib/mp3lame.lib')]
+        extra_objects += [os.path.join(audio_path, 'third_party/ogg/lib/ogg.lib')]
+        extra_objects += [os.path.join(audio_path, 'third_party/sox/lib/lpc10.lib')]
+        extra_objects += [os.path.join(audio_path, 'third_party/sox/lib/gsm.lib')]


In #625, I am merging third party libraries into third_party/build/include and third_party/build/lib for simplicity. Could you adopt the same pattern?

So maybe I should wait for your PR? Or I just do that for Windows?

You can just do that for Windows.

mthrok · 2020-05-17T20:01:50Z

torchaudio/csrc/sox.cpp

 namespace audio {
 namespace {
+#ifdef _WIN32
+int mkstemp(char* tmpl) {


Is this your implementation, or taken from somewhere?

It comes from my PR: https://github.com/pytorch/pytorch/pull/25578/files#diff-dc1022da8c9adb512abfa2caa114008dR30.

mthrok · 2020-05-19T14:38:53Z

@seemethere

Could you take a look please?
This is a great addition to torchaudio, and the code itself looks reasonable to me.
But I do not have enough context around solution to provide pre-built binary for underlying codec libraries and the maintainability of that approach.
Is ossci-windows bucket managed by PyTorch org? (looking at this FAQ, it seems the case).

peterjc123 · 2020-05-19T15:01:55Z

Is ossci-windows bucket managed by PyTorch org? (looking at this FAQ, it seems the case).

Yes. I uploaded the torchaudio_deps.7z to that bucket.

the maintainability of that approach.

Just curious about the update frequency of the libraries. Looks like some of the libraries are not updated for a long time, like Sox, lame and mad.

mthrok · 2020-05-19T15:27:22Z

Just curious about the update frequency of the libraries. Looks like some of the libraries are not updated for a long time, like Sox, lame and mad.

Although chances are small, other potential cases for updating the dependencies for some unanticipated reason are removing one or adding a new one.

I think we want to make sure someone is able to reproduce the build steps you took in some form, just in case.

seemethere · 2020-05-27T18:18:52Z

packaging/vs2019/conda_build_config.yaml

+vc:
+  - 14
+zip_keys:
+  -                             # [win]


Is this purposefully blank? Looks a bit confusing here without a comment.

No, and it should be a typo.

facebook-github-bot · 2020-10-30T17:36:05Z

Hi @peterjc123!

Thank you for your pull request. We require contributors to sign our Contributor License Agreement, and yours needs attention.

You currently have a record in our system, but we do not have a signature on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

…orials Reorganize Production Usage section

facebook-github-bot · 2021-07-31T20:08:11Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

peterjc123 added 7 commits May 17, 2020 22:33

Test building with Sox on Windows

e39a006

Add missing libraries for linker options

74058c2

More fixes

05d81aa

Minor fixes

8908916

Fixing sox effects and debug conda failure

103f83c

More fixes

37b2fed

Fix conda build and unit tests

561d167

peterjc123 force-pushed the test_sox_win branch from e52bf0f to 561d167 Compare May 17, 2020 17:00

peterjc123 added 3 commits May 18, 2020 01:16

Don't add vc to host and try cuda tests

6af3196

Fix conda tests

108e6c5

Remove pySoundFile as a required dep on Windows

3f94833

peterjc123 marked this pull request as ready for review May 17, 2020 17:40

mthrok reviewed May 17, 2020

View reviewed changes

peterjc123 mentioned this pull request May 18, 2020

Windows Support #425

Closed

11 tasks

peterjc123 mentioned this pull request May 23, 2020

Experimental windows builds #219

Closed

seemethere reviewed May 27, 2020

View reviewed changes

vincentqb added the module: windows label Jun 16, 2020

facebook-github-bot added the CLA Signed label Oct 30, 2020

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

Merge pull request pytorch#648 from pytorch/reorganize_production_tut…

98636c4

…orials Reorganize Production Usage section

mthrok closed this Jul 31, 2023

Conversation

peterjc123 commented May 17, 2020 • edited by vincentqb Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peterjc123 commented May 17, 2020

Uh oh!

mthrok left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peterjc123 May 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mthrok commented May 19, 2020

Uh oh!

peterjc123 commented May 19, 2020

Uh oh!

mthrok commented May 19, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Oct 30, 2020

Uh oh!

facebook-github-bot commented Jul 31, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

peterjc123 commented May 17, 2020 •

edited by vincentqb

Loading

peterjc123 May 18, 2020 •

edited

Loading