refactor(player): isolate scene FFmpeg processing

[bee-san]

Warning

Ready for architecture review; not ready to merge without targeted validation. This PR is intentionally candid: the original duplicate-native-library rationale was disproved, and there is not yet a reproduced crash proving that process isolation is required. Review should focus on whether the proposed boundary is justified and whether the validation plan is sufficient.

The problem we are trying to contain

Animated scene capture runs ffprobe and FFmpeg while the player is live. In the original design those commands execute inside Chimahon's main app process, alongside libmpv and the rest of the UI.

That creates two plausible—but not yet reproduced here—risks:

1. FFmpegKit owns process-global state, including session bookkeeping and log/statistics callbacks. Scene capture can change or exercise that state while libmpv is active.
2. A native fault in the scene-capture path terminates the whole app when FFmpegKit runs in the main process. A Java mutex can serialize calls, but it cannot contain SIGSEGV/SIGABRT or reset corrupted process-global state.

The user-visible goal is therefore narrow: if scene FFmpeg/ffprobe fails catastrophically, lose the optional animated scene and fall back cleanly rather than taking the player and card-creation flow down with it.

What we considered—and corrected

“There are duplicate FFmpeg SONAMEs, so a second process is mandatory”

That was the first explanation, and it was wrong.

aniyomi-mpv-lib does not package a competing set of libavcodec.so, libavformat.so, and related libraries. Its libmpv.so depends on the FFmpeg libraries that FFmpegKit supplies. Chimahon also already calls FFmpegKit from AnimeDownloader and FFmpegUtils in the main process.

Because there is no second implementation to separate, mutexes, load ordering, symbol visibility, and dlopen tricks do not solve a real linker collision. This PR does not claim otherwise.

Keep everything in-process and add a mutex

That remains the simplest design and may ultimately be the correct choice. A mutex can prevent concurrent FFmpegKit commands, but it cannot provide a native-crash boundary and does not isolate process-global callbacks/session state from the player.

Spawn a command-line binary

Chimahon ships FFmpegKit libraries rather than a stable external executable. A subprocess wrapper would add packaging, ABI, permissions, argument transport, and lifecycle problems without using Android's service/process model.

Why this specific design

The chosen boundary is an Android service declared as:

android:exported="false"
android:process=":scene_processing"

This gives scene capture a real address-space/process boundary while keeping the service private to the app.

The interface is deliberately small:

• AIDL carries a request ID, command type, argument array, and one-way completion callback.
• The main process binds only for the lifetime of a command and treats bind failure, binder death, null binding, or service disconnect as a failed optional animation.
• Request IDs come from one AtomicLong; the service tracks active jobs only for cancellation.
• Cancellation before submission prevents the command from starting. Cancellation after submission is forwarded to FFmpegKit.
• Cleanup waits for the native completion signal before input/output ownership is released, so cancellation cannot close a descriptor or delete a file still in native use.
• Callback output is capped at 128 Ki characters, well below Binder's transaction limit. Oversized successful output becomes a distinct logged failure rather than risking TransactionTooLargeException.

Why SAF is tokenized

A content:// URI cannot be converted to an FFmpegKit saf: parameter in one process and blindly reused in another. The native SAF registration must happen in the process that executes FFmpegKit.

The main process therefore sends a private, URL-safe encoded token. The worker decodes only content URIs and calls FFmpegKitConfig.getSafParameterForRead() locally. This avoids the earlier /proc/self/fd/N approach, whose descriptor/grant semantics do not survive a process boundary and can fail with EACCES.

Why the worker has minimal application initialization

Android still invokes Application.onCreate() in a named child process. Repeating full DI, Conscrypt, WebView, migrations, widgets, and unrelated app startup work in an FFmpeg-only worker adds latency and new failure modes.

App.onCreate() detects the exact :scene_processing suffix, installs only logging, and returns. The manifest comment and SceneCommandProcess.SUFFIX cross-reference each other so renaming one side does not silently restore full initialization.

Failure behavior

This PR does not make FFmpeg crash-proof. It changes the blast radius and normalizes failure:

• worker bind/death/error → scene command fails;
• oversized callback → logged scene failure;
• cancellation → native command is cancelled and awaited;
• invalid SAF token/registration → fail closed;
• service destruction → worker scope is cancelled;
• the surrounding capture flow retains its still-image fallback.

Only scene mining uses this boundary. Existing downloader/storage FFmpegKit callers remain in the main process.

Scope

This PR contains only the worker-process plumbing:

• private manifest service;
• AIDL request/callback contracts;
• main-process executor and lifecycle handling;
• worker service and bounded result delivery;
• process-aware lightweight app startup;
• SAF token transport and worker-side registration.

It deliberately excludes:

• native FFmpeg fixes/dependency pin (build: use patched ffmpeg-kit for animated AVIF #99);
• MediaCodec selection and AVIF capture portability (fix(player): harden animated scene capture portability #101);
• player surface/PiP/UI lifecycle fixes (fix(player): harden player lifecycle behavior #102).

Verification so far

• One commit directly on upstream main.
• Changed Kotlin/XML files pass scoped Spotless checks.
• git diff --check passes.
• ./gradlew :app:compileDebugKotlin passes.
• Earlier integrated worker builds completed real scene-mining happy paths on:
  • Lenovo TB132FU / Android 14
  • Samsung Galaxy S24 Ultra / Android 16
  • HiBy M500 / Android 14
• Those runs validate predecessor integrated worktrees—not this exact standalone commit—and did not deliberately crash/kill the worker or reproduce process-global-state interference.
• Upstream PR CI currently stops at repository-wide Spotless failures in untouched baseline domain files before build/tests.

Why this should not merge yet

The code establishes a defensible process boundary, but the premise still needs evidence. Additional limitations matter:

• this is a private app process under the normal application UID, not an Android isolated-UID security sandbox;
• the Base64 SAF token is transport encoding, not authorization or redaction;
• multiple scene requests still share one worker and its FFmpegKit global state;
• this split adds no AIDL/service instrumentation tests—the existing JVM scene tests inject a fake executor;
• command arguments cross Binder without an explicit aggregate-size cap;
• another process adds startup, memory, Binder, and lifecycle complexity.

Before merge:

• Reproduce or stress FFmpegKit process-global callback/session interference with live libmpv.
• Force-kill/crash the worker and verify the main player survives and the still fallback completes.
• Test cancellation before bind, during probe, and during encode.
• Test service disconnect/binder death and repeated bind/unbind cycles.
• Test SAF providers across worker restart and revoked grants.
• Stress ffprobe output above the Binder cap.
• Measure startup/memory cost versus keeping scene commands in-process.

If those tests do not demonstrate a benefit, the correct follow-up may be to drop this PR and keep the simpler in-process executor. The split exists so that decision can be made independently of the required portability fixes.

[sohilsayed]

is it really needed thats the question

[bee-san]

This can't be rebased — the code it refactors no longer exists on main

Attempted a rebase onto e5b03663b5 to clear the conflict. It can't be done in a way that preserves what this PR means, and I don't want to paper over that with a merge that compiles but says something different.

Concretely, this PR was based on 2f648f0a68 (v2.3.1) and refactors the ui/player/scene/ package. Since then:

• 65bbe37a (revert: remove animated AVIF scene mining (#78)) deleted that package outright — all 10 source files and 7 test files.
• e5b03663b5 re-implemented scene mining as a single PlayerMediaCaptureService, calling FFmpegKit directly in the main process and encoding through AvifEncoder.

Every type this PR touches or implements against — SceneCommandExecutor, SceneCommandResult, SceneInputLease, AndroidSceneCaptureService, AndroidSceneInputAcquirer, FfmpegKitSceneCommandExecutor — is gone. IsolatedSceneCommandExecutor implements an interface that doesn't exist; SceneSafInput.encodeForRead exists to feed a worker process that doesn't exist. Rebasing would mean re-adding the reverted architecture as a prerequisite, which is the opposite of the direction you took.

To your question on the other thread — "is it really needed, that's the question" — the honest answer from this PR's own code comment is no, not for the reason originally claimed:

an earlier version of this comment claimed the process split was required to avoid a duplicate-SONAME linker conflict between aniyomi-mpv-lib and ffmpeg-kit. That is not correct.

aniyomi-mpv-lib ships no libav*.so; ffmpeg-kit is the sole provider of those SONAMEs, so there's no collision. The remaining arguments for a separate process were isolating FFmpegKit's process-global state from a live libmpv, and containing native crashes in the media path — and neither was ever backed by a reproduction. Your new PlayerMediaCaptureService calls FFmpegKit in the main process exactly as AnimeDownloader and FFmpegUtils already did, which is consistent with there being no linker problem to solve.

So this PR is superseded, not just stale. I'd suggest closing it. If a native crash in the capture path ever does show up in the field, the AIDL worker-process scaffolding here is a reasonable starting point to revisit — but it should be motivated by that crash, not carried forward speculatively.

I've left the branch as-is rather than force-pushing something misleading. Happy to close it myself if you'd prefer.

[bee-san]

Closing as superseded, per the analysis above — the ui/player/scene/ package this refactors was removed in 65bbe37a and re-implemented as PlayerMediaCaptureService, which calls FFmpegKit in the main process. The linker-conflict premise didn't hold up, and the remaining arguments for the process split (isolating FFmpegKit's process-global state from a live libmpv, containing native crashes) were never backed by a reproduction.

Leaving the branch in place. If a native crash in the capture path ever does turn up in the field, the AIDL scaffolding here — particularly the SAF token handling, since /proc/self/fd/N is process-local and doesn't survive the boundary — is a reasonable starting point to revisit.