Skip to content

fix(player): harden animated scene capture portability - #101

Closed
bee-san wants to merge 1 commit into
sohilsayed:mainfrom
bee-san:agent/animated-avif-portability
Closed

fix(player): harden animated scene capture portability#101
bee-san wants to merge 1 commit into
sohilsayed:mainfrom
bee-san:agent/animated-avif-portability

Conversation

@bee-san

@bee-san bee-san commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Important

Ready for code review; device validation remains a merge gate. Local Kotlin compilation and focused scene tests pass, but JVM tests mock the native executor and cannot prove vendor MediaCodec behavior. Unsupported or failed animation generation deliberately returns the already-captured still image.

The problem

Animated Anki scene capture worked on the device and media shape it was first built around, but the implementation made assumptions that Android's codec API does not guarantee:

  • choose the first advertised AV1 encoder;
  • ask it for one fixed 640×640 shape;
  • assume the source's coded width/height are its display geometry;
  • assume 8-bit input and a simple YUV420 layout;
  • treat command success as proof that the AVIF is decodable from frame zero;
  • allow native cancellation and Kotlin cleanup to race each other;
  • silently fall back when any of those gates fail.

Real devices vary in alignment, minimum/maximum dimensions, block budgets, conditional width ranges, rate support, color formats, CQ support, and vendor behavior. Real media also carries rotation, sample-aspect ratio, 10-bit SDR, HDR metadata, embedded subtitle streams, SAF inputs, and remote URLs.

The result was not one isolated bug. Depending on device/input, capture could select an unusable encoder, distort portrait or anamorphic video, request unsupported dimensions, produce an AVIF with an invalid first frame, fail on a harmless subtitle stream, lose cancellation, leak/delete files at the wrong time, or simply return a still with no useful diagnostic.

Concrete failures that drove the design

These were not hypothetical codec-matrix concerns:

  • Samsung Galaxy S24 Ultra / SAF input: ffprobe received /proc/self/fd/497, reopened it by path, lost the SAF grant, and failed with Permission denied before MediaCodec ran. The fix is FFmpegKit's native saf: protocol rather than reopening a FUSE-backed descriptor path. Diagnosis
  • Lenovo TB132FU / capability gate: its only AV1 encoder rejected the synthetic 640×640 @ 8 fps check, so capture returned in roughly 13 ms even though the actual 16:9 source could be encoded at 640×360. Source-aware selection later produced a validated 32-frame, four-second looping AVIF. Failure · follow-up pass
  • Samsung Galaxy S24 Ultra / subtitled MP4: stream discovery rejected a normal H.264/AAC MP4 because its embedded mov_text stream was missing from the bounded decoder whitelist. The subtitle was not output, but libavformat still initialized it while probing. Adding mov_text allowed the same input to produce animated AVIF and sentence audio. Diagnosis and follow-up
  • HiBy M500: a predecessor validation build produced two independently named, valid 32-frame AVIFs and visibly looped the second in AnkiDroid. Device result

These runs establish the actual failure modes and validate predecessor integrated builds. They are not exact-head coverage of this standalone upstream PR; that is why the vendor matrix remains open below.

Approaches tried before this one

Fixed 640×640 capability check

This was simple, but it asked the wrong question. An encoder may support a useful portrait or landscape canvas while rejecting 640×640, or advertise 640×640 while imposing alignment/rate/conditional-range constraints elsewhere. Forcing visible content into that square also risks distortion.

Why rejected: codec capability is a constraint-solving problem, not a single isSizeSupported(640, 640) check.

First advertised AV1 encoder

Codec enumeration order is not a quality or compatibility guarantee. The first encoder may lack planar YUV420, constant-quality mode, a usable quality range, 8 fps support, or enough resolution for the source while a later candidate works.

Why rejected: it turns vendor ordering into application behavior and misses valid alternatives.

Normalize raw AV1 OBUs in Kotlin, then remux in a second FFmpeg pass

An earlier implementation wrote an intermediate AV1 stream, normalized OBUs in Kotlin, and remuxed that file into AVIF.

Why rejected: it duplicated AV1 syntax handling outside FFmpeg, added a second native session and temporary file, complicated cancellation/ownership, and still could not safely repair MediaCodec's internal GOP/reference state. The native invariants are handled by the narrowly patched dependency in #99 instead.

Accept any file that FFmpeg reports as successful

A non-empty file and zero exit code do not prove a valid animated AVIF. Container boxes can point outside the file, sample sizes can disagree, timing can be unusable, and frame 1 may not be independently decodable.

Why rejected: animation is optional; failing closed to the still image is safer than handing corrupt media to Anki.

Retry broadly or permit every input

Blind retries repeat deterministic codec/container failures. Passing extension arguments, credentials, DRM/transient sources, or unrestricted remote protocols into FFmpeg expands both the failure and security surface.

Why rejected: select capabilities before encoding and reject unsafe inputs before native execution.

Why this solution

The final path is probe → select → encode once → validate → deliver or fall back.

1. Freeze and inspect the request

The request carries the resolved scene timing and a stable video-input description. Capture rejects missing/changed state rather than guessing at a different stream or time range.

ffprobe extracts the facts needed for encoding:

  • coded width and height;
  • sample-aspect ratio;
  • display rotation;
  • pixel format and bit depth;
  • color primaries and transfer function;
  • selected video stream.

Invalid dimensions, protected/HDR/BT.2020 content, unsafe sources, and unusable probe output fail before an output file is committed. 8-bit and 10-bit SDR input are accepted; 10-bit input is converted to yuv420p, not advertised as preserved 10-bit output.

2. Evaluate every usable hardware AV1 encoder

SceneAv1EncoderSelector examines each MediaCodec AV1 candidate for:

  • encoder status and hardware suitability;
  • planar YUV420 support;
  • constant-quality mode and target quality;
  • width/height alignment;
  • size/rate and conditional width constraints at 8 fps;
  • a canvas bounded to 640×640.

Display geometry is derived from coded dimensions, SAR, and rotation. Visible content is scaled without intentional distortion and then black-padded into a supported 16-pixel-aligned canvas. Candidate ranking first preserves content resolution, then avoids an unnecessarily large canvas.

This separates content size from encoder canvas size—the key distinction the fixed-square approach lacked.

3. Use one native encode/mux command

The selected stream is sampled at 8 fps, scaled/padded, converted to yuv420p, encoded with the chosen av1_mediacodec encoder, and muxed directly into looping AVIF.

The two-pass Kotlin OBU/remux pipeline is gone. #99 supplies the native parser/MediaCodec reset/configuration/first-keyframe fixes required for this direct path.

The command remains bounded:

  • maximum 10 seconds / 80 frames;
  • maximum 640 pixels per dimension;
  • 10 MiB output limit;
  • restricted decoders and protocols;
  • TLS verification and bounded remote I/O;
  • mov_text allowed so an embedded MP4 subtitle track does not invalidate otherwise safe video.

4. Validate before ownership transfer

A successful native return is necessary but not sufficient. AnimatedAvifValidator checks the output's:

  • AVIF brands and AV1 configuration;
  • dimensions and frame count;
  • duration/timing tables;
  • sample tables, sample sizes, and media-data bounds;
  • first-sample sync evidence when stss is present.

If command construction, probing, encoding, validation, or delivery fails, partial output is removed and the caller keeps the still-image fallback.

5. Make cancellation and cleanup native-aware

Kotlin cancellation does not mean native FFmpeg has already stopped. The implementation therefore separates Kotlin ownership from native-use leases:

  • cancellation is rethrown rather than converted into an ordinary failure;
  • running native work is cancelled and awaited before descriptors/files are released;
  • invalid and partial output is deleted;
  • a validated file remains owned by the capture operation until delivery succeeds;
  • cancellation after validation but before delivery cannot orphan the file.

6. Produce useful diagnostics without leaking inputs

The original path had many early returns and almost no explanation; one device test could return a still without revealing which gate fired.

Tagged SceneMining logs now cover request rejection, probe output, encoder selection, native failure, validation, cancellation, and cleanup. Remote paths and embedded URLs are reduced/redacted, and FFmpegKit's own unredacted logcat output remains suppressed.

Input policy

The path supports seekable local inputs and constrained public HTTP(S)/HLS sources. It intentionally rejects or falls back for:

  • DRM/protected media;
  • HDR transfer functions and BT.2020 primaries;
  • DASH, torrents/transient, or nonseekable sources;
  • extension-provided FFmpeg arguments;
  • credential-bearing URLs and sensitive signed/auth query keys;
  • non-allowlisted headers;
  • devices with no qualifying hardware AV1 encoder.

Those are explicit safety/portability decisions, not accidental unsupported cases.

Scope

This PR includes MediaCodec selection, geometry, probe/command construction, capture ownership, AVIF validation, safe input handling, redacted diagnostics, and focused tests.

It deliberately excludes:

Verification

Verified at exact head f0624257e6237e7fd796f08104f9e4ac4086abbf:

  • ./gradlew :app:compileDebugKotlinBUILD SUCCESSFUL
  • focused eu.kanade.tachiyomi.ui.player.scene.* tests — 69 passed, 0 failed, 0 errors, 0 skipped
  • all changed scene files pass scoped Spotless checks
  • git diff --check passes

The focused test task initially encountered an unrelated baseline test-compile defect: ReaderOcrSourceTest omits the now-required mokuroAvailable argument. The same mismatch exists on upstream main; it was temporarily supplied only in the isolated verification worktree, then reverted. No such change is in this PR.

Upstream PR CI currently stops earlier at repository-wide Spotless failures in untouched presentation-core code, so the red check does not represent a compile/test failure in this diff.

What the tests cover

Focused coverage exercises:

  • landscape, portrait, square, narrow, odd/minimum-size, alignment, conditional-range, and competing-encoder selection;
  • SAR/rotation geometry and exact scale/pad filters;
  • 8/10-bit SDR acceptance and HDR/protected-media rejection;
  • one-command AVIF arguments and decoder/protocol restrictions;
  • AVIF brands, bounds, dimensions, timing, sample sizes, payload, and first-sync validation;
  • success, unavailable encoder, dimension mismatch, argument failure, partial-output deletion, cancellation during native work, and undelivered-output cleanup;
  • tagged/redacted diagnostics.

Remaining device work

  • Qualcomm, MediaTek, Exynos, and Tensor MediaCodec implementations where available
  • devices with no AV1 encoder (must keep the still fallback)
  • rotated, anamorphic/SAR, odd-sized, portrait, landscape, square, 8-bit SDR, and 10-bit SDR media
  • local files, SAF providers, public HTTPS MP4, and public HLS
  • frame-zero decode, looping, duration, aspect ratio, padding, and Anki import
  • cancellation during probe/encode and cancellation after validation
  • malformed output, native failure, and repeated captures
  • confirm rejected credentialed/HDR/DRM/transient inputs fail closed without leaking paths

This PR is the selected design because it asks the platform what it can encode, preserves display geometry, performs one bounded native operation, validates the artifact before delivery, and treats animation as optional. It does not claim universal vendor compatibility until the device matrix is complete.

@bee-san

bee-san commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Local verification at f0624257e6237e7fd796f08104f9e4ac4086abbf

  • ./gradlew :app:compileDebugKotlinBUILD SUCCESSFUL (1m 34s)
  • ✅ Focused eu.kanade.tachiyomi.ui.player.scene.* debug unit tests — 69 passed, 0 failed, 0 errors, 0 skipped
  • ✅ Detached verification worktree restored clean; no PR files changed

The focused-test command initially could not compile the repository's complete debug test source set because the existing ReaderOcrSourceTest calls availableSources(...) without the now-required mokuroAvailable argument. This same mismatch is present on current upstream main and is outside this PR's diff. I temporarily supplied mokuroAvailable = true locally, ran the focused scene tests, then reverted that temporary edit before checking cleanliness.

GitHub CI itself stopped earlier at repository-wide Spotless in untouched code (presentation-core/.../LazyListState.kt), consistent with the known upstream formatting issue; it did not reach build/tests.

@sohilsayed

Copy link
Copy Markdown
Owner

closed as we are going in a different direction

@sohilsayed sohilsayed closed this Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants