fix(android): keep the FFmpeg audio decoder through R8
Flutter enables minification for every release build, and nothing but a
keep rule reaches androidx.media3.decoder.ffmpeg. DefaultRenderersFactory
instantiates FfmpegAudioRenderer with Class.forName, media3's consumer
rules only -keepclassmembers its constructor, and this project had no
proguard-rules.pro at all, so R8 shrank the renderer out of the shipped
dex and the reflective lookup failed with ClassNotFoundException. The
same pass dropped FfmpegAudioDecoder.growOutputBuffer, which ffmpeg_jni
resolves in JNI_OnLoad and whose absence fails the whole
System.loadLibrary("ffmpegJNI") call.
Release builds therefore lost every codec that decoder adds. TrueHD and
DTS-HD fell through to MediaCodecAudioRenderer, which has no decoder for
them, so a 4K Dolby Vision file died with NO_SUITABLE_DECODER_ERROR and
handed off to the mpv fallback — losing ExoPlayer's Profile 7 to 8.1
conversion on hardware that could have direct-played it. Only debug
builds, where R8 never runs, exercised the working path.
Keep the package and the type named in the JNI callback descriptor, and
guard the invariant so it cannot silently rot again: check_shrinker_rules
fails when an app class in a reflected namespace, a FindClass target, a
native callback member, or a descriptor type has no keep covering it.
Also record the built audio renderers, because whether the extension
loaded is otherwise indistinguishable in an uploaded log.
close #1703
This commit is contained in:
Vendored
+20
@@ -0,0 +1,20 @@
|
||||
# Flutter turns minification on for every release build (FlutterPlugin sets
|
||||
# releaseBuildType.isMinifyEnabled), and appends this file when it exists. Anything the
|
||||
# app reaches only by name — reflection or JNI — therefore needs an explicit keep here.
|
||||
|
||||
# The bundled Media3 FFmpeg audio decoder (ALAC, DTS, DTS-HD, TrueHD, ...).
|
||||
#
|
||||
# DefaultRenderersFactory instantiates FfmpegAudioRenderer through Class.forName and no
|
||||
# app code references it, so R8 shrinks the class away; media3's own consumer rules only
|
||||
# -keepclassmembers its constructor, which neither keeps the class nor pins its name.
|
||||
# ffmpeg_jni.cc separately resolves FfmpegAudioDecoder and its growOutputBuffer callback
|
||||
# by name in JNI_OnLoad, and returns JNI_ERR when either is missing, which fails the whole
|
||||
# System.loadLibrary("ffmpegJNI") call.
|
||||
#
|
||||
# Without these keeps a release build silently loses every codec this decoder adds:
|
||||
# TrueHD/DTS-HD land on MediaCodecAudioRenderer, which has no decoder for them, and
|
||||
# playback bails to the mpv fallback and loses ExoPlayer's Dolby Vision handling (#1703).
|
||||
-keep class androidx.media3.decoder.ffmpeg.** { *; }
|
||||
|
||||
# growOutputBuffer's JNI descriptor names this type, so it may not be renamed either.
|
||||
-keep class androidx.media3.decoder.SimpleDecoderOutputBuffer { *; }
|
||||
@@ -18,6 +18,7 @@ import androidx.media3.exoplayer.Renderer
|
||||
import androidx.media3.exoplayer.analytics.PlayerId
|
||||
import androidx.media3.exoplayer.audio.AudioOutput
|
||||
import androidx.media3.exoplayer.audio.AudioOutputProvider
|
||||
import androidx.media3.exoplayer.audio.AudioRendererEventListener
|
||||
import androidx.media3.exoplayer.audio.AudioSink
|
||||
import androidx.media3.exoplayer.audio.AudioTrackAudioOutputProvider
|
||||
import androidx.media3.exoplayer.audio.DefaultAudioSink
|
||||
@@ -115,6 +116,40 @@ class PlezyRenderersFactory(context: Context) : DefaultRenderersFactory(context)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Records which audio renderers the session actually got. Whether the bundled
|
||||
* FFmpeg renderer loaded decides between decoding TrueHD/DTS-HD in ExoPlayer and
|
||||
* bailing to the mpv fallback, and nothing else in an uploaded log distinguishes
|
||||
* the two (#1703).
|
||||
*/
|
||||
override fun buildAudioRenderers(
|
||||
context: Context,
|
||||
extensionRendererMode: Int,
|
||||
mediaCodecSelector: MediaCodecSelector,
|
||||
enableDecoderFallback: Boolean,
|
||||
audioSink: AudioSink,
|
||||
eventHandler: Handler,
|
||||
eventListener: AudioRendererEventListener,
|
||||
out: ArrayList<Renderer>
|
||||
) {
|
||||
val firstAudioIndex = out.size
|
||||
super.buildAudioRenderers(
|
||||
context,
|
||||
extensionRendererMode,
|
||||
mediaCodecSelector,
|
||||
enableDecoderFallback,
|
||||
audioSink,
|
||||
eventHandler,
|
||||
eventListener,
|
||||
out
|
||||
)
|
||||
audioDiagnosticsLogger?.invoke(
|
||||
"info",
|
||||
"audio",
|
||||
"Audio renderers: " + out.subList(firstAudioIndex, out.size).joinToString { it.name }
|
||||
)
|
||||
}
|
||||
|
||||
override fun buildAudioSink(
|
||||
context: Context,
|
||||
enableFloatOutput: Boolean,
|
||||
|
||||
Executable
+198
@@ -0,0 +1,198 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Validate that name-reached Android classes and members survive R8.
|
||||
|
||||
Flutter minifies every release build, so anything the app reaches only by name is
|
||||
invisible to R8 and gets shrunk or renamed away. Three such surfaces exist:
|
||||
|
||||
* app-module classes placed in a library namespace so that library discovers them with
|
||||
``Class.forName`` (the bundled ``androidx.media3.decoder.ffmpeg`` audio decoder);
|
||||
* classes resolved from native code with ``FindClass``;
|
||||
* members resolved from native code with ``Get*MethodID``/``Get*FieldID``, including
|
||||
every type named in the descriptor those lookups pass.
|
||||
|
||||
None of them has a compile-time reference, so only ``android/app/proguard-rules.pro``
|
||||
keeps them, and losing a keep surfaces solely as broken behaviour in a release build.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
PROGUARD_RULES = Path("android/app/proguard-rules.pro")
|
||||
APP_JAVA_ROOT = Path("android/app/src/main/java")
|
||||
CPP_ROOT = Path("android/app/src/main/cpp")
|
||||
NATIVE_SUFFIXES = {".c", ".cc", ".cpp", ".h", ".hpp"}
|
||||
# Namespaces the app borrows from a dependency purely so that dependency can reflect on
|
||||
# them. A class under one of these has no direct caller by construction.
|
||||
REFLECTED_NAMESPACES = ("androidx/media3/",)
|
||||
# Framework types live on the bootclasspath, never in the app's dex, so R8 cannot rename
|
||||
# them and they need no keep.
|
||||
PLATFORM_PREFIXES = ("java.", "javax.", "android.")
|
||||
|
||||
_STRING_LITERAL = re.compile(r'"((?:[^"\\]|\\.)*)"')
|
||||
_ADJACENT_LITERALS = re.compile(r'"((?:[^"\\]|\\.)*)"\s*"((?:[^"\\]|\\.)*)"')
|
||||
_JCLASS_ASSIGNMENT = re.compile(r"(\w+)\s*=\s*[\w:>.\-]*?FindClass\(\s*" + _STRING_LITERAL.pattern + r"\s*\)")
|
||||
_MEMBER_LOOKUP = re.compile(
|
||||
r"Get(?:Static)?(?:Method|Field)ID\(\s*(\w+)\s*,\s*"
|
||||
+ _STRING_LITERAL.pattern
|
||||
+ r"\s*,\s*"
|
||||
+ _STRING_LITERAL.pattern
|
||||
+ r"\s*\)"
|
||||
)
|
||||
_DESCRIPTOR_CLASS = re.compile(r"L([\w/$]+);")
|
||||
# Only -keep and -keepclasseswithmembers protect a class from both shrinking and
|
||||
# renaming. -keepclassmembers/-keepclassmembernames cover members alone, and the
|
||||
# -keepnames family allows shrinking, so none of them save a class nothing references.
|
||||
_KEEP = re.compile(
|
||||
r"^-(?:keep|keepclasseswithmembers)((?:\s*,\s*\w+)*)\s+(?:class|interface|enum)\s+(\S+)"
|
||||
r"(?:\s*\{(.*?)\})?",
|
||||
re.MULTILINE | re.DOTALL,
|
||||
)
|
||||
_UNSAFE_MODIFIERS = ("allowshrinking", "allowobfuscation")
|
||||
|
||||
|
||||
class Keep:
|
||||
"""One parsed ``-keep`` rule: a class-name pattern plus its member block."""
|
||||
|
||||
def __init__(self, pattern: str, modifiers: str, members: str) -> None:
|
||||
self.pattern = pattern
|
||||
self.modifiers = modifiers
|
||||
self.members = members
|
||||
self._regex = re.compile(
|
||||
"".join(
|
||||
# ** spans package separators, * does not, ? is a single character.
|
||||
{"**": r".*", "*": r"[^.]*", "?": r"."}.get(token, re.escape(token))
|
||||
for token in re.findall(r"\*\*|[*?]|[^*?]+", pattern)
|
||||
)
|
||||
)
|
||||
|
||||
def matches_class(self, binary_name: str) -> bool:
|
||||
return self._regex.fullmatch(binary_name) is not None
|
||||
|
||||
def keeps_member(self, member: str) -> bool:
|
||||
return "*" in self.members or member in self.members
|
||||
|
||||
@property
|
||||
def includes_descriptor_classes(self) -> bool:
|
||||
return "includedescriptorclasses" in self.modifiers
|
||||
|
||||
|
||||
def _parse_keeps(rules: str) -> list[Keep]:
|
||||
keeps = []
|
||||
for match in _KEEP.finditer(rules):
|
||||
modifiers = match.group(1) or ""
|
||||
if any(modifier in modifiers for modifier in _UNSAFE_MODIFIERS):
|
||||
continue
|
||||
keeps.append(Keep(match.group(2).replace("$", "."), modifiers, match.group(3) or ""))
|
||||
return keeps
|
||||
|
||||
|
||||
def _collapse_adjacent_literals(source: str) -> str:
|
||||
"""Join C string-literal concatenation so descriptors read as one token."""
|
||||
previous = None
|
||||
while previous != source:
|
||||
previous = source
|
||||
source = _ADJACENT_LITERALS.sub(lambda m: f'"{m.group(1)}{m.group(2)}"', source)
|
||||
return source
|
||||
|
||||
|
||||
def _binary_name(jni_name: str) -> str:
|
||||
return jni_name.replace("/", ".").replace("$", ".")
|
||||
|
||||
|
||||
def _native_sources(root: Path) -> list[Path]:
|
||||
cpp_root = root / CPP_ROOT
|
||||
if not cpp_root.is_dir():
|
||||
return []
|
||||
return sorted(path for path in cpp_root.rglob("*") if path.suffix in NATIVE_SUFFIXES)
|
||||
|
||||
|
||||
def _reflected_classes(root: Path) -> list[str]:
|
||||
java_root = root / APP_JAVA_ROOT
|
||||
if not java_root.is_dir():
|
||||
return []
|
||||
classes = []
|
||||
for source in sorted(java_root.rglob("*.java")):
|
||||
relative = source.relative_to(java_root).as_posix()
|
||||
if relative.startswith(REFLECTED_NAMESPACES):
|
||||
classes.append(_binary_name(relative[: -len(".java")]))
|
||||
return classes
|
||||
|
||||
|
||||
def _check_native_lookups(root: Path, keeps: list[Keep], errors: list[str]) -> None:
|
||||
for source in _native_sources(root):
|
||||
text = _collapse_adjacent_literals(source.read_text(encoding="utf-8"))
|
||||
owners = {match.group(1): _binary_name(match.group(2)) for match in _JCLASS_ASSIGNMENT.finditer(text)}
|
||||
label = source.relative_to(root).as_posix()
|
||||
|
||||
for owner in sorted(set(owners.values())):
|
||||
if not any(keep.matches_class(owner) for keep in keeps):
|
||||
errors.append(f"{label} resolves {owner} with FindClass but no -keep covers it")
|
||||
|
||||
for match in _MEMBER_LOOKUP.finditer(text):
|
||||
variable, member, descriptor = match.group(1), match.group(2), match.group(3)
|
||||
owner = owners.get(variable)
|
||||
if owner is None:
|
||||
errors.append(
|
||||
f"{label} looks up member '{member}' on '{variable}', which this check cannot trace "
|
||||
f"back to a FindClass call; assign the jclass from FindClass or extend {Path(__file__).name}"
|
||||
)
|
||||
continue
|
||||
matching = [keep for keep in keeps if keep.matches_class(owner)]
|
||||
if not any(keep.keeps_member(member) for keep in matching):
|
||||
errors.append(f"{label} resolves {owner}.{member} from native code but no -keep retains that member")
|
||||
for referenced in _DESCRIPTOR_CLASS.findall(descriptor):
|
||||
name = _binary_name(referenced)
|
||||
if name.startswith(PLATFORM_PREFIXES):
|
||||
continue
|
||||
if any(keep.includes_descriptor_classes for keep in matching):
|
||||
continue
|
||||
if not any(keep.matches_class(name) for keep in keeps):
|
||||
errors.append(
|
||||
f"{label} names {name} in the descriptor of {owner}.{member}, so renaming it breaks the "
|
||||
f"lookup, but no -keep covers it (or mark the owner -keep,includedescriptorclasses)"
|
||||
)
|
||||
|
||||
|
||||
def validate(root: Path) -> list[str]:
|
||||
root = root.resolve()
|
||||
reflected = _reflected_classes(root)
|
||||
has_native = bool(_native_sources(root))
|
||||
if not reflected and not has_native:
|
||||
return []
|
||||
|
||||
rules_path = root / PROGUARD_RULES
|
||||
if not rules_path.is_file():
|
||||
return [
|
||||
f"{PROGUARD_RULES} is missing, so R8 will shrink name-reached classes out of release "
|
||||
f"builds{': ' + ', '.join(reflected) if reflected else ''}"
|
||||
]
|
||||
|
||||
keeps = _parse_keeps(rules_path.read_text(encoding="utf-8"))
|
||||
errors = []
|
||||
for binary_name in reflected:
|
||||
if not any(keep.matches_class(binary_name) for keep in keeps):
|
||||
errors.append(
|
||||
f"{binary_name} lives in a reflected namespace but no -keep in {PROGUARD_RULES} covers it"
|
||||
)
|
||||
_check_native_lookups(root, keeps, errors)
|
||||
return errors
|
||||
|
||||
|
||||
def main(argv: list[str] | None = None) -> int:
|
||||
parser = argparse.ArgumentParser()
|
||||
parser.add_argument("--root", type=Path, default=Path(__file__).resolve().parents[1])
|
||||
args = parser.parse_args(argv)
|
||||
errors = validate(args.root)
|
||||
if errors:
|
||||
for error in errors:
|
||||
print(f"error: {error}")
|
||||
return 1
|
||||
print("Name-reached Android classes and members are covered by shrinker keep rules.")
|
||||
return 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
raise SystemExit(main())
|
||||
@@ -18,6 +18,7 @@ for checker in \
|
||||
scripts/check_build_workflow.py \
|
||||
scripts/check_apple_spm_locks.py \
|
||||
scripts/check_tvos_test_wiring.py \
|
||||
scripts/check_shrinker_rules.py \
|
||||
scripts/verify_runtime_inputs.py \
|
||||
scripts/check_workflow_security.py \
|
||||
scripts/check_workflow_action_pins.py \
|
||||
|
||||
Executable
+157
@@ -0,0 +1,157 @@
|
||||
#!/usr/bin/env python3
|
||||
|
||||
import importlib.util
|
||||
import tempfile
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
SCRIPT = Path(__file__).with_name("check_shrinker_rules.py")
|
||||
SPEC = importlib.util.spec_from_file_location("check_shrinker_rules", SCRIPT)
|
||||
CHECKER = importlib.util.module_from_spec(SPEC)
|
||||
assert SPEC.loader is not None
|
||||
SPEC.loader.exec_module(CHECKER)
|
||||
|
||||
# The descriptor is split across two literals exactly as ffmpeg_jni.cc has it, so the
|
||||
# fixture also covers C string concatenation.
|
||||
JNI_SOURCE = """
|
||||
JNIEXPORT jint JNI_OnLoad(JavaVM* vm, void* reserved) {
|
||||
jclass clazz = env->FindClass("androidx/media3/decoder/ffmpeg/FfmpegAudioDecoder");
|
||||
growOutputBufferMethod = env->GetMethodID(
|
||||
clazz, "growOutputBuffer",
|
||||
"(Landroidx/media3/decoder/"
|
||||
"SimpleDecoderOutputBuffer;I)Ljava/nio/ByteBuffer;");
|
||||
return JNI_VERSION_1_6;
|
||||
}
|
||||
"""
|
||||
|
||||
FULL_RULES = (
|
||||
"-keep class androidx.media3.decoder.ffmpeg.** { *; }\n"
|
||||
"-keep class androidx.media3.decoder.SimpleDecoderOutputBuffer { *; }\n"
|
||||
)
|
||||
|
||||
|
||||
class ShrinkerRulesCheckerTest(unittest.TestCase):
|
||||
def setUp(self) -> None:
|
||||
self.temporary = tempfile.TemporaryDirectory()
|
||||
self.root = Path(self.temporary.name)
|
||||
java_path = self.root / "android/app/src/main/java/androidx/media3/decoder/ffmpeg"
|
||||
java_path.mkdir(parents=True)
|
||||
(java_path / "FfmpegAudioRenderer.java").write_text("// fixture\n", encoding="utf-8")
|
||||
(java_path / "FfmpegAudioDecoder.java").write_text("// fixture\n", encoding="utf-8")
|
||||
self.cpp_path = self.root / "android/app/src/main/cpp/media3_ffmpeg_decoder"
|
||||
self.cpp_path.mkdir(parents=True)
|
||||
self.jni_path = self.cpp_path / "ffmpeg_jni.cc"
|
||||
self.jni_path.write_text(JNI_SOURCE, encoding="utf-8")
|
||||
self.rules_path = self.root / "android/app/proguard-rules.pro"
|
||||
self.rules_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
def tearDown(self) -> None:
|
||||
self.temporary.cleanup()
|
||||
|
||||
def _write_rules(self, rules: str) -> None:
|
||||
self.rules_path.write_text(rules, encoding="utf-8")
|
||||
|
||||
@staticmethod
|
||||
def _failure_kinds(errors: list[str]) -> set[str]:
|
||||
kinds = set()
|
||||
for error in errors:
|
||||
for marker in ("reflected namespace", "with FindClass", "from native code", "in the descriptor"):
|
||||
if marker in error:
|
||||
kinds.add(marker)
|
||||
return kinds
|
||||
|
||||
@staticmethod
|
||||
def _every_failure_kind() -> set[str]:
|
||||
return {"reflected namespace", "with FindClass", "from native code", "in the descriptor"}
|
||||
|
||||
def test_repository_rules_cover_every_name_reached_class_and_member(self) -> None:
|
||||
self.assertEqual([], CHECKER.validate(Path(__file__).resolve().parents[1]))
|
||||
|
||||
def test_package_keep_plus_descriptor_keep_passes(self) -> None:
|
||||
self._write_rules(FULL_RULES)
|
||||
|
||||
self.assertEqual([], CHECKER.validate(self.root))
|
||||
|
||||
def test_missing_rules_file_is_reported(self) -> None:
|
||||
errors = CHECKER.validate(self.root)
|
||||
|
||||
self.assertEqual(1, len(errors))
|
||||
self.assertIn("proguard-rules.pro is missing", errors[0])
|
||||
|
||||
def test_uncovered_reflected_class_is_reported(self) -> None:
|
||||
self._write_rules(
|
||||
"-keep class androidx.media3.decoder.ffmpeg.FfmpegAudioDecoder { *; }\n"
|
||||
"-keep class androidx.media3.decoder.SimpleDecoderOutputBuffer { *; }\n"
|
||||
)
|
||||
|
||||
self.assertEqual(
|
||||
["androidx.media3.decoder.ffmpeg.FfmpegAudioRenderer lives in a reflected namespace "
|
||||
"but no -keep in android/app/proguard-rules.pro covers it"],
|
||||
CHECKER.validate(self.root),
|
||||
)
|
||||
|
||||
def test_single_star_does_not_span_packages(self) -> None:
|
||||
self._write_rules("-keep class androidx.media3.* { *; }\n")
|
||||
|
||||
self.assertEqual(self._every_failure_kind(), self._failure_kinds(CHECKER.validate(self.root)))
|
||||
|
||||
def test_keepclassmembers_alone_does_not_count_as_a_keep(self) -> None:
|
||||
# media3's consumer rules only keep the constructor, which neither keeps the class
|
||||
# nor pins its name. That is the state that shipped a release without the decoder.
|
||||
self._write_rules(
|
||||
"-keepclassmembers class androidx.media3.decoder.ffmpeg.FfmpegAudioRenderer {\n"
|
||||
" <init>(android.os.Handler);\n"
|
||||
"}\n"
|
||||
)
|
||||
|
||||
self.assertEqual(self._every_failure_kind(), self._failure_kinds(CHECKER.validate(self.root)))
|
||||
|
||||
def test_class_keep_without_the_native_callback_member_is_reported(self) -> None:
|
||||
self._write_rules(
|
||||
"-keep class androidx.media3.decoder.ffmpeg.** {\n"
|
||||
" <init>(android.os.Handler);\n"
|
||||
"}\n"
|
||||
"-keep class androidx.media3.decoder.SimpleDecoderOutputBuffer { *; }\n"
|
||||
)
|
||||
|
||||
self.assertEqual(
|
||||
["android/app/src/main/cpp/media3_ffmpeg_decoder/ffmpeg_jni.cc resolves "
|
||||
"androidx.media3.decoder.ffmpeg.FfmpegAudioDecoder.growOutputBuffer from native code "
|
||||
"but no -keep retains that member"],
|
||||
CHECKER.validate(self.root),
|
||||
)
|
||||
|
||||
def test_renamable_descriptor_class_is_reported(self) -> None:
|
||||
self._write_rules("-keep class androidx.media3.decoder.ffmpeg.** { *; }\n")
|
||||
|
||||
errors = CHECKER.validate(self.root)
|
||||
|
||||
self.assertEqual(1, len(errors))
|
||||
self.assertIn("names androidx.media3.decoder.SimpleDecoderOutputBuffer in the descriptor", errors[0])
|
||||
|
||||
def test_includedescriptorclasses_satisfies_the_descriptor_requirement(self) -> None:
|
||||
self._write_rules("-keep,includedescriptorclasses class androidx.media3.decoder.ffmpeg.** { *; }\n")
|
||||
|
||||
self.assertEqual([], CHECKER.validate(self.root))
|
||||
|
||||
def test_platform_descriptor_types_need_no_keep(self) -> None:
|
||||
# java.nio.ByteBuffer is in the return descriptor and must not be demanded.
|
||||
self._write_rules(FULL_RULES)
|
||||
|
||||
self.assertEqual([], CHECKER.validate(self.root))
|
||||
|
||||
def test_untraceable_member_lookup_fails_loudly(self) -> None:
|
||||
self.jni_path.write_text(
|
||||
'growOutputBufferMethod = env->GetMethodID(someClass, "growOutputBuffer", "()V");\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
self._write_rules(FULL_RULES)
|
||||
|
||||
errors = CHECKER.validate(self.root)
|
||||
|
||||
self.assertEqual(1, len(errors))
|
||||
self.assertIn("cannot trace", errors[0])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user