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

Remove special cases for torchaudio FFmpeg #4358

Merged
merged 2 commits into from
Jul 11, 2023

Conversation

mthrok
Copy link
Contributor

@mthrok mthrok commented Jul 7, 2023

This commit removes all the code related to FFmpeg, which are specific to torchaudio.

pytorch/audio#3460 changed the way torchaudio is built. FFmpeg integration is handled automatically and enabled by default. This eliminates the need to supply FFmepg externally. test-infra no longer needs to build FFmpeg and use pre/post-scripts for torchaudio.

Note that smoke test was updated in pytorch/audio#3465 to not use FFmpeg for wheel.
We will revisit this later with multiple FFmpeg version support.

@vercel
Copy link

vercel bot commented Jul 7, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
torchci ⬜️ Ignored (Inspect) Jul 10, 2023 9:34pm

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 7, 2023
@mthrok mthrok changed the title Remove special cases for torchaudio FFmpeg [WIP] Remove special cases for torchaudio FFmpeg Jul 7, 2023
facebook-github-bot pushed a commit to pytorch/audio that referenced this pull request Jul 10, 2023
Summary:
1. Update smoke test script to change directory so that there is no `torchaudio` directory in CWD when smoke test is being executed.
2. Disable the part of smoke test which requires FFmpeg for wheel. The preparation for pytorch/test-infra#4358

Pull Request resolved: #3465

Reviewed By: nateanl

Differential Revision: D47345117

Pulled By: mthrok

fbshipit-source-id: 95aad0a22922d44ee9a24a05d9ece85166b8c17e
@mthrok mthrok closed this Jul 10, 2023
@mthrok mthrok reopened this Jul 10, 2023
pytorch/audio#3460 changed the way torchaudio is built,
and there is no longer need to supply FFmepg externally.
test-infra no longer needs to build FFmpeg and use pre/post-scripts for torchaudio.

The only issue remaining for now is smoke test.
Currently torchaudio's smoke test script by default expects that FFmpeg libraries
are available. This can be disabled with `--no-ffmpeg`, but the way YML files are
written in test-infra repo does not allow to pass a flag to smoke test.

We can switch the behavior, or use other smoke test.
@mthrok mthrok force-pushed the remove-ffmpeg-special-handling branch from 3bd382b to 87673d9 Compare July 10, 2023 21:15
mthrok added a commit to pytorch/audio that referenced this pull request Jul 10, 2023
Now that we do not build FFmpeg as part of CI build process,
we can remove the pre/post build scripts.

Needs to land after pytorch/test-infra#4358
@mthrok mthrok changed the title [WIP] Remove special cases for torchaudio FFmpeg Remove special cases for torchaudio FFmpeg Jul 10, 2023
@mthrok mthrok marked this pull request as ready for review July 10, 2023 21:43
@mthrok mthrok requested a review from atalman July 10, 2023 21:43
@atalman atalman merged commit 44d9410 into main Jul 11, 2023
59 checks passed
mthrok added a commit to pytorch/audio that referenced this pull request Jul 11, 2023
Now that we do not build FFmpeg as part of CI build process,
we can remove the pre/post build scripts.

Needs to land after pytorch/test-infra#4358
@mthrok mthrok deleted the remove-ffmpeg-special-handling branch July 11, 2023 14:07
facebook-github-bot pushed a commit to pytorch/audio that referenced this pull request Jul 11, 2023
Summary:
Now that we do not build FFmpeg as part of CI build process, we can remove the pre/post build scripts.

Needs to land after pytorch/test-infra#4358

Pull Request resolved: #3466

Reviewed By: atalman

Differential Revision: D47367022

Pulled By: mthrok

fbshipit-source-id: 17aafff74ee7d269236cffb8a88c803a8d4c44b7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants