-
Notifications
You must be signed in to change notification settings - Fork 759
Add a switch to build ffmpeg binding #2048
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
Conversation
ac2d5c0 to
a316b1c
Compare
Summary: (This is a part of refactor series, followed up by #2039 and #2040. The goal is to make it easy to add a new library artifact alongside with `libtorchudio`, as in 4ced990 #2048 .) We plan to add prototype/beta third party library integrations, which could be unstable. (segfault, missing dynamic library dependencies etc...) If we add such integrations into the existing libtorchaudio, in the worst case, it will prevent users from just `import torchaudio`. Instead, we would like to separate the prototype/beta integrations into separate libraries, so that such issues would not impact all users but users who attempt to use these prototytpe/beta features. Say, a prototype feature `foo` is added in `torchaudio.prototype.foo`. The following initialization procedure will achieve the above mechanism. 1. Place the library file `libtorchaudio_foo` in `torchaudio/lib`. 2. In `torchaudio.prototype.foo.__init__.py`, load the `libtorchaudio_foo`. Note: The approach will be slightly different for fbcode, because of how buck deploys C++ libraries and standardized environment, but the code change here is still applicable. Pull Request resolved: #2038 Reviewed By: carolineechen, nateanl Differential Revision: D32682900 Pulled By: mthrok fbshipit-source-id: 0f402a92a366fba8c2894a0fe01f47f8cdd51376
d49808b to
1933b9f
Compare
3c08723 to
4899df3
Compare
|
@mthrok has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
| if (BUILD_FFMPEG) | ||
| find_package(PkgConfig REQUIRED) | ||
| pkg_check_modules(LIBAV REQUIRED IMPORTED_TARGET | ||
| # requires ffmpeg>=4.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the comment intended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. ffmpeg package is composed of multiple libraries, each of which has diff version numbers. https://ffmpeg.org/download.html#release_4.1
Without a comment It is hard to see which version of ffmpeg package is required here.
|
@mthrok has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Summary: (This is a part of refactor series, followed up by pytorch#2039 and pytorch#2040. The goal is to make it easy to add a new library artifact alongside with `libtorchudio`, as in pytorch@4ced990 pytorch#2048 .) We plan to add prototype/beta third party library integrations, which could be unstable. (segfault, missing dynamic library dependencies etc...) If we add such integrations into the existing libtorchaudio, in the worst case, it will prevent users from just `import torchaudio`. Instead, we would like to separate the prototype/beta integrations into separate libraries, so that such issues would not impact all users but users who attempt to use these prototytpe/beta features. Say, a prototype feature `foo` is added in `torchaudio.prototype.foo`. The following initialization procedure will achieve the above mechanism. 1. Place the library file `libtorchaudio_foo` in `torchaudio/lib`. 2. In `torchaudio.prototype.foo.__init__.py`, load the `libtorchaudio_foo`. Note: The approach will be slightly different for fbcode, because of how buck deploys C++ libraries and standardized environment, but the code change here is still applicable. Pull Request resolved: pytorch#2038 Reviewed By: carolineechen, nateanl Differential Revision: D32682900 Pulled By: mthrok fbshipit-source-id: 0f402a92a366fba8c2894a0fe01f47f8cdd51376
Summary: This PR adds `BUILD_FFMPEG` switch to torchaudio build process so that features related to ffmpeg are built. The flag is false by default, so no CI jobs or development flow are affected. This is because handling the dependencies around ffmpeg is a bit tricky. Currently, the CMake file uses `pkg-config` to find an ffmpeg installation in the system. This works fine for both conda-based installation and system-managed installation (like `apt`). In subsequent PRs, I will find a solution that works for local development and binary distributions. Pull Request resolved: pytorch#2048 Reviewed By: hwangjeff, nateanl Differential Revision: D33367260 Pulled By: mthrok fbshipit-source-id: 94517acecb62bd6d4e96d4b7cbc3ab3c2a25706c
Downgrading to this version of protobuf may be necessary for this workflow to succeed. This is reflected in our build here https://github.com/pytorch/pytorch/blob/master/.github/workflows/_ios-build-test.yml#L168
This PR adds
BUILD_FFMPEGswitch to torchaudio build process so that features related to ffmpeg are built.The flag is false by default, so no CI jobs or development flow are affected.
This is because handling the dependencies around ffmpeg is a bit tricky.
Currently, the CMake file uses
pkg-configto find an ffmpeg installation in the system.This works fine for both conda-based installation and system-managed installation (like
apt).In subsequent PRs, I will find a solution that works for local development and binary distributions.