diff --git a/android/app/proguard-rules.pro b/android/app/proguard-rules.pro new file mode 100644 index 00000000..b863a89d --- /dev/null +++ b/android/app/proguard-rules.pro @@ -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 { *; } diff --git a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/PlezyRenderersFactory.kt b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/PlezyRenderersFactory.kt index ae0b826c..0c08ca11 100644 --- a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/PlezyRenderersFactory.kt +++ b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/PlezyRenderersFactory.kt @@ -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 + ) { + 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, diff --git a/scripts/check_shrinker_rules.py b/scripts/check_shrinker_rules.py new file mode 100755 index 00000000..a704287b --- /dev/null +++ b/scripts/check_shrinker_rules.py @@ -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()) diff --git a/scripts/ci_guard_checks.sh b/scripts/ci_guard_checks.sh index 2af8a7a4..91291ad6 100644 --- a/scripts/ci_guard_checks.sh +++ b/scripts/ci_guard_checks.sh @@ -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 \ diff --git a/scripts/test_check_shrinker_rules.py b/scripts/test_check_shrinker_rules.py new file mode 100755 index 00000000..633a788d --- /dev/null +++ b/scripts/test_check_shrinker_rules.py @@ -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" + " (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" + " (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()