diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index bf2b0f6..9e0d7ea 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1,9 +1,8 @@ name: Build -# Pull requests are covered by status_check.yml, which builds the real FFmpeg AAR and -# runs the instrumented suite across API 33-37. This workflow keeps the post-merge and -# release duties, and deliberately does not duplicate PR validation: its unit job falls -# back to a stub AAR, which is a weaker check than the one status_check.yml performs. +# Pull requests are covered by status_check.yml, which runs the unit tests and the +# instrumented suite across API 33-37. This workflow keeps the post-merge and release +# duties and does not duplicate PR validation. on: push: branches: [main] @@ -23,19 +22,6 @@ jobs: - uses: gradle/actions/setup-gradle@v4 - # The FFmpeg AAR is not committed, so provide a stub for jobs that only need - # to compile and run JVM tests. Anything that actually calls into FFmpeg is an - # instrumented test and does not run here. - - name: Stub the FFmpeg AAR - run: | - mkdir -p app/libs - if [ ! -f app/libs/ffmpeg-kit-next-8.1.1.aar ]; then - echo "::warning::Using an empty FFmpeg AAR stub; instrumented tests are skipped." - mkdir -p /tmp/stub/jni && printf '' > /tmp/stub/AndroidManifest.xml - (cd /tmp/stub && zip -qr ffmpeg-kit-next-8.1.1.aar .) - cp /tmp/stub/ffmpeg-kit-next-8.1.1.aar app/libs/ - fi - - name: Unit tests run: ./gradlew testDebugUnitTest @@ -46,32 +32,9 @@ jobs: name: unit-test-report path: app/build/reports/tests/ - ffmpeg: - name: Build FFmpeg AAR - runs-on: ubuntu-latest - # Expensive (a full cross-compile of FFmpeg, x264, x265 and friends), so it runs - # only for releases rather than on every push. - if: startsWith(github.ref, 'refs/tags/v') - steps: - - uses: actions/checkout@v4 - - - name: Build the AAR in a container - run: | - cd tools/ffmpeg - podman build -t ffmpeg-kit-builder:ci -f Containerfile . || \ - docker build -t ffmpeg-kit-builder:ci -f Containerfile . - mkdir -p out - (podman run --rm -v "$PWD/out":/work/out:Z ffmpeg-kit-builder:ci full || \ - docker run --rm -v "$PWD/out":/work/out ffmpeg-kit-builder:ci full) - - - uses: actions/upload-artifact@v4 - with: - name: ffmpeg-aar - path: tools/ffmpeg/out/ffmpeg-kit-next-*.aar - release: name: Release - needs: [test, ffmpeg] + needs: [test] runs-on: ubuntu-latest if: startsWith(github.ref, 'refs/tags/v') steps: @@ -84,11 +47,6 @@ jobs: - uses: gradle/actions/setup-gradle@v4 - - uses: actions/download-artifact@v4 - with: - name: ffmpeg-aar - path: app/libs/ - - name: Build release artifacts run: ./gradlew assembleRelease bundleRelease diff --git a/.github/workflows/status_check.yml b/.github/workflows/status_check.yml index e400e26..1ebf42a 100644 --- a/.github/workflows/status_check.yml +++ b/.github/workflows/status_check.yml @@ -4,93 +4,70 @@ on: pull_request: branches: [main] -# A newer push to the same PR makes the in-flight run obsolete. Emulator matrices are -# expensive, so cancel rather than let them pile up. +# A newer push to the same PR makes the in-flight run obsolete. An emulator matrix is +# expensive, so cancel rather than let runs pile up. concurrency: group: status-check-${{ github.ref }} cancel-in-progress: true -env: - # Must match the coordinate app/build.gradle.kts loads from app/libs/. - FFMPEG_AAR: ffmpeg-kit-next-8.1.1.aar - jobs: # --------------------------------------------------------------------------- - # FFmpeg is not committed: the AAR is ~35 MB of native code, and F-Droid strips - # checked-in binaries. Every other job needs it, because the app module compiles - # against it and the instrumented tests exercise it for real. + # Validates the committed FFmpeg archive. It does not build anything: the whole + # point of checking the binary in is that a red run means broken code rather than + # a cross-compile that hiccuped. # - # Building it is a full cross-compile of FFmpeg, x264, x265 and SVT-AV1, so it is - # cached on the contents of tools/ffmpeg. That directory pins the upstream tag and - # the configure flags, which is exactly what determines the output. + # Its own job so a bad archive reports once, clearly, instead of surfacing as five + # confusing emulator failures. It takes seconds, so gating the matrix on it costs + # almost nothing. # --------------------------------------------------------------------------- ffmpeg: - name: FFmpeg AAR + name: FFmpeg binary runs-on: ubuntu-latest - timeout-minutes: 120 + timeout-minutes: 10 steps: - uses: actions/checkout@v4 - - name: Restore cached AAR - id: cache - uses: actions/cache@v4 - with: - path: tools/ffmpeg/out - key: ffmpeg-aar-${{ hashFiles('tools/ffmpeg/Containerfile', 'tools/ffmpeg/build-ffmpeg.sh') }} - - # The Nix store plus the build tree runs to several gigabytes, which does not fit - # alongside the runner's preinstalled toolchains. - - name: Free disk space - if: steps.cache.outputs.cache-hit != 'true' + - name: Verify the committed archive run: | - sudo rm -rf /usr/share/dotnet /usr/local/lib/android /opt/ghc /usr/local/share/boost - sudo docker image prune -af || true - df -h / + AAR=bin/ffmpeg-kit-next-8.1.1.aar + test -f "$AAR" || { echo "::error::$AAR is missing"; exit 1; } - - name: Build the AAR - if: steps.cache.outputs.cache-hit != 'true' - working-directory: tools/ffmpeg - run: | - mkdir -p out - docker build -t ffmpeg-kit-builder:ci -f Containerfile . - docker run --rm -v "$PWD/out":/work/out ffmpeg-kit-builder:ci full - - - name: Check the AAR is present and plausibly complete - run: | - AAR=$(find tools/ffmpeg/out -name 'ffmpeg-kit-next*.aar' | head -1) - test -n "$AAR" || { echo "::error::no AAR produced"; exit 1; } - # A truncated or stub AAR would still satisfy `test -f`, so check it carries - # native libraries for both ABIs before letting the matrix depend on it. + # A truncated file, or a Git LFS pointer checked out without LFS, would pass + # a file-exists check and then surface much later as a confusing linker + # error. Assert the archive actually carries native libraries for both ABIs. for abi in arm64-v8a x86_64; do n=$(unzip -l "$AAR" | grep -c "jni/$abi/.*\.so$" || true) echo " $abi: $n shared libraries" test "$n" -gt 0 || { echo "::error::AAR has no $abi libraries"; exit 1; } done - cp "$AAR" "${{ env.FFMPEG_AAR }}" - - uses: actions/upload-artifact@v4 - with: - name: ffmpeg-aar - path: ${{ env.FFMPEG_AAR }} - retention-days: 1 + # 16 KB alignment is a Play requirement and is easy to lose in a rebuild, + # so it is checked here rather than discovered at submission. + unzip -q -o "$AAR" 'jni/*' -d /tmp/aarcheck + bad=0 + for f in /tmp/aarcheck/jni/*/*.so; do + align=$(readelf -lW "$f" | awk '$1=="LOAD"{print $NF}' | sort -u) + if [ "$align" != "0x4000" ]; then + echo "::error::$(basename "$f") is $align, not 16 KB aligned"; bad=1 + fi + done + test "$bad" -eq 0 || exit 1 + echo " all libraries are 16 KB aligned" + + # Record what shipped, so a failing run elsewhere can be tied to a version. + echo " sha256: $(sha256sum "$AAR" | cut -d' ' -f1)" # --------------------------------------------------------------------------- # JVM tests: the routing matrix, the FFmpeg argument builder, the concat planner - # and the retry rule. These need no device and are the fastest signal on a PR. + # and the retry rule. No device needed, so this is the fastest signal on a PR. # --------------------------------------------------------------------------- unit: name: Unit tests runs-on: ubuntu-latest - needs: ffmpeg timeout-minutes: 30 steps: - uses: actions/checkout@v4 - - uses: actions/download-artifact@v4 - with: - name: ffmpeg-aar - path: app/libs/ - - uses: actions/setup-java@v4 with: distribution: temurin @@ -98,7 +75,8 @@ jobs: - uses: gradle/actions/setup-gradle@v4 - - run: ./gradlew :app:testDebugUnitTest + - name: Unit tests + run: ./gradlew :app:testDebugUnitTest - uses: actions/upload-artifact@v4 if: always() @@ -107,18 +85,24 @@ jobs: path: app/build/reports/tests/ # --------------------------------------------------------------------------- - # Instrumented tests across the whole supported range. minSdk is 33 and targetSdk - # is 37, and the foreground-service type differs across that range -- none below - # 34, dataSync at 34, mediaProcessing from 35 -- so a single API level would leave - # two thirds of that branch unexercised. + # One runner per API level, across the whole supported range. + # + # The range is the point: minSdk is 33 and targetSdk is 37, and the foreground + # service type differs across it -- none below 34, dataSync at 34, mediaProcessing + # from 35. Testing a single level would leave two thirds of that branch unexercised, + # and running the range locally is what caught a test that had baked in an + # assumption about the host's encoders. + # + # FFmpeg is not built here. The AAR is committed under bin/, so a red run means the + # code is broken rather than that a cross-compile hiccuped. # --------------------------------------------------------------------------- - instrumented: + e2e: name: E2E API ${{ matrix.api-level }} runs-on: ubuntu-latest needs: ffmpeg timeout-minutes: 60 strategy: - # Report every API level rather than stopping at the first red one: knowing + # Report every API level rather than stopping at the first red one. Knowing # whether a failure is universal or specific to one level is most of the # diagnosis. fail-fast: false @@ -132,18 +116,13 @@ jobs: system-image-api-level: 35 - api-level: 36 system-image-api-level: 36 - # API 37 is published as android-37.0, not android-37, so the image level - # has to be given separately or the download resolves to nothing. + # API 37 is published as android-37.0, not android-37, so the image level has + # to be given separately or the download resolves to no package at all. - api-level: 37 system-image-api-level: "37.0" steps: - uses: actions/checkout@v4 - - uses: actions/download-artifact@v4 - with: - name: ffmpeg-aar - path: app/libs/ - - uses: actions/setup-java@v4 with: distribution: temurin @@ -168,9 +147,9 @@ jobs: target: google_apis arch: x86_64 profile: pixel_6 - # -gpu swiftshader_indirect is the usual CI choice. It is correct here only - # because runners have no GPU to pass through; on a workstation the same - # setting routes through SwiftShader's JIT, which is a known crash source. + # swiftshader_indirect is correct here only because runners have no GPU to + # pass through. On a workstation the same setting routes through + # SwiftShader's JIT, which is a known crash source. emulator-options: -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none disable-animations: true script: ./gradlew :app:connectedDebugAndroidTest diff --git a/.gitignore b/.gitignore index 9184f80..b95fa12 100644 --- a/.gitignore +++ b/.gitignore @@ -14,5 +14,6 @@ .cxx local.properties -# Built FFmpeg AAR - see tools/ffmpeg/ for the build recipe -app/libs/*.aar +# The FFmpeg AAR is committed under bin/ so test runs do not depend on a rebuild. +# Build outputs from tools/ffmpeg are not. +tools/ffmpeg/out/ diff --git a/README.md b/README.md index 4475f24..8ee56e1 100644 --- a/README.md +++ b/README.md @@ -82,19 +82,10 @@ restored after a restart. Requires JDK 17+ (AGP 9 will not run on older) and the Android SDK with API 37. -The FFmpeg AAR is **not committed** — it is a 35 MB binary, and F-Droid strips -checked-in native libraries. Build it first: - -```sh -cd tools/ffmpeg -podman build -t ffmpeg-kit-builder:local -f Containerfile . -mkdir -p out -podman run --name ffmpeg-build -v "$PWD/out":/work/out:Z \ - localhost/ffmpeg-kit-builder:local full -cp out/ffmpeg-kit-next-*.aar ../../app/libs/ -``` - -Then: +FFmpeg is committed as a prebuilt archive under [`bin/`](bin/README.md), so a clone +builds without a cross-compile. That is deliberate: rebuilding it per CI run made test +results ambiguous, because a red build could mean broken code or a build that hiccuped. +See [`bin/README.md`](bin/README.md) for its provenance and how to regenerate it. ```sh ./gradlew :app:assembleDebug # debug APK @@ -104,7 +95,9 @@ Then: ``` See [`tools/ffmpeg/README.md`](tools/ffmpeg/README.md) for why the build is -containerised and which flags matter. +containerised and which flags matter. That recipe remains the authority — the committed +archive is its output, and is also what satisfies the GPL corresponding-source +obligation. ## Testing diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 3cb0a41..4778ad7 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -76,10 +76,13 @@ dependencies { implementation(libs.media3.common) implementation(libs.media3.muxer) - // FFmpeg, built from source by tools/ffmpeg. Not on any Maven repo: ffmpeg-kit was - // archived and delisted, and ffmpeg-kit-next is source-only by design. - // The AAR is gitignored; see app/libs/README.md to produce it. - implementation(files("libs/ffmpeg-kit-next-8.1.1.aar")) + // FFmpeg, committed under bin/. Not on any Maven repo: ffmpeg-kit was archived and + // delisted, and ffmpeg-kit-next is source-only by design. + // + // The prebuilt archive is checked in on purpose. Rebuilding it per CI run made test + // results ambiguous -- a red build could mean broken code or a cross-compile that + // hiccuped. See bin/README.md for provenance and how to regenerate it. + implementation(files(rootProject.file("bin/ffmpeg-kit-next-8.1.1.aar"))) // A local .aar carries no transitive dependencies, so the wrapper's own runtime // dependency has to be declared here explicitly. implementation(libs.smart.exception.java) diff --git a/app/libs/README.md b/app/libs/README.md deleted file mode 100644 index fc50130..0000000 --- a/app/libs/README.md +++ /dev/null @@ -1,17 +0,0 @@ -# Local FFmpeg AAR - -`ffmpeg-kit-next-*.aar` is **built, not committed**. Build it with: - -```sh -cd tools/ffmpeg -podman build -t ffmpeg-kit-builder:local -f Containerfile . -mkdir -p out -podman run --name ffmpeg-build -v "$PWD/out":/work/out:Z \ - localhost/ffmpeg-kit-builder:local full -cp out/ffmpeg-kit-next-*.aar ../../app/libs/ -``` - -The `.aar` is deliberately gitignored. It is a ~35 MB binary blob, and F-Droid's build -process strips checked-in prebuilt native libraries — committing it would break the -F-Droid build and bloat the repository. The reproducible recipe in `tools/ffmpeg/` is -the artifact of record, and it also serves as the GPL corresponding-source obligation. diff --git a/bin/README.md b/bin/README.md new file mode 100644 index 0000000..703e34c --- /dev/null +++ b/bin/README.md @@ -0,0 +1,80 @@ +# Prebuilt FFmpeg + +`ffmpeg-kit-next-8.1.1.aar` is committed here deliberately, and this file records what +it is so the binary is auditable rather than opaque. + +## Why it is committed + +Tests must be deterministic. When CI rebuilt FFmpeg on every run, a red build could mean +"the code is broken" or "a 40-minute cross-compile hiccuped", and those are not the same +signal. Committing the artifact removes the second possibility entirely: a test run +either passes or points at real code. + +It also removes roughly forty minutes from every cold CI run. + +## Provenance + +| | | +|---|---| +| Upstream | [arthenica/ffmpeg-kit-next](https://github.com/arthenica/ffmpeg-kit-next) v8.1.1 | +| FFmpeg | 8.1.2 | +| NDK | r27d (27.3.13750724), pinned by the upstream flake | +| API level | 33, matching the app's minSdk | +| ABIs | arm64-v8a, x86_64 | +| Shared libraries | 20 (10 per ABI) | +| SHA-256 | `ae188c9aec3c89a1c87a169589253c85438d57cfdcc3ce8b40fb3e87de368ff2` | + +Configure line, read back out of the shipped `libavutil.so`: + +``` +--enable-asm --enable-cross-compile --enable-gpl --enable-iconv +--enable-inline-asm --enable-jni --enable-libass --enable-libdav1d +--enable-libfontconfig --enable-libfreetype --enable-libfribidi +--enable-libharfbuzz --enable-libjxl --enable-libmp3lame --enable-libopus +--enable-libsvtav1 --enable-libvpx --enable-libx264 --enable-libx265 +--enable-lto --enable-mediacodec --enable-neon --enable-optimizations +--enable-pic --enable-pthreads --enable-shared --enable-small +--enable-swscale --enable-v4l2-m2m --enable-version3 --enable-zlib +``` + +Every `.so` reports `LOAD align 0x4000`, so the archive satisfies the 16 KB page-size +requirement. Verify with: + +```sh +unzip -o bin/ffmpeg-kit-next-8.1.1.aar 'jni/*' -d /tmp/aarcheck +for f in /tmp/aarcheck/jni/*/*.so; do + readelf -lW "$f" | awk -v f="$f" '$1=="LOAD"{print f, $NF}' +done | sort -u -k2 +``` + +## Licensing + +Built with `--enable-gpl` and `--enable-version3`, so this binary is **GPL-3.0** and the +distributed APK is GPL-3.0 with it. That is deliberate: x264 and x265 are the only route +to CRF and two-pass rate control, which no Android hardware encoder exposes. See +[`../LICENSES/README.md`](../LICENSES/README.md). + +GPL-3.0 obliges us to ship corresponding source with the binary. The recipe in +[`../tools/ffmpeg`](../tools/ffmpeg) is that source, and it remains the authority: this +archive is its output, not a substitute for it. + +## Rebuilding + +```sh +cd tools/ffmpeg +podman build -t ffmpeg-kit-builder:local -f Containerfile . +mkdir -p out +podman run --name ffmpeg-build -v "$PWD/out":/work/out:Z localhost/ffmpeg-kit-builder:local full +cp out/ffmpeg-kit-next-*.aar ../../bin/ +``` + +Update the SHA-256 above when you do. Note that replacing this file adds another ~34 MB +blob to git history permanently, so rebuild only when the FFmpeg version or the configure +flags actually change. + +## F-Droid + +F-Droid's scanner flags checked-in prebuilt native libraries. If the app is submitted +there, the metadata needs a `scandelete` entry for `bin/` so their build uses the recipe +in `tools/ffmpeg` rather than this archive. Nothing here prevents a from-source build; +the recipe is complete on its own. diff --git a/bin/ffmpeg-kit-next-8.1.1.aar b/bin/ffmpeg-kit-next-8.1.1.aar new file mode 100644 index 0000000..36a6e4c Binary files /dev/null and b/bin/ffmpeg-kit-next-8.1.1.aar differ