Add Mac to FFmpeg build CI #201
Conversation
facebook-github-bot
added
the
CLA Signed
scotts
force-pushed
the
mac_wheel
branch
8 times, most recently
from
0f99689 to
54f1574
Compare
| fail-fast: false | ||
| matrix: | ||
| ffmpeg-version: ["4.4.4", "5.1.4", "6.1.1", "7.0.1"] | ||
| runner: ["macos-m1-stable", "macos-12"] |
There was a problem hiding this comment.
Pytorch doesn't support x86 for mac anymore
| runner: ["macos-m1-stable", "macos-12"] | |
| runner: ["macos-m1-stable"] |
There was a problem hiding this comment.
Oh, interesting. The curious bit here is that my system is an x86 Mac, so if I'm going to do any local testing of Mac wheels, then I'll need to keep this in. I can just use CI for everything instead. Let me know which you think is more important: aligning with general PyTorch, or enabling local testing.
There was a problem hiding this comment.
You shouldn't need to keep this in for local testing. This just instructs the CI to spin x86 instances. This is never executed on your machine. The actual build script should execute just fine for you locally.
There was a problem hiding this comment.
Yes, agreed, this never executes on my machine. But I will want to locally define BUILD_AGAINST_ALL_FFMPEG_FROM_S3 and build on my machine to test that flow locally. And if we don't build non-GPL FFmpeg x86 binaries for Mac in this step in CI, then they won't be available to upload to S3, and I won't be able to test that flow locally.
We can remove it after I've gotten everything setup, but I think at least while I'm working on this, it's better that these binaries are available.
|
As I look forward to how to how we'll want to pull these artifacts to use in builds, I think it will actually be easier if we build Linux and Mac ffmpeg non-GPL binaries using the same infra, and upload them together in the same tar file. I'm going to change this PR to reflect that. I'll request a re-review. @NicolasHug, please let me know if you think we should prioritize alignment with PyTorch's support, or keep the repo in a state we can all locally test. |
I was going to suggest that we add a "TODO: we should migrate the linux job to pytorch infra as well". This doesn't need to happen now, but if you think it makes more sense, that's perfectly fine too! |
scotts
force-pushed
the
mac_wheel
branch
3 times, most recently
from
f4404d2 to
29bfc1f
Compare
|
I moved Linux over to PyTorch infra as well. All binaries now get uploaded into a single tar file. Example: https://github.com/pytorch/torchcodec/actions/runs/10744678413 |
|
The failing test is unrelated to these changes, see Issue #203. Merging to unblock; we can remove macos-12 after the entire Mac wheel flow works. |
Adds a Mac build of the FFmpeg non-GPL libraries. The code is lifted almost verbatim from TorchAudio's setup, which reuses pytorch/test-infra. Note there are subtle differences between our current Linux job spec and this new one for Mac - those differences are necessary to work with pytorch/test-infra's setup.
Since this action will only execute once a week on Sunday, I tested it by temporarily hacking it to run on a pull request. See the result of one of those runs to see successful execution: https://github.com/pytorch/torchcodec/actions/runs/10730119629